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

Align trailing comments #252

Open
matthargett opened this issue Aug 31, 2021 · 6 comments
Open

Align trailing comments #252

matthargett opened this issue Aug 31, 2021 · 6 comments
Labels
enhancement New feature or request requires option This feature request would require option configuration rfc/in discussion This issue is currently in discussion, comments are welcome!

Comments

@matthargett
Copy link

matthargett commented Aug 31, 2021

in this example, spacing before and inside of comments is key to legibility (in fixed-width font, anyway):

function foo(
	tinky,  -- this is
	winky,  -- my aligned
	inky,   -- ascii art
	blinky  -- awesomeness
)
end

the aligned comments become this:
image

@JohnnyMorganz
Copy link
Owner

This is an interesting one, because it leads us onto something else to consider: should StyLua purposefully align items, such as comments? I know its something done in gofmt, but not in prettier or others. It has its pros and cons, the issue mainly being polluting diffs and it being quite challenging to implement.

Back to your original suggestion of keeping the spacing of comments alone in the first place, one thing I try not to do is rely on the input AST too much for formatting choices. In the case you mentioned it works because you are trying to align things, but it also starts affecting places where you want stylua to remove the spacing.

local x = 1      -- i did some refactoring leaving spacing before this comment, i want it removed

@matthargett
Copy link
Author

in your example, it's not an inline comment on a type/argument/table element list. forcing alignment in those cases is interesting, but as you mention it then requires a larger AST block to be the basis of the analysis/formatting, potentially opening up a can of worms in terms of complexity/accuracy.

@matthargett
Copy link
Author

ping on this. is this doable for argument, type, and table lists?

@JohnnyMorganz
Copy link
Owner

I'm still not 100% because it opens a gap in the automated formatting.
If I were to add it, it would have to be behind an option

@JohnnyMorganz JohnnyMorganz changed the title leave inline comment spacing alone Align trailing comments Feb 20, 2022
@JohnnyMorganz JohnnyMorganz added enhancement New feature or request rfc/in discussion This issue is currently in discussion, comments are welcome! labels Feb 20, 2022
@CKolkey
Copy link

CKolkey commented Apr 4, 2022

Another use case to consider for this is when lining up equal signs:

{
  foo_bar = { a = 10, b = 2 },
  fo_br = { a = 100, b = 2 },
  foo = { a = 1, b = 2 },
  fooo_bar = { a = 1000, b = 2 },
}

vs.

{
  foo_bar  = { a = 10,   b = 2 },
  fo_br    = { a = 100,  b = 2 },
  foo      = { a = 1,    b = 2 },
  fooo_bar = { a = 1000, b = 2 },
}

The ability to quickly parse the data structure and see the common elements is very nice in my opinion. Rubocop (ruby formatter) respects manual formatting of columns, though doesn't automatically format them for you, effectively allowing an implicit user override, which I think is a good approach. I'm not sure if its helpful, but I believe this is the implementation: https://github.com/rubocop/rubocop/blob/master/lib/rubocop/cop/layout/extra_spacing.rb

@baggiponte
Copy link

As as side note, luals does this by default through EmmyLua

@JohnnyMorganz JohnnyMorganz added the requires option This feature request would require option configuration label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request requires option This feature request would require option configuration rfc/in discussion This issue is currently in discussion, comments are welcome!
Projects
None yet
Development

No branches or pull requests

4 participants