fix(hooks): post-edit-format discards dotnet format failures - #28
Open
kig777 wants to merge 2 commits into
Open
Conversation
`dotnet format --include` resolves its paths against the current directory, so the absolute path the hook passes matches no file. The command still exits 0, and `2>/dev/null || true` swallows the outcome, so the hook has been a silent no-op — edited files are never formatted and nothing reports it. On Git Bash there is a second, independent cause: the script normalizes the edited path to the mixed C:/... form for its directory walk and then hands that same form to dotnet as the project argument. dotnet loads the project fine, but --include then matches nothing even when the include path is correct. The project argument has to be a native Windows path. Fix both: convert the project with `cygpath -w` when cygpath is present, and make --include relative to the current directory when the file lives under it. Paths outside the current directory keep their previous form, and on Linux and macOS the project argument is untouched. Verified against a real solution on Windows 11 / Git Bash by introducing an indentation violation and checking `git status` after the hook ran — before the change the violation survived, after it the file comes back formatted. Covered all three input paths the hook accepts: PostToolUse stdin JSON, `$1` with a backslash path, and `$1` with a relative path.
`2>/dev/null || true` discarded both the error stream and the exit code, so a broken invocation looked exactly like a successful one. That is what kept the --include path bug invisible: the hook had been formatting nothing, and there was no signal anywhere to say so. Capture the output and print it when dotnet format exits non-zero. Still exit 0 — files mid-edit routinely fail to compile, and a hook that failed there would interrupt an ordinary edit for something the next edit fixes. Output is printed only on failure, so a successful format stays quiet: dotnet format prints nothing on success at default verbosity, and it exits 0 for soft problems such as workspace load warnings, which would otherwise add noise to every single edit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #27, as offered there.
Stacked on #27 — this branch contains that commit, so the PR shows two. Only the second one (
fix(hooks): report a failed dotnet format instead of discarding it) belongs to this PR. Merge #27 first and this rebases to a single commit; if you would rather have it standalone, say so and I will rebase ontomain.Problem
This throws away the error stream and the exit code together, so a broken invocation is indistinguishable from a working one. That is precisely what let the
--includepath bug in #27 survive unnoticed: the hook formatted nothing, on every edit, and no signal existed anywhere to say so.Fix
Capture the output, print it when
dotnet formatexits non-zero.Still
exit 0deliberately: files mid-edit routinely fail to compile, and a hook that failed there would interrupt an ordinary edit over something the next edit fixes. The goal is visibility, not a gate.Printing only on failure also keeps a successful run quiet —
dotnet formatprints nothing on success at default verbosity, and it exits 0 for soft problems like workspace load warnings, which would otherwise add a line of noise to every edit.Verification
Windows 11, Git Bash, .NET 10 SDK, against a real solution.
Success path — introduce an indentation violation, run the hook:
Failure path — same call with
dotnetoffPATH:Worth noting for the record: a corrupt
.csprojdoes not trigger this branch —dotnet formatreportsWarnings were encountered while loading the workspaceand still exits 0. So this surfaces hard failures (missing SDK, bad arguments, IO errors), not every soft problem.