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

comment DeltaPosition indentation logic #53

Open
lspitzner opened this issue Jun 4, 2017 · 10 comments
Open

comment DeltaPosition indentation logic #53

lspitzner opened this issue Jun 4, 2017 · 10 comments

Comments

@lspitzner
Copy link
Contributor

Sorry, but I will nag once more about the logic behind DeltaPositions/addEntryDelta, and under what circumstances the "layouting context" (in relation to which positions are expressed) changes. Consider:

func =
  [ foo x
  | y <- z
    -- the DP of this comment is (1, 0). i would expect (1, 4).
   -- the DP of this comment is (1, -1). i would expect (1, 3).
  , x <- y
  ]

From the pure haskell standard perspective, there is only one item in this code affected by the layouting rule: the func binding. This means that the "layouting context" for all the above code is column 0, and thus the (entry)deltas should be relative to column 0. Current behaviour is different - some part of the list/monad comprehensions adds an artificial context. This means that I did not fully describe the current behaviour in #48.

I'd really appreciate if the documentation could be completed, perhaps by those that actually implemented the behaviour. Most important would be a complete list of the cases where artificial contexts are inserted.

(the above code is not printed correctly via brittany at the moment, as a consequence.)

@alanz
Copy link
Owner

alanz commented Jun 4, 2017

A comment is attached to the preceding AST item. In this case it is y <- z, as the | is not part of it, being attached to the list comprehension as a whole. From that perspective the two delta's make sense.

The current principle in ghc-exactprint is to attach comments as close as possible to a given syntax item, so that if it is ever moved, or the AST fragment used elsewhere, or deleted, the comment stays attached to it.

@lspitzner
Copy link
Contributor Author

lspitzner commented Jun 7, 2017

What about the DPs in the following testcase?

-- let's start indented, for fun
 func =
  -- DP == (1, 1) mercury
    [
        -- DP == (1, 7) venus 
      foo x
         -- DP == (1, 8) earth
     |   y <- z
       -- DP == (1, -2) pluto
      -- DP == (1, -3) saturn
     ,    x <- y
           -- DP == (1, 2) jupiter
     ,      pred x
  -- DP == (1, 1) nepturn
      ]

@lspitzner
Copy link
Contributor Author

the content of the comments refers to the DP of the comment itself there.

@alanz
Copy link
Owner

alanz commented Jun 7, 2017

The important point is what item the comment is attached to. Because the DP is relative to that.

@lspitzner
Copy link
Contributor Author

just to explore alternatives, here are some ideas how DPs for that case might look like:

--                       status   standard   previous
--                       quo      layouting  /associated
--                                rule only  ast item
 func =
  -- mercury             (1, 1)   (1,1)      (1,1)
    [
        -- venus         (1, 7)   (1, 7)     (1, 7) or (1, 2) if connected to "foo x"
      foo x
         -- earth        (1, 8)   (1, 8)     (1, 3)
     |   y <- z
       -- pluto          (1, -2)  (1, 6)     (1, -2)
      -- saturn          (1, -3)  (1, 5)     (1, -3)
     ,    x <- y
           -- jupiter    (1, 2)   (1, 10)    (1, 1)
     ,      pred x
  -- nepturn             (1, 1)   (1, 1)     (1, -10)
      ]

@alanz
Copy link
Owner

alanz commented Jun 7, 2017

And now that I actually look at the code, I see that comments are put in annPriorComments in the delta phase.

So -- venus is attached to foo x, BUT, the DP is relative to the last output of the current context, and is processed before entering the span for foo x. In this case the current context is the list comprehension as a whole.

It's pretty confusing.

@alanz
Copy link
Owner

alanz commented Jun 7, 2017

@lspitzner I want to experiment with this a bit, what is the end result you are trying to achieve for this example in brittany? How should it end up formatted?

@lspitzner
Copy link
Contributor Author

Ah, so many slightly different questions/issues.

  1. Independent of how the monadcomp case is fixed, I fear that more such cases might be hiding somewhere, which is why i asked for fixing the docs;

  2. So -- venus is attached to foo x, BUT, the DP is relative to the last output of the current context, and is processed before entering the span for foo x. In this case the current context is the list comprehension as a whole.

    brittany already expects this (including the case where the attached-to-part defines a new layouting-rule-indentation-level - you have to insert pre-comments relative to old indentation-context, and post-comments relative to new-..).

    But I agree with the "confusing" part, or even more: Consider the following, relatively well-formed input to brittany:

    foo = [  -- some long explanation
             -- for the complex expression below
             r
             -- foo
          |  y <- z
             -- some more docs
          ,  x <- y
          ]

    Brittany messes this up a bit atm, but let us assume that I somehow adapt only brittany, and the ghc-exactprint behaviour remains as-is. Then the output would look lile:

    foo =
      [  -- some long explanation
             -- for the complex expression below
        r
             -- foo
      | y <- z
        -- some more docs
      , x <- y
      ]

    which is bad.

  3. Just looking at the current behaviour, the comments after "y <- z" have indentations relative to that node: apparently the elements of a monad-comprehension are treated like the elements of do-notation, where really the layouting rule only applies in the latter case.

    Normally I could just say "oh, another exception to the layouting rule" and insert an artificial indentation block in brittany too, but here there is one complication: The commas in between elements of the monadcomp have negative DPs (and negative indentation, in brittany's internals). That is a something I had not really planned for, so adapting brittany would be some work in this case.

    This is perhaps irrelevant because the current logic does not give desired results anyways, but provides a criterion for solutions (how likely/where will there be negative indents in DPs).

  4. How should it end up formatted?

    For the planet-comment-example.. I have no idea; the input is a bit too messy to derive a good approach. For the above example that is relatively well-formed, I think I'd really want as output

    foo =
      [ -- some long explanation
        -- for the complex expression below
        r
        -- foo
      | y <- z
        -- some more docs
      , x <- y
      ]

    But that would certainly require significant changes to the DP semantics - you don't even arrive at this if you manage to express all DPs after the "[" relative to the "[" (which would match the do-notation behaviour).

@alanz
Copy link
Owner

alanz commented Jun 8, 2017

I will look at this in detail later when I have time. In the meantime, it struck me that adjacent single-line comments should be treated as a single entity in ghc-exactprint.

So

    -- some long explanation
    -- for the complex expression below

would be formatted as a unit.

@lspitzner
Copy link
Contributor Author

I agree that adjacent single-line comments should retain their alignment, but I am not sure if focusing the implementation on this is the right approach. E.g. if both lines above were independently connected to the following expression r, you'd already get this behaviour without ever having to recognise/acknowledge that they form a block.

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

2 participants