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 18:00:29 EST 2019


martinvonz added a comment.


  In https://phab.mercurial-scm.org/D5800#85220, @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.
  >
  > What I thought confusing is `scmutil.getuipathfn(ctx.repo(), legacyvalue=True)`
  >  in https://phab.mercurial-scm.org/D5801. "What does the `True` mean? relative, absolute, or a complete
  >  different stuff?"
  
  
  Same reason it's confusing, I believe: it's unclear if "legacyvalue=True" means "use the legacy value" (incorrect) or "for the legacy value, use the value True" (correct). I was hoping the "value" part would clarify that, but I agree that it's still not clear.  I think you're also saying that the fact that the function deals with producing a cwd-relative or absolute (well, repo-relative) is also not clear and I agree with that too. Maybe "usedtoberelative=True"?
  
  > I still think somehow including "relative" in the variable name is better than
  >  "forceto" or "forcevalue".
  
  Sure, I can change it to "forcerelative". It will be used in a single place (because there's only one existing config variable about relative paths), so it doesn't matter much anyway.

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