-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: go/format: simplify old directive comments #68061
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
Comments
cc @griesemer @adonovan @findleyr @lfolger from the related #68021 |
Though I would like to live in the world where the proposed conversion process has completed, it does seem like quite a lot of effort for a marginal improvement, versus living with a few legacy Namespace="" directives. |
While that's the motivation that led me here, the simplification goes further into having a single simple rule without any special cases to remember. That makes things easier to learn and easier to use. Use covers a lot: writing a directive, using the iterator in the related issue, or adding the pattern to a syntax highlighter. It is minor and perhaps it is more trouble than its worth but may as well weigh that before locking it in further. |
Maybe I'm missing something but unless you fully deprecate the old forms (which seems very costly), you still have to special case and remember them, don't you? |
(I'm going to stick with line for discussion but everything applies to the other two as well.) Making
Though minor those simplifications mean no one using go/ast to examine directives needs to worry about the special case. It's handled transparently. Having gofmt rewrite The code for handling |
This makes sense but only if #68021 is accepted because otherwise the only way to retrieve line directives is to actually parse the raw comments. Or are you suggesting to also change the values of the raw comments? |
Part 2 of the proposal is to have gofmt rewrite X → go:X (for modules with a language version after they become synonymous). It already moves comment directives so it's not entirely unprecedented and these directives are defined by Go so it is safe to rewrite them. I think it's important to settle this matter first as #68021 locks the format into the compatibility agreement more tightly than it is now. If the iterator goes in and then later it's decided to do this it would be unfortunate if for compatibility reasons |
I agree that it would be nice to live in a world where this were consistent, and share the concern that it seems somewhat disruptive to get there. I find myself considering a variation of this proposal that might be less disruptive:
This variant means that anyone hand-writing code still doesn't need to remember the rule -- they can just always write a I think this would also help avoid mistakes where someone uses a new toolchain to work on a codebase that's intended to still work on older Go versions and ends up with the comments rewritten into a shape that would not be accepted (possibly silently ignored?) by other toolchain versions. (Though I understand that the rule about using the version in Of course, it does mean that the exceptions must remain exceptional forever. This alternative does not lead to a future where the unprefixed forms might become invalid altogether. |
For //export and //go:export, that was a detail that only affected cgo really. The same is true, to a lesser extent, for //extern. |
This proposal has been added to the active column of the proposals project |
It sounds like this is not worth doing. It has very little benefit and will inevitably cause churn and perhaps even introduce bugs. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
Proposal Details
#51082 standardized Go directive comments. #37974 extended the syntax to allow for third party directives.
This left three old directives with custom formats:
line
,export
, andextern
.#38785 agreed to treat
//export
and//go:export
as synonyms, though this did not go through the proposal process, was never implemented, and did not includeline
andextern
.We should revive and expand this to all three:
For
line
(and similarly forexport
andextern
)//line
and//go:line
in go1.N//line
to//go:line
when the module language version >= go1.NThe text was updated successfully, but these errors were encountered: