D2623: dispatch: adding config items for overriding flag defaults

[rdamazio (Rodrigo Damazio Bovendorp)]

rdamazio added inline comments.

INLINE COMMENTS

> dploch wrote in dispatch.py:625-626
> This doesn't handle callables properly.  I wonder if the something like the following would work instead:
> 
> oldopt = fancyopts._defaultopt(olddefault)
> newdefault = old.opt.newstate(olddefault, ui.config("commands", cfgitem)
> c[idx] = (opt[0], opt[1], fancyopts._withnewdefault(oldopt, newdefault), opt[3])
> 
> Where '_withnewdefault' is a wrapper customopt that just changes the default.

I thought callables were meant to be used to generate the default default, not with overridden values?

> dploch wrote in dispatch.py:639-640
> This makes me nervous.  What if someone re-uses a customopt instance in multiple commands?  i.e.:
> 
> DATE_FLAG = mypkg.dateopt()
> ...
> ('b', 'before', DATE_FLAG, '')
> ('a', 'after', 'DATE_FLAG', '')
> 
> Now, setting commands.defaults.before=2018-03-05 also silently changes the default for 'after'.  I suspect we need to introduce a wrapper class like what I suggest on lines 625-625, that delegates and leaves the original default unchanged.  And either way, we should probably clarify in the docs on customopts what expected use of the class is (i.e., should we just forbid reuse, is 'oldstate' safe to mutate, etc.)

Nobody should use the same *instance* on multiple flags. Even with the current flags, if you use e.g. the same list on many, that'll cause problems with listopt.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2623

To: rdamazio, #hg-reviewers, yuja
Cc: dploch, mercurial-devel