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

add option to use soft breaks for markdown rendering #1408

Merged
merged 4 commits into from
Apr 21, 2019

Conversation

tessus
Copy link
Collaborator

@tessus tessus commented Apr 10, 2019

This PR adds an option to settings to use the traditional line-break behavior.

It is marked as Work In Progress because of the following 2 items:

  • I added the option under General because it's a general markdown setting. We can always move it to a different section.
  • I used a separate variable breaks_, because I thought it made the code more readable. I can remove it and use the ternary operation directly. Please advise.

@tessus tessus added enhancement Feature requests and code enhancements mobile All mobile platforms desktop All desktop platforms labels Apr 10, 2019
@tessus
Copy link
Collaborator Author

tessus commented Apr 15, 2019

fixes #1406

@laurent22
Copy link
Owner

I'd prefer all the Markdown rendering options to be under the same section - I think for users it will be easier to find. For now, to keep it simple, let's put it under the "plugins" section (it's not really a plugin but maybe later we can rename the section or arrange things differently).

Also it means we can remove the description from the setting because from the section it will be implied it's about Markdown (as much as possible, it's better to reduce the localisable text we add to the app).

@tessus
Copy link
Collaborator Author

tessus commented Apr 20, 2019

I will put it under the plugins sections. I am on my way out, so I will look into it tomorrow.

@tessus
Copy link
Collaborator Author

tessus commented Apr 21, 2019

@laurent22 I've put it under the section "plugins". While I do understand that you want to minimize localizable text (which makes sense btw), I am afraid that the term "soft breaks" might not be understood easily, therefore I changed the text to Enable traditional markdown line-break behaviour. This makes it definitely clearer. But if you really want it to just read Enable soft breaks, please let me know. (In that case we should add the explanation for the different plugins to the documentation.)

@laurent22
Copy link
Owner

I see what you mean but no matter what text we choose, it's an advanced setting that most users won't need (I only remember one person who ever asked about it), and Enable traditional markdown line-break behaviour won't mean much as most people aren't expert in Markdown and don't know what's a traditional line-break.

So I'd prefer to keep it short and use Enable soft breaks - the one user who cares about it will know what it means :-) and the rest can ignore it. Adding more info to the Readme of course is fine.

@tessus
Copy link
Collaborator Author

tessus commented Apr 21, 2019

Ok, here you go...

@tessus tessus changed the title [WIP] add option to use soft breaks for markdown rendering add option to use soft breaks for markdown rendering Apr 21, 2019
@laurent22
Copy link
Owner

Perfect, thanks @tessus!

@laurent22 laurent22 merged commit 12ebf44 into laurent22:master Apr 21, 2019
@tessus
Copy link
Collaborator Author

tessus commented Apr 21, 2019

You are welcome!

@tessus tessus deleted the settings-softbreaks branch April 22, 2019 03:56
@taw00
Copy link
Contributor

taw00 commented Apr 22, 2019

Woot. The one user that cares about it... really cares about it :)
Seriously though. Anyone that uses a markdown editor for more than short notes, starts to care about this. :) Regardless, thank you for implementing.

@tessus
Copy link
Collaborator Author

tessus commented Apr 22, 2019

@taw00 it also came up on github and I also know people who use the traditional format. So you might not be the only one, but you were the one who was the most vocal about it. ;-)

@Nebucatnetzer
Copy link

And I lived with the illusion that my fantastic issue sparked the development :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop All desktop platforms enhancement Feature requests and code enhancements mobile All mobile platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants