Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable markdown indented code blocks by default #2438

Closed
zachleat opened this issue Jun 14, 2022 · 19 comments
Closed

Disable markdown indented code blocks by default #2438

zachleat opened this issue Jun 14, 2022 · 19 comments
Labels
breaking-change This will have to be included with a major version as it breaks backwards compatibility. enhancement

Comments

@zachleat
Copy link
Member

zachleat commented Jun 14, 2022

In markdown there is a specific feature called Indented Code Blocks that causes much confusion! We have a big warning on the docs about it.

https://www.11ty.dev/docs/languages/markdown/#there-are-extra-and-in-my-output

Awhile back @drewm posted a lovely workaround to opt-out of this feature.

eleventyConfig.setLibrary("md", markdownIt(options).disable('code'));

https://twitter.com/drewm/status/1167821259662663682

I’d like to change the default Eleventy behavior to do this as well. Maybe even in 2.0 👀

Related: #402 #180 #1971 (#1635 maybe) and part of https://twitter.com/brob/status/1530620544680337412 from @brob and probably a bunch of others

@zachleat zachleat added needs-discussion Please leave your opinion! This request is open for feedback from devs. breaking-change This will have to be included with a major version as it breaks backwards compatibility. labels Jun 14, 2022
@zachleat zachleat added this to the Eleventy 2.0.0 milestone Jun 14, 2022
@zachleat
Copy link
Member Author

zachleat commented Jun 14, 2022

This would, of course, allow folks to re-enable the feature by using markdown-it’s enable instead of disable:

// Not this
// eleventyConfig.setLibrary("md", markdownIt(options).enable('code'));

// Opt-in via new 2.0 amendLibrary Configuration API method.
eleventyConfig.amendLibrary("md", mdLib => mdLib.enable('code'));

@jina
Copy link

jina commented Jun 14, 2022

YESSSSSS. This is such a pain point.

@pdehaan
Copy link
Contributor

pdehaan commented Jun 14, 2022

But I think the problem is that now I need to figure out what markdownIt() is, and which default settings Eleventy uses vs markdown-it defaults (which we outline on https://www.11ty.dev/docs/languages/markdown/#optional-set-your-own-library-instance, if you know where to look for it; spoiler: html: true (markdown-it default is false)).

Does it make more sense to add it as some new config flag?

eleventyConfig.markdownCodeIndent(false);

Liquid has an .setLiquidOptions() API, but I don't think that'd help us here (although it might make it easier to set default markdown-it options without needing to create a custom library instance just to set one option). And something like .setMarkdownOptions({code: false}); to disable code blocks (or ideally code blocks disabled by default and reenabled via {code: true}) might be too obscure and crossing streams.

@brob
Copy link

brob commented Jun 15, 2022

I prefer using my own code block syntax (liquid or nunjucks tag) and have been bitten by this a number of times.

Totally in agreement on this.

Additionally, I feel like any time something behaves unexpectedly is also a time for new users (and often newer devs) to get very confused. When magic happens, it should be more explicit.

I'd be curious to know how many folks actively use tab-based code blocks. To me, that's a relatively unintuitive way of handling it on markdown's part (but that ship has sailed)

@solution-loisir
Copy link

I think this would be a very sensible default since I feel that most users don't even know about the feature and it creates a gotcha. Personally, I've been desabling the code feature in every projects.

@AleksandrHovhannisyan
Copy link
Contributor

Just to clarify, this doesn't break code blocks or inline code, right? I tried it out and it seems to work fine, and I'm also 100% in support of this since it's bitten me way too many times. I do agree that it's probably better to stick it behind an option, though, and maybe set it to true by default while allowing users to opt out if they need to.

@zachleat
Copy link
Member Author

@AleksandrHovhannisyan correct, the traditional method (triple backtick) for code blocks and inline code (single backtick) in Markdown both still work when .disable("code") is in play.

Enabling or disabling the triple backtick feature is referenced as fence in markdown-it and the single backtick feature is backticks

https://github.com/markdown-it/markdown-it#manage-rules

@zachleat
Copy link
Member Author

Per your comment @pdehaan I think this might be a good option if the feature had some modicum of popularity 😅. From the feedback here so far, it seems universally hated 😳 (and to be fair, same from me) #2438 (comment)

@MattWilcox
Copy link

Agreed, the "indent for code" was a mistake in the design of Markdown IMO. Get rid! :)

