Preserve trailing whitespace for Markdown, HTML & MJML (RedwoodJS project + create-redwood-app template)

[Philzen]

While trailing whitespace is just bit noise in code (well, 99 % of the time...), for markdown it has actual signifiance.

This used to drive me crazy for a while until i found this setting. This will save others the trouble.

---

There was some scope creep in the discussion below, so this PR now both introduces changes and also fixes intended behaviour, namely:

• (Fix) Bring VSCode settings in line with .editorconfig in the RedwoodJS main project
  • this is particularly nice b/c trailing whitespace is now preserved when editing the docs
• (change) Not only preserve whitespace for markdown, but also HTML & MJML files in the project template

[netlify[bot]]

✅ Deploy Preview for redwoodjs-docs canceled.

Name	Link
🔨 Latest commit	33bec167890d73b66832632b364a3815c133817f
🔍 Latest deploy log	https://app.netlify.com/sites/redwoodjs-docs/deploys/62f100411a37180008f39613

[dthyresson]

For apps, this setting is code via editorConfig rather than VSCode:

https://github.com/redwoodjs/redwood/blob/13fbf296e3b3c313be7d454b936dfa0b6bd17ba0/packages/create-redwood-app/template/.editorconfig#L10

[dthyresson]

And also in the framework in https://github.com/redwoodjs/redwood/blob/13fbf296e3b3c313be7d454b936dfa0b6bd17ba0/packages/create-redwood-app/template/.editorconfig#L10

Is there a way to ignore markdown files using editorConfig rather than VSCode settings?

[Philzen]

For apps, this setting is code via editorConfig rather than VSCode:

https://github.com/redwoodjs/redwood/blob/13fbf296e3b3c313be7d454b936dfa0b6bd17ba0/packages/create-redwood-app/template/.editorconfig#L10

Thanks @dthyresson good catch! I just checked in a test project ... it's crazy but they are both required (→ fix in 1f37abbb697c1bbda8f0e2a53a7e41e8baa04e26) – it seems otherwise any true value takes precedence over the other.

I urge you to test out yourself, but from my tests it seems there's no way around adding the exclusion to both VSCode and .editorconfig to make it stop removing our precious markdown line-breaks on save.

May i ask if there's any reason changing the .vscode/settings.json should be discouraged?

[Tobbe]

@dthyresson, @Philzen had this question for you, can you take a look please?

May i ask if there's any reason changing the .vscode/settings.json should be discouraged?

[dthyresson]

May i ask if there's any reason changing the .vscode/settings.json should be discouraged?

For me, it is because EditorConfig supports multiple editors and doesn't assume devs will use VSCode.

See:

These editors come bundled with native support for EditorConfig. Everything should just work.

Therefore, it is a more general solution.

For example, if I edit the markdown file within GitHub, it will respect the rules set in the configuration.

[Philzen]

@dthyresson Totally with you on that one.

Is there a way to ignore markdown files using editorConfig rather than VSCode settings?

I misunderstood that changes to .vscode/settings.json were discouraged in general, which is not the case (and actually required for this one). We need to treat .editorconfig as the first-class cititzen, absolutely.

[dthyresson]

Question, should these editorConfig's be added to

• framework root editorConfig (ie, handle markdown files in framework)
• the /docs editorConfig for Docusaurus since these are all markdown file, too ?

Not just in the project template?

[dthyresson]

Oh @Philzen I just saw that this is in the framework's editorConfig:

[*.{md,html,mjml}]
trim_trailing_whitespace = false

So, maybe this configuration is better for the project template?

[Philzen]

Question, should these editorConfig's be added to

• framework root editorConfig (ie, handle markdown files in framework)
• the /docs editorConfig for Docusaurus since these are all markdown file, too ?

Not just in the project template?

Excellent catch! This adds even more value for docs contributors that work on their fork locally. Updated in 1f35f48ebd17279c4a4ac25e0b4caaa94fd03de2 – now also works like a charm in the docs folder.

[Philzen]

[*.{md,html,mjml}] trim_trailing_whitespace = false

