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

Don't trim trailing spaces in markdown #522

Merged
merged 1 commit into from
Jan 27, 2021

Conversation

unode
Copy link
Contributor

@unode unode commented Nov 22, 2020

This caused formatting issues when using Theia through Gitpod

More info at:
carpentries-incubator/jekyll-pages-novice#96
gitpod-io/gitpod#896 (comment)

Originally submitted at: carpentries/lesson-example#310

@maxim-belkin
Copy link
Contributor

maxim-belkin commented Dec 9, 2020

Hi @unode. Thanks for the PR!

I don't mind keeping whitespaces but I don't think this is something we should be enforcing. In fact, GitHub lets you hide such changes when reviewing pull requests -- click on the gear icon, select "Hide whitespace changes", and click "Apply and reload":

Screenshots

The option

image

Its effect

image

Also, I think trailing whitespaces have no effect on the produced webpages, so, if other maintainers of this repo decide to merge this, I'd suggest to update or remove the comment.

Note that I can't comment on how things work in GitPod -- it looks very interesting but currently The Carpentries doesn't use / recommend it for lesson development.

@unode
Copy link
Contributor Author

unode commented Dec 10, 2020

Also, I think trailing whitespaces have no effect on the produced webpages, so, if other maintainers of this repo decide to merge this, I'd suggest to update or remove the comment.

I'm not sure if this is a lesser known feature but a double trailing space in markdown is significant and is translated into a hard line break.

This can't be replicated in GitHub because the markdown flavour here renders soft line breaks as <br/>.

However, kramdown does follow the soft/hard break distinction and the rendered result is different (notice the highlighted 2 trailing spaces in the first screenshot):

screenshot_2020-12-10_03-41-58_670170497screenshot_2020-12-10_03-41-31_119080832

@maxim-belkin
Copy link
Contributor

maxim-belkin commented Dec 10, 2020

Oh, interesting! I wasn't aware of this double whitespace feature in kramdown. Thanks for sharing this.

I think we need to discuss this more. Is this feature present in the regular Markdown? If it is, then I'd vote to preserve whitespaces as you suggest. Otherwise, I wonder if trimming whitespaces would be a better choice because even though we use kramdown we might want to be closer to the plain-vanilla Markdown. What do you think? Do you rely on this feature in your lessons?

If we decide to preserve whitespaces, we might want to document/warn lesson developers about it in carpentries/lesson-example.

@unode
Copy link
Contributor Author

unode commented Dec 10, 2020

Is this feature present in the regular Markdown?

Yes.

Do you rely on this feature in your lessons?

I can't say I use this often in lessons but I do use it regularly when editing Markdown.
We also included this syntax in one exercise of the WIP Jekyll lesson.

@maxim-belkin
Copy link
Contributor

Oh, great! Let's update the comment then to clarify that we're talking about 2+ spaces, e.g.:

# preserve trailing whitespace: 2+ trailing spaces are translated to a hard break

And again, I think we should document this in carpentries/lesson-example. Though, because you've got this info in your lesson, we can also point people to your lesson instead "for more information on Markdown, see episode X of this lesson".

Copy link
Contributor

@maxim-belkin maxim-belkin left a comment

Choose a reason for hiding this comment

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

Thanks, @unode! 🥇

@maxim-belkin
Copy link
Contributor

Thanks, Renato. This change makes sense to me (and I learned something new) so I approve it. I do want to give other maintainers time (a day or two) to review the conversation and respond or merge the PR (if they approve of the changes too).

Again, thank you!

@unode
Copy link
Contributor Author

unode commented Dec 10, 2020

I've updated the comment in the commit.

As for the lesson, I've created an issue on the Jekyll lesson to improve the exercise to better highlight this syntax and its pitfalls.

Waiting for others to review sounds good to me. Thanks a lot for the review and happy to share this knowledge 😺

@maxim-belkin maxim-belkin merged commit 740773a into carpentries:gh-pages Jan 27, 2021
@maxim-belkin
Copy link
Contributor

Thanks for your contribution, @unode!

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