satyr / coco

Unfancy CoffeeScript
http://satyr.github.com/coco/
MIT License
497 stars 48 forks source link

Ignore trailing spaces in strings #176

Open satyr opened 12 years ago

satyr commented 12 years ago

To ensure that invisible code has no effect.

(From #175)

ksdlck commented 12 years ago

It's worth a clarification that this really shouldn't apply to single-quoted strings, e.g. "There's a space at the end. "; the clarity is already pretty well there, and, moreover, one can't use a backslash at the end of the string to indicate it should not be trimmed, as it would escape the final quote. For heredocs, I think it is sufficient to fix #175, which simply presents an undesirable and unexpected behavior, but not require trailing backslashes. If these may already be used, then they can be used at the discretion of the individual author for the sake of clarity. That said, an underlying issue remains: the ambiguity in indentation as the result of allowing heredocs to drop indentation levels. Why do we specialcase this? The ambiguity is easily removed by forcing heredocs to keep the indentation level on which they appear, and making improperly indented heredocs a syntax error like everything else. I am, of course, assuming that this would not unnecessarily impinge on the way they are being used, which it might. Another reasonable behavior, which is a bit closer to the current (minus #175) could be to set the indentation level based on the first visible (in the code, not in its display) character--which could be an escaped space--in the heredoc, if such a one appears before the current expected indentation level, otherwise using the current indentation level, and trimming the spaces appearing before this level as usual.

An essential corollary to whatever behavior is chosen would be to have vim-coco highlight strings (or at least heredocs) with a background color to make the presence of spaces--not just those on empty lines--evident. Clearly the behavior of this highlighting must match exactly the behavior of coco itself; only the parts of the text which contain data that will end up in the resulting string should be highlighted, not the entire span of the heredoc delimiters. It is my strong opinion that, rather than having spaces automatically trimmed--building in an invisible behavior to solve an invisible problem--everyone would be musch better served by making the invisible problem a visible one. Surely a similar solution to that proposed for vim is also applicable to most editors people are using.

satyr commented 12 years ago

Note that we're already trimming leading spaces. This proposal seems natural extension to that.

shouldn't apply to single-quoted strings, e.g. "There's a space at the end. "

I believe you mean single-line strings. Obviously this issue don't apply to them as there can't be trailing spaces within them.

the ambiguity in indentation as the result of allowing heredocs to drop indentation levels

Which ambiguity in particular? Code examples?

ksdlck commented 12 years ago

I do indeed mean single-line strings; seems Coco's behavior with multi-line single-quoted string ignores lines and spaces entirely. However, the clarification was more to address the "Ignore trailing spaces in strings" as the title, which is a bit too broad.

The ambiguity is not one of grammar but rather what the indentation level of the lines of the heredoc should be considered to be for the purposes of stripping off leading spaces; I suppose there exists some desired behavior already--I've just pulled the latest commits to test against Coco post-#175. Both of the proposals I made are examples of a principled target behavior that is easily understood, and address this issue.

ksdlck commented 12 years ago

So far as I can tell, the behavior still strikes me as a bit unusual. Consider this testcase: https://gist.github.com/3876617

The invisible line with three spaces doesn't get anything trimmed, but the line with one space indent and some printable characters gets its one space trimmed. For comparison, the first behavior I suggested would call this a syntax error because the line with "there" is improperly indented, and the second behavior would calculate the indentation level to be 1 space, which is lower than the current indentation level (2 spaces) and the lowest indentation level of a non-space and non-escaped-space character.

satyr commented 12 years ago

I suppose there exists some desired behavior already

The trimming process, derived from CoffeeScript, goes like:

  1. If the last (but not first) line is space-only, trim it.
  2. Calculate the lowest indent level among remaining lines. The first line and empty lines are ignored for this.
  3. Trim the leading spaces by the calculated amount.
  4. Trim the first character if newline.

The special treatment for the first and last lines ensures ''' foo ''' be equal to ' foo '. We don't count an empty line as having zero level indent as per jashkenas/coffee-script#704.

The invisible line with three spaces doesn't get anything trimmed

It's with four spaces, gets trimmed one.

the first behavior I suggested would call this a syntax error because the line with "there" is improperly indented

I don't see how that's improper. The "there" line is visually the determining line for the lowest indent level.

ksdlck commented 12 years ago

Good catch regarding the spaces. This leaves me thinking the current behavior is pretty similar to the one I described. The improper part is merely a matter of opinion; heredoc's violate the rules of indentation followed by other components of the syntax. It's not ambiguous, and it works, so maybe it's fine. I have noticed that I can already force a space to be non-indent material by preceding it with a backslash, which I like very much.