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

go: Proper indent in compiler #2976

Merged
merged 1 commit into from
May 15, 2024
Merged

go: Proper indent in compiler #2976

merged 1 commit into from
May 15, 2024

Conversation

fishy
Copy link
Member

@fishy fishy commented May 14, 2024

This is a "trivial" change for go compiler to always use the combination of indent_up, indent_down, and indent, over manual indentation (by adding 2 spaces at the beginning of the string). Also change go compiler's indent_str to tab over 2 spaces.

While I'm here, also made a few minor tweaks on generated go code.

@fishy fishy requested a review from dcelasun May 14, 2024 21:09
This is a "trivial" change for go compiler to always use the combination
of indent_up, indent_down, and indent, over manual indentation (by
adding 2 spaces at the beginning of the string). Also change go
compiler's indent_str to tab over 2 spaces.

While I'm here, also made a few minor tweaks on generated go code.
Copy link
Member

@dcelasun dcelasun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. Does this make go fmt a no-op?

@fishy
Copy link
Member Author

fishy commented May 15, 2024

Great idea. Does this make go fmt a no-op?

Not really. There are still quite a few things not there yet:

  • imports need to be sorted alphabetically
  • vertical alignment (for example, for field names of a struct, need to figure out the longest one and align everything else)
  • proper blank lines (no two blank lines between functions, etc.)
  • ...

@fishy fishy merged commit 4930cac into master May 15, 2024
49 of 66 checks passed
@fishy fishy deleted the go-compiler-indentation branch May 15, 2024 16:24
@Jens-G
Copy link
Member

Jens-G commented May 15, 2024

Just to mention it ... there once was a call to the net effect of system("go fmt") in the go generator so the maintainer did not have to care at all. That might explain why it is at it is. That call was later thrown out because some automated code scanner labeled it as the pure evil. Removing it was the most effective solution. So just in case someone comes up with that same idea, you can immediately give the -1 yourself. ;-)

@fishy
Copy link
Member Author

fishy commented May 15, 2024

Thanks for the context! It might not be that bad an idea if for each language we allow the compiler to define a command to run optionally after the file is generated to format it, and configured the "official" formatter for the languages if available.

But there are definitely challenges to make it work on both windows and posix :) And security concerns :)

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.

3 participants