Add parse error location to unparsable environment file warning#11996
Open
wasimat404 wants to merge 4 commits into
Open
Add parse error location to unparsable environment file warning#11996wasimat404 wants to merge 4 commits into
wasimat404 wants to merge 4 commits into
Conversation
When a GHC environment file cannot be parsed, the warning now includes the line, column, and reason from the underlying parser, instead of only stating that the file is unparsable. Fixes haskell#11963 Signed-off-by: wasimat404 <huaweiwasim7@gmail.com>
Signed-off-by: wasimat404 <huaweiwasim7@gmail.com>
Text.Parsec.Error.ParseError has no Exception instance on older GHCs, so displayException does not type-check. Its Show instance already renders the line, column, and reason. Signed-off-by: wasimat404 <huaweiwasim7@gmail.com>
zlonast
reviewed
Jun 24, 2026
| -- merely state that the file is unparsable. | ||
| -- See https://github.com/haskell/cabal/issues/11963 | ||
| res <- cabal' "install" ["--lib", "base", "--package-env=" ++ envFile] | ||
| assertOutputContains "line 1, column 1" res |
Collaborator
There was a problem hiding this comment.
This is a really poor test. Couldn't we get a clearer test by removing DoNotRecord? Or should we capture more output from assertOutputContains?
Author
There was a problem hiding this comment.
I went with capturing more output. It now asserts on the full warning (location, unexpected character, and expected tokens), not just the line and column.
and tried removing DoNotRecord first, but the install exits successfully here, so golden recording treats that as a failure. Should I switch to golden?
Assert on the full warning message (location, unexpected character, and expected tokens) rather than only the line and column, addressing review feedback on test coverage. Signed-off-by: wasimat404 <huaweiwasim7@gmail.com>
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
2 participants
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.
Fixes #11963.
When a GHC environment file cannot be parsed, cabal previously warned only that the file "is unparsable", giving no indication of where or why. The underlying parser already produces a Parsec error carrying the line, column, and the unexpected/expected tokens, but that information was discarded at the warning site in
getExistingEnvEntries. This surfaces it.Before:
After:
The reporter's case was a stray trailing newline, which the parser reports as "unexpected end of input" at the offending line, so this change points directly at it. The fix is message-only; parser behaviour is unchanged.
Added a
cabal-testsuitepackage test underPackageTests/EnvironmentFile/UnparsableWarningthat points cabal at a malformed environment file and asserts the warning includes the location. Verified the test fails on unpatched cabal (the location line is absent) and passes with the change.QA Notes
Create a malformed GHC environment file, e.g. a file
bad.envwhose first line isthis is not valid. Then run:The warning should name the parse location, for example:
Before this change, only the first line ("is unparsable") was printed, with no location.