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

[docs] Rescue intentional line breaks #4763

Merged
merged 12 commits into from
Mar 19, 2023

Conversation

mattstein
Copy link
Collaborator

@mattstein mattstein commented Mar 17, 2023

The Issue

.editorconfig improvements in #4712 strip trailing spaces, which in Markdown removes two-space line endings ( ) than translate into <br> elements.

How This PR Solves The Issue

This removes the problematic trim_trailing_whitespace = true setting, re-adds two-space line endings that have since been stripped out of Markdown files, and fixes some other mangled spacing I noticed along the way.

The result allows Markdown to continue operating like someone would expect, without needing to resort to using <br> elements all over the place or unwittingly having their breaks stripped on save.

Note
After further discussion, this PR changed from originally inserting <br> elements to reverting trim_trailing_whitespace = true.

Manual Testing Instructions

Preview changes locally or inspect the automatic build.

Automated Testing Overview

n/a

Related Issue Link(s)

Release/Deployment Notes

n/a

# Conflicts:
#	docs/content/developers/writing-style-guide.md
#	docs/content/users/debugging-profiling/step-debugging.md
#	docs/content/users/install/ddev-installation.md
#	docs/content/users/install/shell-completion.md
#	docs/content/users/topics/hosting.md
#	docs/content/users/usage/architecture.md
#	docs/content/users/usage/cms-settings.md
#	docs/content/users/usage/database-management.md
#	docs/content/users/usage/developer-tools.md
#	docs/content/users/usage/troubleshooting.md
@mattstein mattstein self-assigned this Mar 17, 2023
@mattstein mattstein requested a review from rfay March 17, 2023 21:12
@rfay
Copy link
Member

rfay commented Mar 17, 2023

Sorry if I'm being dense, but if trim_trailing_whitespace makes us dance around with <br> everywhere, and you have to know magic knowledge to accomplish that, then let's remove the trim_trailing_whitespace instead.

Normal markdown treats double linefeeds with respect. We'd rather have "normal" markdown.

Am I lost still?

@mattstein
Copy link
Collaborator Author

I brought this up here, reiterated again why trim_trailing_whitespace is useful here, and you suggested a PR to fix the Markdown and keep trim_trailing_whitespace here.

This is that PR.

The trim_trailing_whitespace setting is still enabled and those line breaks are stripped out. I don’t care what happens with the setting, I’m just trying to save the line breaks I added on purpose and I don’t want to keep circling the issue of what we do with that setting.

If you want me to drop trim_trailing_whitespace and turn those <br>’s back into double spaces again I can do that instead.

@rfay
Copy link
Member

rfay commented Mar 17, 2023

I'm the one that thinks trailing whitespaces do no harm. I apologize about this confusion, but I never thought this was a problem that needed solving. I guess we all think we're not being listened to. Do you think the trailing spaces is even a problem that needs solving? Does anybody but @gilbertsoft think it's a problem that needs solving?

@mattstein
Copy link
Collaborator Author

Do you think the trailing spaces is even a problem that needs solving?

Not in Markdown, no. But I can’t speak for the whole codebase or @gilbertsoft’s intentions—just defending my precious line breaks.

@mattstein
Copy link
Collaborator Author

@rfay Updated my approach here along with the description. Ready to review again.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we feel the pain receding?

@rfay rfay merged commit ae53957 into ddev:master Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants