Improved whitespace collapsing rules

[syranide]

I propose the following stricter and more well-defined rules for collapsing and removing whitespace, and have provided a patch to do it:

1. Any whitespace sequence which CONTAIN A NEWLINE and TOUCHES the start or end of the string is removed completely \t ABC \n → \t ABC
2. Any whitespace sequence which CONTAIN A NEWLINE but DOES NOT TOUCH the start or end of the string is replaced with a single newline ABC \n DEF \n → ABC\nDEF \n

* The start and end of the string relates to <tag>this plain text in the middle here{expr}

Special-cases for whitespace between text literals

Rule 2, variation A: A \n B → A\nB (current)

• Normally renders the same way as a space, but adds a lot of flexibility when intended (i.e, white-space: pre)
• Non-whitespace dependent markdown can be written as-is (using Rule 3B it would be even more well-supported)
• Space can be inserted instead using A{' '}\nB → A B

Rule 2, variation B: A \n B → A B

• Perhaps slightly more intuitive result in final code (but when would this actually matter?)
• Newline can be inserted instead using A{'\n'}\nB → A\nB

Rule 3, variation A: A \t B → A B

• Overall consistent result in final code (minimal whitespace)
• If additional whitespace is wanted, one must now explicit insert it {' \t '}

Rule 3, variation B: A \t B → A \t B (current, so no rule 3)

• The whitespace is probably intentional, and if it isn't, it normally renders the same way as if it was collapsed
• Better support for inline markdown without braces
• Now there's only two rules instead of three, and both are for newlines

There are a few possible variations of these as well, but they very rarely make sense. So overall I feel like the above rules are the most intuitive, consistent and quite flexible.

I've switched to using Rule 3B instead (feel free to object), it doesn't make a difference for normal rendering, but allows almost all types of markdown to be written inline, it doesn't require being explicit like Rule 3A, but it is much more flexible when it comes to special inline text uses (white-space: pre, markdown, etc), with virtually no negatives.

Example:

<div>
  <div>
    Total: <span>2</span><br />
    <span>Count: </span>3<br />
    Amount:<span> </span>0<br />
    <span>20</span>/
    <span>30</span>
  </div>
  <div>
    This {'the'} {'story'}.
    That story.

    Of the snow{' '}
    white space.
  </div>
</div>

What JSXTransformer produces:

In my opinion, a lot of unnecessary and unintentional whitespace is produced simply because the code is arranged into multiple lines for readability. This may sometimes produce unintentional "padding" in the resulting elements. {'A'} {'B'} does also not produce A B as one would expect.

" Total: 2"
"Count: 3"
" Amount:0"
"20/ 30"
" This thestory. That story. Of the snow  white space. "

React.DOM.div(null, 
  React.DOM.div(null, 
    " Total: ", React.DOM.span(null, "2"),React.DOM.br(null ),
    React.DOM.span(null, "Count: " ),"3",React.DOM.br(null ),
    " Amount:",React.DOM.span(null ),"0",React.DOM.br(null ),
    React.DOM.span(null, "20"),"/ ",
    React.DOM.span(null, "30")
  ),
  React.DOM.div(null, 
    " This ", 'the', 'story',". "+
    "That story. "+
    "Of the snow",' ',
    " white space. "
  )
)

What my patch produces:

It produces whitespace according to well-defined and hopefully intuitive rules. It also mirrors the original code structure as closely as possible. Note that \n renders identically to space unless otherwise specified with CSS.

"Total: 2"
"Count: 3"
"Amount: 0"
"20/30"
"This the story.\nThat story.\nOf the snow white space."

React.DOM.div(null, 
  React.DOM.div(null, 
    "Total: ", React.DOM.span(null, "2"),React.DOM.br(null ),
    React.DOM.span(null, "Count: " ),"3",React.DOM.br(null ),
    "Amount:",React.DOM.span(null, " " ),"0",React.DOM.br(null ),
    React.DOM.span(null, "20"),"/",
    React.DOM.span(null, "30")
  ),
  React.DOM.div(null, 
    "This ", 'the'," ", 'story',".\n"+
    "That story.\n"+

    "Of the snow",' ',
    "white space."
  )
)

[jeffmo]

80char line width plz

[syranide]

Redid the example for a much more understandable view of the problem, and my solution to it.

@jeffmo code has been cleaned up slightly and the rules clarified more thoroughly.

[jeffmo]

Would you mind rebasing on top of the following commit so it's a little easier for me to patch this and play with it?

a4f8ad1

[syranide]

@jeffmo Commits rebased (shout if I messed it up)

[jeffmo]

Hmm, looks like something funky happened in your rebase maybe? This file was removed in 0dc011c

[syranide]

@jeffmo Yeah. Reject this one and I'll create a fresh pull request instead?