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

Optionally render carriage return as new line #555

Closed
polarathene opened this issue Mar 31, 2017 · 14 comments
Closed

Optionally render carriage return as new line #555

polarathene opened this issue Mar 31, 2017 · 14 comments

Comments

@polarathene
Copy link

Expected behaviour

Markdown should render as below does using the code snippet:

Text line one.
Text line two.
Text line three.

Text line one.
Text line two.  
Text line three.

Actual behaviour

Only the last line appears on a new line due to line two in the snippet having two spaces at the end. A carriage return should optionally(by default?) imply two spaces.

I'm not sure if this will cause breakage in certain desired rendering of markdown, but a settings option to render as Github does without having to add the two spaces would be nice(or perhaps having the enter key add two spaces?

Steps to reproduce

Text line one.
Text line two.  
Text line three.
@pbek
Copy link
Owner

pbek commented Mar 31, 2017

I'm sorry, https://github.com/hoedown/hoedown is used for converting the markdown to html and the markdown spec says that only two spaces trigger a newline.
Changing this would cause irritation for many users...

@pbek pbek closed this as completed Mar 31, 2017
@polarathene
Copy link
Author

@pbek Would there be harm in having enter key add two spaces to satisfy the spec? (As a checkbox setting to enable). Github and a few other Markdown editors seem to differ from that spec.

Had a quick look around, hoedown has not had much dev activity(nothing in 2016?), there has been talk about following the CommonMark spec here. CommonMark seems like the better spec to support, it has some notable devs behind hailing from Github, Reddit and StackExchange among others.

Google has published a parser that conforms to the CommonMark spec in Rust and outputs HTML: pulldown-cmark, Rust has switched from hoedown to this parser for their documentation as discussed here.

The spec also identifies the issue I raised here with:

A conforming parser may render a soft line break in HTML either as a line break or as a space.

A renderer may also provide an option to render soft line breaks as hard line breaks.

pulldown-cmark has an example on their ReadMe of the small snippet to treat soft line breaks as hard line breaks. This was also discussed on forums for CommonMark spec here. Github has also recently switched to using CommonMark, so that might indicate it being a fairly safe move, along with better performance/memory?

Would you be open to switching from hoedown to pulldown-cmark?

@pbek
Copy link
Owner

pbek commented Mar 31, 2017

I think automatically adding two spaces will do more harm than good in many cases where no spaces should be added...

pulldown-cmark is written in Rust, not C/C++...

@polarathene
Copy link
Author

@pbek Rust can be called from C via FFI(foreign function interface), should be possible to use :)

Fair point with automatically adding the spaces, if switching to pulldown-cmark is an option, you should be able to just pass on a boolean setting for treating soft line breaks as hard provided from QOwnNotes settings.

If you need any help on the Rust side I can give it a shot :) not too familiar with C or your codebase.

@pbek
Copy link
Owner

pbek commented Mar 31, 2017

I'm not sure if I want Rust code in a Qt project that should run on all platforms... 😸

@polarathene
Copy link
Author

@pbek Far as I know it should run on all platforms? Rust compiles to Linux/Windows/macOS. I don't know if I'll find the time for a while, but if you'd rather I give it a shot myself and send in a PR for review that's fine? :)

@pbek
Copy link
Owner

pbek commented Mar 31, 2017

Sure you can if it doesn't break any current feature of the preview, doesn't have any external dependencies, works with cmake and qmake and builds and runs on Linux (there are lots of distros), Mac and Windows. ;)

@polarathene
Copy link
Author

works with cmake and qmake and builds

Is it ok to supply binary for each platform or does the rust source need to be compiled with everything else to be built by user? That would require rust compiler to be invoked, not familiar with cmake/qmake, but uhh rustc to compile the code would be an external dependency I guess?

@pbek
Copy link
Owner

pbek commented Mar 31, 2017

supply binary for each platform

sorry, source code only. there are platforms that compile QOwnNotes theirself

rustc to compile the code would be an external dependency I guess?

yes :/

@polarathene
Copy link
Author

polarathene commented Mar 31, 2017

yes :/

If having another compiler is an unacceptable dependency then there isn't a way to include Rust without supplying binaries far as I know :\

There seems to be two CommonMark parsers/renderers in C. I'm unable to assist with these but both seem to supply an option for converting soft line breaks into hard line breaks. cmark, md4c.

@pbek
Copy link
Owner

pbek commented Mar 31, 2017

Thank you for your research, but integrating such libraries is always a bit of a pain. I cannot promise that I take on that endeavor without pressing needs to do so....

@pbek
Copy link
Owner

pbek commented Apr 1, 2017

But if you really want to use an other markdown to html converter very badly you could write a script with the help of notetomarkdownhtmlhook and use your own one.
see:
http://docs.qownnotes.org/en/develop/scripting/README.html#notetomarkdownhtmlhook.

@polarathene
Copy link
Author

@pbek thanks I might give that a try :)

Out of curiousity what is the benefits from using Qt widgets in the preview pane over a webview? With a webview they'd be better styling options as well as fixes for a styling bug(Qt bug with lists from memory), however I'm guessing that'd lose features like note link integration?

@pbek
Copy link
Owner

pbek commented Apr 2, 2017

plenty of reasons: speed, memory consumption, size of the packages (when libraries are included), the webview that was present in qt 5.3 is now deprecated, but qt 5.3 compatibility is still needed for debian...

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

No branches or pull requests

2 participants