AleksandrHovhannisyan added a commit to AleksandrHovhannisyan/aleksandrhovhannisyan.com that referenced this issue Jun 15, 2022
@tabatkins
Copy link

Yes please. Omitting indented code blocks is the major divergence that Bikeshed-flavored Markdown makes from CommonMark as well; it's a terrible misfeature on it's own and is even worse when combined with markup.

@BenDMyers
Copy link

Huge support for this.

@nachtfunke
Copy link

Yes, yes, 100% yes. Possum paws up for this. This is such an unknown markdown feature, that the unintended problems it causes are rarely attributed to it. I think this is a wonderful quality of life improvement.

@zachleat
Copy link
Member Author

Whew, time to issue a mercy rule on this one. It’ll ship with 2.0.0-canary.12

@zachleat
Copy link
Member Author

zachleat commented Jun 16, 2022

Opt-ing back into this feature will be a little trickier than I suggested earlier, I went pretty hard on this default based on how the existing Markdown documentation looks with setLibrary and Markdown plugins.

Put plainly, this calls .disable("code") on both the default library instance and any instance set via setLibrary too.

So, to opt-back-in I added a new (multi-template-language) configuration API method called amendLibrary that allows you to run your own callbacks on both default and custom library instances.

Here’s what that looks like for this feature specifically:

// Opt-back-in via new 2.0 amendLibrary Configuration API method.
eleventyConfig.amendLibrary("md", mdLib => mdLib.enable('code'));

But this will also simplify markdown-it plugins too, so you don’t have to know anything about the existing markdown-it options or passing those in (credit to @pdehaan above for that callout)

const mdEmoji = require("markdown-it-emoji");

module.exports = function(eleventyConfig) {
  eleventyConfig.amendLibrary("md", mdLib => mdLib.use(mdEmoji));
};

zachleat added a commit that referenced this issue Jun 16, 2022
@AleksandrHovhannisyan
Copy link
Contributor

Woo-hoo! 🥳 This has bitten me way too many times.

Should the pitfall docs also be updated to mention that outdent is no longer needed as of 2.0.0-canary.12? (Or maybe that should happen when 2.0 goes live.)

@zachleat
Copy link
Member Author

Yeah @AleksandrHovhannisyan I’m updating the docs as we speak/type 🙌🏻

zachleat added a commit to 11ty/11ty-website that referenced this issue Jun 16, 2022
@zachleat zachleat added enhancement and removed needs-discussion Please leave your opinion! This request is open for feedback from devs. labels Jun 16, 2022
@Moto42
Copy link

Moto42 commented Oct 30, 2022

This just bit me again, thanks for the reminder of what the problem is.

@zeroby0
Copy link

zeroby0 commented Jan 19, 2023

I LOVE this! I was running my own fork of MarkdownIt because of this and I'm very glad Eleventy 2.0 has a better way to disable it.

nhoizey added a commit to nhoizey/nicolas-hoizey.photo that referenced this issue Jan 20, 2023
@tcurdt
Copy link

tcurdt commented May 20, 2023

Urgh. I just got bitten by that change.

Totally fine to change the default, but not so great when one passes in a configured parser via setLibrary.

Gladly the workaround is well documented (thanks!) but

config.amendLibrary("md", mdLib => mdLib.enable("code"))

feels ugly as hell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will have to be included with a major version as it breaks backwards compatibility. enhancement
Projects
None yet
Development

No branches or pull requests