This is an archive of the discontinued Mercurial Phabricator instance.

errors: raise ConfigError on failure to parse config file
ClosedPublic

Authored by martinvonz on Nov 20 2020, 7:25 PM.

Details

Summary

This replaces two raises of ParseError by ConfigError, which makes
it so we get the desired exit code when ui.detailed-exit-code is
enabled. Because the exceptions include a location, I had to add that
to ConfigError as well. I considered making ConfigError a subclass
of ParseError, but it doesn't feel like it quite passes the "is-a"
test.

I used "config error: " as prefix for these errors instead of the
previous "hg: parse error: ", which seems a little less accurate now
(and, as I've said before, I don't know what the "hg: " part is
supposed to signify anyway). I can easily be convinced to change the
prefix to something else (including "abort: ").

Some of the exceptions raised here mean that we fail to even load the
ui object in the dispatch module. When that happens, we don't know
to use detailed exit codes, so some tests (e.g. test-hgrc.t) still
see exit code 255. I'll try to get back to that later. It should be
possible to give detailed exit codes if at least part of the config
can be read (e.g. when the system-wide one enables detailed exit codes
and the user's config fails to parse).

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Nov 20 2020, 7:25 PM
mharbison72 added inline comments.
tests/test-hgrc.t
20

Why do the exit codes in the first couple of tests above get converted to 30 for ConfigError, but not here?

martinvonz added inline comments.Nov 21 2020, 12:56 AM
tests/test-hgrc.t
20

Ah, good question! The explanation is in the commit message of D9355 (we can't enable detailed exit codes if we fail to parse the config file that would tell us to do so, although we can probably do a little better than we do here). Now that I've fixed your comment there about using ConfigError and exit code 30, that comment belongs in this commit. I'll move it here in a bit.

martinvonz edited the summary of this revision. (Show Details)Nov 21 2020, 1:35 AM
mharbison72 accepted this revision.Nov 21 2020, 3:10 PM
This revision is now accepted and ready to land.Nov 21 2020, 3:10 PM