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

Refactor preserve eol with trivia #434

Merged
merged 348 commits into from
Oct 11, 2019
Merged

Refactor preserve eol with trivia #434

merged 348 commits into from
Oct 11, 2019

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Apr 21, 2019

Suggestion to tackle #390
In essence, we collect information from the F# tokens that are not present in the AST (comments, newlines, keywords, ...) and we use to information in CodePrinter to restore the original behaviour.

@nojaf
Copy link
Contributor Author

nojaf commented May 1, 2019

@jindraivanek I've encountered something tricky while inserting newlines that have been found.

F.ex

type C () = 
    let rec g x = h x
    and h x = g x

    member x.P = g 3

We print this in CodePrinter with 2 newlines after the and.

However, those newlines are also found in the tokens. So the test fails like:

  Expected string length 75 but was 77. Strings differ at index 54.
  Expected: "... let rec g x = h x\n    and h x = g x\n\n    member x.P = g 3\n"
  But was:  "... let rec g x = h x\n    and h x = g x\n\n\n\n    member x.P = g 3\n"

This all makes sense but how will be ignore found newlines in the trivia when we are already printing them?

@jindraivanek
Copy link
Contributor

@nojaf I think we need to change sepNln to also get node range as parameter and remove NewLine trivia before (maybe we would also need version of sepNln for after node) that node. That way we only print "extra" newlines from trivia.

@nojaf
Copy link
Contributor Author

nojaf commented May 2, 2019

Yeah thought of that as well. However, that could lead to a tricky situation where you have a mix of newlines and comments. Worth investigating though.

jindraivanek and others added 26 commits June 29, 2019 13:21
Fix test: "multiple let in lines, should remove in, block comment"
When merging only split on valid hash directives.
… and add them to tokenize function."

This reverts commit 88ffab0.
…e line above.

Else find the last (smallest) node from the end of the line above.
Skipping trailing newlines from trivia.
@nojaf nojaf marked this pull request as ready for review October 11, 2019 10:14
@nojaf nojaf merged commit cf56bae into master Oct 11, 2019
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.

6 participants