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 include extra whitespace in compiled text #130

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbdietrich
Copy link

Currently Curly ends up adding extra newlines after components like conditional or context block tags, which can make the resulting markup hard to read.

With this PR the following output:

<section class="knowledge-base">

  <div class="category-tree">

      <section class="category">




            <section class="section">

is shortened to:

<section class="knowledge-base">

  <div class="category-tree">

      <section class="category">

            <section class="section">

Alternatively Curly could strip text output altogether, but that may make the markup unreadable for lack of whitespace.

/cc @dasch

@@ -179,7 +179,9 @@ def compile_component(component)
end

def compile_text(text)
output "buffer.safe_concat(#{text.value.inspect})"
if non_whitespace_text = text.value.presence
output "buffer.safe_concat(#{non_whitespace_text.inspect})"
Copy link
Contributor

Choose a reason for hiding this comment

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

This would remove indentional whitespace as well, right?

{{one}}   {{two}}   {{three}}

There may be a reason for the whitespace, e.g. when sending plain text emails.

@dasch
Copy link
Contributor

dasch commented Jan 8, 2015

I'm not quite sure I agree with the way it's implemented. I agree that for block components, it would make sense to remove the entire line in the output, iff there's only whitespace and the component itself on that line, e.g.

<div>
{{@post}}
  <ul>
  {{*comments}}
    <li>{{body}}</li>
  {{/comments}}
  </ul>
{{/post}}
</div>

... should output

<div>
  <ul>
    <li>one</li>
    <li>two</li>
    <li>three</li>
  </ul>
</div>

I'm just not sure what the best way to do that is. Maybe add tokens for end-of-line in the scanner, and collapse leading and trailing whitespace in the parser?

@jbdietrich
Copy link
Author

Yeah the method proposed in this PR is pretty heavy-handed. I was also unsure what the expectation was for preserving whitespace, so thanks for clarifying that.

I'll take a look into a smarter way to to this in the scanner/parser.

@dasch
Copy link
Contributor

dasch commented Jan 8, 2015

In theory, we could have a stream modifier that post-processes the token stream (which is output by the scanner) by looking for NEWLINE, WHITESPACE, BLOCK COMPONENT START/END, WHITESPACE, NEWLINE and replacing with BLOCK COMPONENT START/END, NEWLINE, but I'm not sure it's worth it.

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