D5800: config: introduce a new value for ui.relative-paths getting old behavior
martinvonz (Martin von Zweigbergk)
phabricator at mercurial-scm.org
Sun Feb 3 12:36:11 EST 2019
martinvonz added a comment.
In https://phab.mercurial-scm.org/D5800#85128, @yuja wrote:
> Looks good, but I find it isn't easy to parse the meaning of
> `getuipathfn(repo, forcevalue=True)`. Perhaps it can be spelled as
> `forcerelative=True`.
The problem is that this an override value for a boolean value, so it's easy to mistake `forcerelative=False` to mean "don't force it to be relative". That's why I went for `forcevalue` (meaning "force to this value"). I agree that it can still easily be read as "force a value". Perhaps `forceto` works better, although it's a bit unconventional? I'll change it to that and you can tell me if it seems worse.
>
>
>> +def getuipathfn(repo, legacyvalue=False, forcevalue=None):
>
>
>
>> + if forcevalue is not None:
>> + relative = forcevalue
>> + else:
>> + config = repo.ui.config('ui', 'relative-paths')
>> + if config == 'legacy':
>> + relative = legacyvalue
>> + else:
>> + relative = stringutil.parsebool(config)
>
> If we want to report an invalid boolean value, we still need to use
> `ui.configbool()`.
I had not realized that we errored out when attempting to use a bad config value. Anyway, I did what you suggested at first, but then I got the following from `test-check-config.t`:
+ relative = repo.ui.configbool('ui', 'relative-paths')
+ conflict on ui.relative-paths: ('bool', '') != ('str', '')
+ at mercurial/scmutil.py:748:
I just checked what we do for `ui.color` (which accepts e.g. "auto" and booleans). We don't report bad values there either. That doesn't mean it's right, of course. But I guess I'll have to duplicate the exception-raising rather than calling into `ui.configbool()` in order keep `test-check-config.t` happy.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D5800
To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel
More information about the Mercurial-devel
mailing list