So, maybe this configuration is better for the project template?

I sense intensive scope creep here :monocle_face: :wink: … but so be it, as i understand there seems to be a consensus amongst the core team and also widely across the rest of the community that you want full control over your own whitespace in HTML & MJML as well. I can actually relate to that, i had cases where i wished it would respect my plain content within JSX tags and had to work around the editor's interference by either putting   or prerendering a variable that includes my fully intended whitespace.

Note that this setting was actually never respected by VSCode before … until now: dcd9ba0c2c7a38bb2bdb9e65ad836072e49565e0

bec87ab20ab2f523b54dd423ae6e60dcd6a493ad introduces the same setting consistently for the project template as well.

---

I've updated the issue title & description accordingly so the whole scope of this PR is transparent in the commit history once merged.

[Tobbe]

I've updated the issue title & description accordingly so the whole scope of this PR is transparent in the commit history once merged.

🌟

[jtoar]

@dthyresson thanks for moving this one along! As this was opened before the bot started assigning people to PRs, I just 1) assigned you to it and 2) put it into the current cycle cause it feels like it's out of triage, but feel free to undo if that's not what you had in mind!

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 33bec167890d73b66832632b364a3815c133817f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

---

✅ Successfully ran 14 targets

• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run build
• cli run build
• cli run build
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run test --concurrency 2 -- --colors --maxWorkers 2
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build
• cli run build

---

Sent with 💌 from NxCloud.

[Tobbe]

To me it sounds like all your comments were addressed @dthyresson. Anything preventing us from merging this? (Except for failing tests, but those seems unrelated - I've triggered a new round of tests now)

[Philzen]

@dthyresson @jtoar @Tobbe seems nobody is assigned to this anymore – just a little bump so it's not forgotten :smirk:

@Tobbe wrote:

To me it sounds like all your comments were addressed @dthyresson. Anything preventing us from merging this? (Except for failing tests, but those seems unrelated - I've triggered a new round of tests now)

[dthyresson]

Having looked at the changes, I realized this change needs documentation to explain why developers would see there behavior of whitespace not being trimmed in these cases.

But, if we have to document why this is happening and how to revert the behavior, I propose instead to simply add documentation to say "How to configure trailing whitespace in your RedwoodJS project".

And then developers can add these settings themselves.

Thoughts?

[Philzen]

I do support the notion of adding some info regarding this (i guess https://redwoodjs.com/docs/project-configuration-dev-test-build would be a good place) – after all, working with Redwood and its docs taught me many new things, so showing how .editorconfig and ESLint/prettier overshadow each other and how to make them behave sounds like a worthwhile addition :+1:

But i strongly think the settings suggested in this PR should be the default and not vice-versa for the following reasons:

• As stated in the initial description, the current setting interrupted my work and thus sidetracked me for a consiberable amount of time until i was able to find a fix, time in which i could not make progress on actual implementations. As Redwood is an opinionated project, if feel it should strive for the best possible DX and avoid people bumping into such an issue being caused by its default settings

• whitespaces in Markdown do have significance, so imho the current setting makes files invalid on save

• basically this PR fixes an existing inconsistency b/c the .editorconfig in the root folder already has that setting (for good reasons) – it is only overruled b/c ESLint/VSCode settings are not harmonized with it:
  https://github.com/redwoodjs/redwood/blob/25138132f7a7b9f8a02bcd5898ee6fd89f3b75d1/.editorconfig#L17-L18

(BTW i've used trailing whitespace to format this markdown comment – something that Github respects but the current Redwood settings sabotage :wink: )

[Tobbe]

I do support the notion of adding some info regarding this (i guess redwoodjs.com/docs/project-configuration-dev-test-build would be a good place)

Yes, that would be the place. We actually discussed adding more info there about how to configure your RW App DX settings. Like if you wanted to further configure (or turn off) import order formatting. And I think there was one more thing, but I can't remember right now. @jtoar Do you remember?

[Philzen]

I remember: https://github.com/redwoodjs/redwood/pull/5868#issuecomment-1198241939 (have it on my TODO list)