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

resolves #1084 preserve newlines in linewise formatter #1156

Conversation

mojavelinux
Copy link
Contributor

  • allow tag name to be configured using :tag_name option
  • preserve newlines
  • delegate to stream method instead of span method on inner formatter

- allow tag name to be configured using :tag_name option
- preserve newlines
- delegate to stream method instead of span method on inner formatter
@mojavelinux
Copy link
Contributor Author

Note that the newline is inserted before the closing tag for the line. This is done to avoid cluttering up the end of the line so it's clear where the content of the line ends. This is consistent with how Pygments does linewise formatting. It also allows postprocessors to more easily find the end of each line for further processing, such as adding annotations. (The start of the line is already clear because the enclosing tag has a CSS class).

@pyrmont
Copy link
Contributor

pyrmont commented Jun 4, 2019

Thanks!

A note for possibly future me: this fixes my merge of #1085 before it was rebased against #1083 (which caused a test to be fail). In addition to the changes that were part of #1085, the PR updates that test.

@pyrmont pyrmont merged commit 7bd180f into rouge-ruby:master Jun 4, 2019
@jneen
Copy link
Member

jneen commented Jun 5, 2019

Maybe a little late to comment but I am a little concerned about backwards compatibility with this, given that the line may be contained in <pre> and thus result in extra spaces between lines.

@mojavelinux
Copy link
Contributor Author

mojavelinux commented Jun 5, 2019

I can assure you that won't happen. I have tested this extensively when integrating Rouge and Pygments into Asciidoctor. HTML guarantees that the final newline before the closing tag of a block element is insignificant, and browsers honor this.

I'll share the example I shared with Michael to prove this:

Single-line example:

<!DOCTYPE html>
<pre style="background-color: gray"><div>foo
</div></pre>

Multi-line example:

<!DOCTYPE html>
<pre style="background-color: gray"><div>foo
</div><div>bar
</div></pre>

The same is true if you use span as the tag (but then you don't have to force the enclosing element to display as a block).

The key benefit of using the newline + closing tag is that the number of lines stays the same, which is important for line counting either inside of Rouge or in postprocessing. If we drop the newlines, we end up with something fundamentally different and rely on the block styling on the wrapping div to put the newline back.

@pyrmont
Copy link
Contributor

pyrmont commented Jun 5, 2019

@jneen Well, @mojavelinux beat me to it! I was just writing a comment saying I'd spoken with him about the PR to better understand its consequences (I'm very opposed to anything that would be a breaking change, too!) and he explained that it would not have that effect (using the examples above).

@mojavelinux mojavelinux deleted the issue-1084-preserve-newlines-in-linewise-formatter branch August 7, 2021 09:11
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