11ty / eleventy

A simpler site generator. Transforms a directory of templates (of varying types) into HTML.
https://www.11ty.dev/
MIT License
16.86k stars 487 forks source link

Disable markdown indented code blocks by default #2438

Closed zachleat closed 2 years ago

zachleat commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

YESSSSSS. This is such a pain point.

pdehaan commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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) https://github.com/11ty/eleventy/issues/2438#issuecomment-1155812945

MattWilcox commented 2 years ago

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

tabatkins commented 2 years ago

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 commented 2 years ago

Huge support for this.

nachtfunke commented 2 years ago

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 commented 2 years ago

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

zachleat commented 2 years ago

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));
};
AleksandrHovhannisyan commented 2 years ago

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 commented 2 years ago

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

Moto42 commented 1 year ago

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

zeroby0 commented 1 year ago

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.

tcurdt commented 1 year ago

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.