I could live with an alternative of a different flag to set the exit code, to avoid breaking existing things. i am sure there's someone out there who has gofmt in a script with set -e. But yes, please make it possible to use gofmt as a checker in a sane way.

@griesemer for notifications on this issue.

If we guard the exit-status change with a flag, It would be nice if we could make the flag's syntax and semantics match whatever flag we add for #27005.

If we think this change is too invasive, or could break too many users,

I think changing the exit status of gofmt -d probably would break some fraction of users, especially for Playground-like servers, and non-gopls integrations, and perhaps CI systems. I have a mild preference for guarding it with a flag.

You could add the flag for now and put in a deprecation warning that in X releases the flag will be inverted and the default will be exit codes.

In terms of breakage of CI systems, that is the behaviour I want, If i am enforcing gofmt on my code and you don't do it I want the CI to fall over. IMHO not doing that is the same as a compiler exiting 0 when the code failed to compile.

(i do realise my feelings may be a tad towards the hardline side of this :) ) in all honesty though I'll take whichever implementation we can get. :)

I don't think we can ever make the entire tool's default be using exit codes for "was the formating changed". It's entirely reasonable to use some-generator | gofmt to easily generate gofmt-formatted Go code, for example, and all those use cases where one wants to format Go code instead of checking for valid format can't be broken.

I'm also not sure that adding a deprecation or transition period will really help. Breaking a user's script in six months or in two years isn't much different, and I imagine most users will not even see the deprecation notices and remember all the scripts they need to update.

I think changing the exit status of gofmt -d probably would break some fraction of users, especially for Playground-like servers, and non-gopls integrations, and perhaps CI systems. I have a mild preference for guarding it with a flag.

At least from what I've seen, nearly all uses of gofmt -d fall under "check that Go code is correctly formatted", so I think nearly all users would want the exit status behavior. We could have them move to something like gofmt -d -check, but it would be a bit unfortunate to nearly always use both options at the same time.

My intuition is that most scripts using gofmt -d won't break, since what I've most commonly seen is in the form of:

DIFF="$(gofmt -d .)"
if [[ -n $DIFF ]]; then
    echo "$DIFF"
    echo "please run gofmt"
    exit 1
fi

In that kind of scenario, the exit code is by default discarded via the subshell. That's how I've seen many people use gofmt -l, too. The new form, actually using the exit code, could be as follows, or in shorter form via gofmt -d . || exit 1:

if ! gofmt -d .; then
    echo "please run gofmt"
    exit 1
fi

And for the cases where a script would break, I imagine it should be fairly easy to get the old behavior back, such as replacing gofmt -d . with gofmt -d . || true.

All that said, I don't have a strong opinion on new flag versus changing -d. I slightly lean towards not adding a flag, and seeing how many users report breakages during the beta and RC cycle.

In that kind of scenario, the exit code is by default discarded via the subshell.

By default, sure — but many shell style guides require explicit checks, and we probably don't want to break scripts that conform to that style either.

I slightly lean towards not adding a flag, and seeing how many users report breakages during the beta and RC cycle.

FWIW, in my experience changes outside the runtime, compiler, and standard library receive comparatively little testing during the beta and RC. (I would expect most breakage to be reported after the release.)

I prefer an explicit flag that's orthogonal to -d for a different reason than compatibility: in CI we don't use -d, we use -l. We don't care about the diffs; only reporting which files aren't formatted.

I would agree with @cespare diffs are only really used for when a human is looking, in CI I would want ideally two returns 1) the file that is failing and 2) the line number of the fail would be nice in the case of a few lines but once you get about 10 lines or so this matters less as at that point it's likely the whole files that is wrong.

Still currently any implementation is preferable to none, so additional flag or not I'd just be happy for a non zero exit code.

I personally don't mind too much whether this happens with -d, or via a separate flag, but I definitely think we should do this.

I agree that we want to be consistent with whatever #27005 ends up doing. At the same time, I don't particularly feel like waiting a few more years for a flag that's trivial to implement and very much needed for CI :)

If #27005 is expected to land on a flag design soon, like -check, then we can follow suit. If not, I think we should go ahead with something that makes sense for gofmt, which could be -check or something akin to git diff --exit-code, such as -exit or -exitcode.

Either way, I want to help us reach a decision for 1.21. If we're sure we want a decision on #27005 first, then I'll try to help push that forward.

Catching up on #27005, it seems like it's landing on -diff which will print a diff, and exit with a non-zero code if the diff isn't empty. There wouldn't be any other flag or behavior change.

So, if we wanted to be consistent, we could make -d use exit codes, like I originally proposed. We could make exit codes useful for any mode that prints changes that would be made without actually making them. This would be gofmt -d and gofmt -l, but not gofmt (which writes to stdout) nor gofmt -w (which writes to disk). Presumably, this would satisfy #46289 (comment) as well, even if it's a bit of magic.

If we prefer an explicit flag, like git diff --exit-code, which can be used in combination with any gofmt mode - that's also fine by me. It might also be the right idea in terms of not breaking existing users of the -d and -l flags. In that scenario, I'd attempt to copy git diff via -exitcode.

@rogpeppe suggests -check, and @findleyr seems to like it.