feat: add support for processing inline code #336
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for processing inline code
Apologies in advance for the long description but providing for two reasons:
inline code
processing other than the very high-level comments in Support for inline code syntax highlighting #250 so wanted to provide context on why this approach was taken to hopefully pre-emptively address as many questions as possible to accelerate approval/merge 😃TL;DR
console.log('Hello!')
console.log('Hello!'){:js}
Design Goals
v0.41.2
- Achieved with two exceptions specific to generated HTML output, neither of which should cause any functional backwards compatibility issues UNTIL the user opts-in toinline code
processing and even then, they may or may not cause issues (see Styling for more information):.expressive-code
but is now wrapped in a.ec-container-block
class.ec-line-inline
class.ec-container-inline
CSS class - New class included in generated styles that did not previously existec-type
attributed added topre
elementinline code
- AchievedFunctional Implementation
Four (4) different approaches were considered:
inline
.inline
<hookname>Inline
set of hooksinline
blocksExpressiveCodeBlock
,ExpressiveInlineCodeBlock
) and Hook function signatures to differentiate parameters (e.g.,inline
does not support gutters soaddGutterElement
wouldn't be defined on it's hook properties)supportedCodeBlockTypes
plugin option that returns an array of which types of code blocks that the plugin wants its hooks called for with the default beingblock
only ifsupportedCodeBlockTypes
is not defined by the plugininline
andcodeblock
differently within the existing hooksinline
on only a subset of hooks)sideEffects: false
in package.json which leads to unnecessary increase to bundle size in some situations. If a style registration API were to be added, it could be coupled with a hook registration API.Decision: Option 2 is the most robust and type-safe approach, however it requires much more code to be written to accomidate. Additionally, it's likely an over-enginered approach since there are only a few differences between
inline
andblock
in terms ofExpressiveCodeBlock
handling. For these reasons, Option 3 was chosen since it is least invasive and most straightforward.Styling
Unlike the functional implementation, accounting for styles was not a straightforward solution. There were two main reasons for this:
v0.41.2
is written to assume there are onlycode blocks
within.expressive-code
so styles are loosely applied in certain cases. For example, any.ec-line
, even if it doesn't live within apre
is styled by plugin-frames. Similarly,expressive-code
itself styles any .ec-line that lives anywhere inside of.expressive-code
.pre
and more specificallypre > code
? Are there situations that possibly I'm overlooking where anec-line
could exist outside ofpre > code
that would require styling directly under.expressive-code
?ExpressiveCodeBlock
instances which may now contain a mixture ofinline
andblock
so there needs to be a way to target styles specific to the ExpressiveCodeBlockType for the given code block.Given the above, the approach taken had to ensure that it did not use some of the existing classes, like
.ec-line
to avoid styling issues wheninline code
is processed.To achieve this, a couple of minor adjustments to generated styles and HTML were required as mentioned in Design Goals. The current approach is as follows with the changes between
v0.41.2
and this PR highlighted:Single
code block
rendered regardless ofinline
configuration:Multiple
code blocks
rendered regardless ofinline
configuration:The structure above is the same for
inline code
(single or multiple) whenrender
is called with onlyinline
block(s) with the only differences beingspan
instead ofdiv
and the classes/attributes beinginline
variants.Single
inline code
rendered withinline
enabledMultiple
inline code
rendered withinline
enabledWhen a mixture of block types is passed to render, the structure with this PR is as follows:
Bottom Line
While the existing PR achieves the desired outcome with minimal changes to generated styles and HTML, I believe that a better approach would be to change the generated HTML to structure each individual
ExpressiveCodeBlock
in its own container within the outer.expressive-code
wrapper. Doing so would provide the following benefits:ExpressCodeBlocks
are rendered and what theirExpressCodeBlockType
isinline
andblock
by targeting classes and/or data attributes instead of tag namesblock
andinline
could use the same classes (e.g.,ec-line
instead ofec-line
&ec-line-inline
)This approach, in some situations and depending on styling could introduce a breaking change. With that said, EC and it's stock plugins would all be updated and versioned to handle in a release so it really comes down to non-EC core plugins and any user specified CSS styling that is applied. The user based styling would be easy enough to account for by the users as they upgrade EC. Looking at the community plugins, most define
peerDependencies
as^0.#.#
(e.g., color chips). Unfortunately, it does not appear thatpeerDependencies
semver rules work the same asdependencies/devDependencies
when it comes toinstall/update
. Withdependencies/devDependencies
, package managers respect the leading zero semver and treat any minor bump as a breaking change and therefore^0.#.#
won't update when minor increases. However, forpeerDependencies
, it's more loose and will not warn when the segment that is considered "major", in this case the minor value itself, increases. For this reason, there is no automatic way to warn users upon upgrading EC that a plugin they are using doesn't support the EC version. Given this, it may still be necessary to use different classes (e.g.,ec-line
andec-line-inline
) to minimize potential breaking changes and even then, depending on the styles that the plugin adds and what selectors they target, it could blead in toinline
blocks. One thing to consider is making this change and releasing asv1.0.0
which is something I think is a goal anyway. This would allow for users to be warned on upgrading EC if the plugin doesn't supportv1
yet. One exception to this is twoslash which defines peer deps as>=0.40.0
.Suggested HTML structure:
Multiple
code blocks
Multiple
inline code
Multiple
inline code
&code block
Resolves #250