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

Allow closing parens after indented comment lines #92

Open
SevereOverfl0w opened this issue Feb 8, 2016 · 11 comments
Open

Allow closing parens after indented comment lines #92

SevereOverfl0w opened this issue Feb 8, 2016 · 11 comments
Labels

Comments

@SevereOverfl0w
Copy link

Sometimes comments are within a block for a reason, and keeping them within the block may be useful. For example, in indent mode, moving around

[:foo
 ;; :bar ;; This does bar things
]

becomes

[:foo]
  ;; :bar ;; This does bar things

I think that the second form loses some obviousness in that it's no longer obvious on what the comment is relating to. Should parinfer be updated to include this?

@shaunlebron
Copy link
Collaborator

This is a good question. I've thought about it some, and I just think it is difficult to reconcile.

  • it would break consistency of close-paren placement
  • it would create a case where parinfer must insert a newline
  • comment-reversibility is lost

For example, given we have this:

[:foo
 ;; :bar ;; This does bar things
]

If I comment out the last line, indent-mode has to insert a close-paren somewhere, so it's forced to create a new line to include the indented comment:

[:foo
  ;; :bar ;; This does bar things
  ]
;]

Once you uncomment the last line, the ] disappears. I don't want Parinfer to ever add or remove lines from the file, and I want to preserve comment-reversibility. So at the moment, I think supporting this feature would be a loss for the simplicity and user-predictability of close-paren inference.

@shaunlebron
Copy link
Collaborator

I can't think of a solution for this, so I'll document it in a faq

@shaunlebron
Copy link
Collaborator

shaunlebron commented Aug 16, 2017

This came up again. This is the first I've heard Parinfer "breaking UX" for non-parinfer users, despite preserving the code. This means parinfer changed the code format in such a way that makes it easy for non-parinfer users to break the code.

screen shot 2017-08-16 at 9 25 07 am

from: status-im/status-mobile@883b702#commitcomment-23648790

on the one hand, it’s a given that incorrect indentation is going to mislead you about implied structure.

on the other hand, if users setup their code so that they can safely uncomment lines without thinking, then it is parinfer that threw a fork in their process and will take the blame for the broken code

@shaunlebron shaunlebron changed the title Should comments be considered "code" Hanging-parens after indented comment lines Aug 16, 2017
@shaunlebron shaunlebron changed the title Hanging-parens after indented comment lines Allow closing parens after indented comment lines Aug 16, 2017
@gilch
Copy link

gilch commented Aug 18, 2017

This Parinfer behavior has always bothered me. ;; comments are indented like list elements. They should not be excluded from the form they belong to.

One partial workaround for now, at least in Clojure, is to use the discard syntax, #_. It could be as simple as a one-character symbol, like #_+) at the end of the list on the line following the comment, just to hold it open.

In Common Lisp and Emacs Lisp, the user can use a dotted list with an explicit nil, like . nil) instead.

But on team projects, this would likely violate the style norms.

@shaunlebron
Copy link
Collaborator

Thanks for commenting, I've been thinking about how to fix this, which I think is worth doing now based on what I've heard. elevating priority.

@shaunlebron
Copy link
Collaborator

Paren trail lines will be created as a result of indented comments. When the comments are removed or indented differently, stale paren trail lines should be removed rather than emptied. This will help keep Parinfer from sprinkling too many padded line artifacts when comments shift around. (tracking at #133)

@shaunlebron
Copy link
Collaborator

shaunlebron commented Sep 20, 2022

I will again close this, but move it to a WIP parstager tool which I think should take the responsibility of this UX issue: parinfer/parstager#1

@shaunlebron
Copy link
Collaborator

shaunlebron commented Sep 6, 2024

New idea

Re-opening (again), because:

  1. Parstager can only undo this formatting in untouched top-level expressions, not in new or edited ones.
  2. Standard Clojure Style is considering the possibility of preventing its adopters from doing this, if only because Parinfer can’t handle it. Chris argues essentially that the cost of everyone adopting this constraint would be less than the cost of excluding Parinfer users from using this standard. This might be true, but it also might prevent the adoption of that standard in the first place, because…
  3. …this might be the biggest, longstanding complaint about Parinfer, and I’d rather it not prevent Clojure from becoming the first Lisp community to adopt a standard auto-formatter.
  4. New possible solution presented. I think we might be able to avoid the problem with new rules for Indent Mode, explored below.

New Hygiene rule for cleaning up lines

  1. Delete any line if its paren trail is moved to a previous line, and all that remains is whitespace. For example:

    Before:

    (foo
       bar
       )
    
    ;; something

    After:

    (foo
       bar)
    
    ;; something

New Indentation rules for comment tokens

  1. The start of a comment ; is counted as an "indentation point". For example:

    (foo)
      ;; bar
      ^ indentation point is inside `(foo`
  2. Any indentation point starting with a comment has an associated "virtual" paren trail. For example:

    (foo
      ;; bar
            ^ virtual paren trail `)`, but we can’t put it here
  3. Any virtual pare trail is inserted into its own subsequent line, aligned to the start of the comment. For example:

    (foo
      ;; bar
      )
      ^ reified paren trail

Reversibility of rule 4 is enabled by rule 1

Here, “reversibility” means that if you type something, Parinfer’s effect on your code can be undone by removing what you just typed. My original concern above was that if someone repeatedly adds and removes a semicolon, it would create a flood of new lines being added over and over again. The hygiene rule (# 1) prevents this.

For example, if I comment out the last ) like so…

(foo
  ;; bar
  ;)
  ^ commenting out `)`

…then a new one will be inserted on a new line:

(foo
  ;; bar
  ;)
  )
  ^ new `)` on new line

And if I remove the semicolon that I just typed, the hygiene rule removes the line that Parinfer added:

(foo
  ;; bar
  )
  ^ uncommented (and subsequent line hygienically removed)

Impact on Design

These new rules seem to be systematic enough to allow comments to behave like normal expressions when participating in indentation rules— by just prefixing their paren trails with a newline and extra whitespace. And this is kept in check by a new hygiene rule which produces standard-looking formatted code in other cases as well.

The properties of these new rules should propagate pretty predictably, but I’ll see how it affects existing test cases and report back any unforeseen effects here.

Why did the original design include the constraint of not adding or removing lines?

I don't want Parinfer to ever add or remove lines from the file

Forgive me, I’m having trouble remembering. I think symmetry between input and output seemed nice or important in some contexts, but I can’t recall the reasons if I had them.

@shaunlebron shaunlebron reopened this Sep 6, 2024
@imrekoszo
Copy link

How would the following be formatted according to your new idea, Shaun?

;; example 1
(foo bar ;; something
 )

;; example 2
(foo
  bar ;; something
  )

@shaunlebron
Copy link
Collaborator

It‘s a good question— should inline comments and indented comments have their positions preserved in the AST, and if so, when? The current rules I stated above only relate to indented comments, and don’t change Parinfer’s behavior toward inline comments. So the closing parentheses are moved after bar in both cases: bar).

I opened an issue in the Clojure Style Guide and left my thoughts about inline comments there:
bbatsov/clojure-style-guide#258

@shaunlebron
Copy link
Collaborator

shaunlebron commented Sep 11, 2024

One thing I noticed is that Parinfer already has logic for preserving the relative indentation of child comments when their parent expression moves, but it looks like the way it’s set up actually breaks the reversibility goal. The video below shows how dedented comments can be captured by sufficiently dedenting the parent expression. This means that implementing these new rules would actually improve the behavior there.

dedented-comments-violate-reversibility.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants