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

Line Separator U+2028 in comment fails to format entire file #901

Closed
avioli opened this issue Mar 13, 2020 · 3 comments · Fixed by #944 · May be fixed by #908
Closed

Line Separator U+2028 in comment fails to format entire file #901

avioli opened this issue Mar 13, 2020 · 3 comments · Fixed by #944 · May be fixed by #908
Labels

Comments

@avioli
Copy link

avioli commented Mar 13, 2020

Got a dart file with a Line Separator character U+2028 (hex: E2 80 A8) in a comment (copy and pasted extra text from a web page).

dartfmt input.dart fails with:

The formatter produced unexpected output. Input was:
... source file
Which formatted to:
... correctly formatted file with removed Line Separator
Please report at github.com/dart-lang/dart_style/issues.

flutter format input.dart fails with the same as dartfmt above, but with the added:

Formatting failed: 65

It would be nice if it points which line/character the unexpected output starts/ends.

munificent added a commit that referenced this issue Mar 20, 2020
@munificent
Copy link
Member

What would expect this to do? Should it remove the line separator, or should it preserve it and then just not fail?

@avioli
Copy link
Author

avioli commented Mar 20, 2020

I think the formatter should not remove anything when it fails, but report the line and character position, so the developer can remove it themselves.

If the guard is specifically about a Line Separator character in comments (which I consider an invisible/hidden character) then I would expect it being removed without failure.

Edit: I re-read what I just wrote and it may sound like a contradiction, but what I meant is that I expect the LS in comments to be removed in my case, while for any other failure I expect, if possible, a line/character position of the fail.

@munificent
Copy link
Member

what I meant is that I expect the LS in comments to be removed in my case

That sounds good to me too. The failure mode it's getting into now is that it does remove it, but then the sanity check it runs to make sure it did not make any incorrect changes does not expect to see that character removed, so complains. I'll fix that so that it allows the line separator to be stripped.

@munificent munificent added bug and removed needs info labels Mar 24, 2020
munificent added a commit that referenced this issue Aug 15, 2020
* Don't crash when trailing non-ASCII whitespace is trimmed.
* Add tests for trailing whitespace trimming.
* Add tests for other Unicode characters.

Fix #901.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants