Skip to content

Conversation

@scop
Copy link
Contributor

@scop scop commented Aug 6, 2025

No description provided.

@mvdan
Copy link
Owner

mvdan commented Aug 12, 2025

Are we positive that this is correct? It certainly is for //go: directives, but I have no idea whether other tools with their own syntax like //nolint also support // nolint equally. Do all these external tools prefer the no-space form? If any of them encourage the form with a space, then I think this change would cause too much trouble.

Copy link
Owner

@mvdan mvdan left a comment

Choose a reason for hiding this comment

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

I am also now thinking that this may fall out of the scope of a formatter. If someone writes

// go:whatever

and we rewrite the comment into

//go:whatever

we are likely enabling the directive, i.e. changing the behavior of how the program compiles or runs. You should be able to run gofumpt on a codebase and be confident that the behavior hasn't changed.

If you think the former is most likely a mistake, then it should be some sort of warning or "fix" suggestion to the user, and they should decide if or how they want to fix it. That fits into tools like go vet or staticcheck, but not gofumpt.

@mvdan
Copy link
Owner

mvdan commented Sep 16, 2025

I'm going to close this for now - as I described above, I don't think this is a sensible change to make in general. But please feel free to comment below if you disagree or have any useful examples.

@mvdan mvdan closed this Sep 16, 2025
@scop
Copy link
Contributor Author

scop commented Sep 18, 2025

I think essentially the same could be said about adding spaces which is what gofumpt already does. That could result in behavioral changes with tools that happen to be processing those comments, e.g. fail to process them if there's a space.

FWIW none of the tools I'm aware of that work with comments recommend a space there, on the contrary.

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.

2 participants