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

deltaposition anchor inconsistency #62

Open
lspitzner opened this issue Mar 11, 2018 · 3 comments
Open

deltaposition anchor inconsistency #62

lspitzner opened this issue Mar 11, 2018 · 3 comments

Comments

@lspitzner
Copy link
Contributor

lspitzner commented Mar 11, 2018

Considering the elements of annsDP, and the anchor of DPs for the contained elements:

  • for comments (AnnComment _, _someDP) the DP is relative to the "current cursor position", i.e. relative to the end of the last thing inserted when pretty-printing;
  • for "normal keyword"s (G _, _someDP) the DP is relative to the node's position (the "cursor position" before pretty-printing anything for the relevant node.

Consider this example:

list = [ some laaaaaaaaaaaaarge expresssion -- this comment has DP (0,1)

       , another thing -- comma at start of this line has DP relative
                       -- to the position before "[", i.e. DP (2,7)
       ]

-- might be re-formatted like this:

list = [ some
           laaaaaaaaaaaaarge
           expresssion -- still at DP (0,1)

       , another thing -- now would be DP (4,7)
       ]

That is, node-relative DPs change when using a different layout for some random child node. If at all possible, an encoding that does not change for such a re-format should be used instead. This would make it much easier for brittany to respect/retain such additional newlines.

(And imho the status quo is just inconsistent and surprising, and it requires additional statefulness when processing annotations.)

@alanz
Copy link
Owner

alanz commented Mar 12, 2018

The model is used because the AST elements follow a definite structure, and must be able to nest, indent, etc.

The comments can appear anywhere, and simply take up what would otherwise be whitespace.

There is a substantial amount of complexity/subtlety to get the thing to actually work, so I am not confident that the requested change can be done easily.

I would welcome a PR to do it, so long as all the tests still pass with it.

@lspitzner
Copy link
Contributor Author

I don't see how my encoding would not be able to nest, indent, etc. But you certainly have a better picture of the many special cases around this stuff.

But regardless, does the current model not break for refactoring operation such as, lets say, "transform a:[b,c] into [a,b,c]"? Because the DP of the comma between b and c would become unusable? It seems to me that the current design runs into problems sooner or later not only for re-formatting but also refactoring uses.

But I can completely understand that redesigning this would be an enormous amount of work. So I'd like to ask a different question: Would you be opposed to adding redundant information to the exactprint/annotation API, where this additional data is not used for exactprinting but available for downstreams like brittany? The "DP relative to the last non-whitespace character" would be such data that would enable brittany to support retaining empty lines inside literals (tuples, lists, etc.)

Of course that usecase is not terribly important, but I have encountered situations where such feature would be nice-to-have. Either way, thanks for considering.

@rwe
Copy link

rwe commented May 9, 2019

I wrestled with this today too. All I wanted was to get apply-refact to actually delete unused language pragmas instead of leaving blank lines…but the annotation offsets are inscrutable and unpredictable. For example, given this:

{-# LANGUAGE Foo #-}

{-# LANGUAGE Bar #}

The DP of both of those is (0, 0), for some reason. But if you add an identifier:

{-# LANGUAGE Foo #-}

{-# LANGUAGE Bar #}
foo = 12

Then suddenly the DP of the second pragma has (2, 0). I can't tell if this is a bug in ghc-exactprint, or if it just has surprising semantics. But in either case, I'm not sure how use it to actually transform documents.

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

No branches or pull requests

3 participants