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 break-on-newline option #130

Closed
wants to merge 3 commits into from
Closed

Conversation

cdman
Copy link
Contributor

@cdman cdman commented Sep 23, 2013

This changeset adds the option for "break-on-newline" which adds a
tag on each newline inside a paragraph. This makes the library behave more like GitHubs markdown.

This changeset adds the option for "break-on-newline" which adds a <br /> tag on each newline inside a paragraph. This makes the library behave more like [GitHubs markdown](http://github.github.com/github-flavored-markdown/).
@mcepl
Copy link

mcepl commented Oct 3, 2013

I would very much against this PR. Many people want to use Markdown for writing in their $EDITOR and normal standard there is to have s put in the paragraphs to have nicer looking text in their editor. I agree with many that significant trailing whitespace is pure Evil (which is what Markdown currently uses), and some extension to the original Markdown should be created (hopefully in cooperation with Markdown implementors), but what GFM done is IMHO not The RIght Way™.

Moreover, this project has been very much (AFAIUI) about doing the “standard” compliant implementation. Well, this isn’t.

@cdman
Copy link
Contributor Author

cdman commented Oct 3, 2013

This is an option (which isn't enabled by default) and you can enable it if you find it advantageous. For example if your users are also GitHub users, they might expect this way of working. I also simplifies the case when the user doesn't know what markdown is and pastes a chunk of code into a (markdown) editor - having this option ensures that at least the linebreaks are preserved as opposed to everything being put on one line.

if "break-on-newline" in self.extras:
text = re.sub(r" *\n", "<br%s\n" % self.empty_element_suffix, text)
else:
text = re.sub(r" {2,}\n", " <br%s\n" % self.empty_element_suffix, text)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of repeating the entire sub, could we just switch out the pattern? For example:

if "break-on-newline" in self.extras:
    _break_re = r" *\n" 
else:
    _break_re = r" {2,}\n"
text = re.sub(_break_re, " <br%s\n" % self.empty_element_suffix, text)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, simplified it.

@nicholasserra
Copy link
Collaborator

@cdman I cherry picked the initial commit into master. You were correct in your original patch, as the break tags needed the preceding space toggled. Thanks!

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.

3 participants