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 codeprocessing 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 codeprocessing and even then, they may or may not cause issues (see Styling for more information):.expressive-codebut is now wrapped in a.ec-container-blockclass.ec-line-inlineclass.ec-container-inlineCSS class - New class included in generated styles that did not previously existec-typeattributed added topreelementinline code- AchievedFunctional Implementation
Four (4) different approaches were considered:
inline.inline<hookname>Inlineset of hooksinlineblocksExpressiveCodeBlock,ExpressiveInlineCodeBlock) and Hook function signatures to differentiate parameters (e.g.,inlinedoes not support gutters soaddGutterElementwouldn't be defined on it's hook properties)supportedCodeBlockTypesplugin option that returns an array of which types of code blocks that the plugin wants its hooks called for with the default beingblockonly ifsupportedCodeBlockTypesis not defined by the plugininlineandcodeblockdifferently within the existing hooksinlineon only a subset of hooks)sideEffects: falsein 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
inlineandblockin terms ofExpressiveCodeBlockhandling. 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.2is written to assume there are onlycode blockswithin.expressive-codeso styles are loosely applied in certain cases. For example, any.ec-line, even if it doesn't live within apreis styled by plugin-frames. Similarly,expressive-codeitself styles any .ec-line that lives anywhere inside of.expressive-code.preand more specificallypre > code? Are there situations that possibly I'm overlooking where anec-linecould exist outside ofpre > codethat would require styling directly under.expressive-code?ExpressiveCodeBlockinstances which may now contain a mixture ofinlineandblockso 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-lineto avoid styling issues wheninline codeis 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.2and this PR highlighted:Single
code blockrendered regardless ofinlineconfiguration:Multiple
code blocksrendered regardless ofinlineconfiguration:The structure above is the same for
inline code(single or multiple) whenrenderis called with onlyinlineblock(s) with the only differences beingspaninstead ofdivand the classes/attributes beinginlinevariants.Single
inline coderendered withinlineenabledMultiple
inline coderendered withinlineenabledWhen 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
ExpressiveCodeBlockin its own container within the outer.expressive-codewrapper. Doing so would provide the following benefits:ExpressCodeBlocksare rendered and what theirExpressCodeBlockTypeisinlineandblockby targeting classes and/or data attributes instead of tag namesblockandinlinecould use the same classes (e.g.,ec-lineinstead 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
peerDependenciesas^0.#.#(e.g., color chips). Unfortunately, it does not appear thatpeerDependenciessemver rules work the same asdependencies/devDependencieswhen 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-lineandec-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 toinlineblocks. One thing to consider is making this change and releasing asv1.0.0which is something I think is a goal anyway. This would allow for users to be warned on upgrading EC if the plugin doesn't supportv1yet. One exception to this is twoslash which defines peer deps as>=0.40.0.Suggested HTML structure:
Multiple
code blocksMultiple
inline codeMultiple
inline code&code blockResolves #250