D2935: fancyopts: fix rendering of customopt defaults in help text

[dploch (Daniel Ploch)]

dploch created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Also make a getdefaultvalue() function to prevent unwanted mutation.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/fancyopts.py
  mercurial/help.py
  tests/test-help.t

CHANGE DETAILS

diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -716,15 +716,23 @@
 
   $ cat > helpext.py <<EOF
   > import os
-  > from mercurial import commands, registrar
+  > from mercurial import commands, fancyopts, registrar
   > 
+  > def func(arg):
+  >     return '%sfoo' % arg
+  > class customopt(fancyopts.customopt):
+  >     def newstate(self, oldstate, newparam, abort):
+  >         return '%sbar' % oldstate
   > cmdtable = {}
   > command = registrar.command(cmdtable)
   > 
   > @command(b'nohelp',
-  >     [(b'', b'longdesc', 3, b'x'*90),
+  >     [(b'', b'longdesc', 3, b'x'*67),
   >     (b'n', b'', None, b'normal desc'),
-  >     (b'', b'newline', b'', b'line1\nline2')],
+  >     (b'', b'newline', b'', b'line1\nline2'),
+  >     (b'', b'callableopt', func, b'adds foo'),
+  >     (b'', b'customopt', customopt(''), b'adds bar'),
+  >     (b'', b'customopt-withdefault', customopt('foo'), b'adds bar')],
   >     b'hg nohelp',
   >     norepo=True)
   > @command(b'debugoptADV', [(b'', b'aopt', None, b'option is (ADVANCED)')])
@@ -786,10 +794,14 @@
   
   options:
   
-      --longdesc VALUE xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
-                       xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (default: 3)
-   -n --               normal desc
-      --newline VALUE  line1 line2
+      --longdesc VALUE
+                                    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+                                    xxxxxxxxxxxxxxxxxxxxxxx (default: 3)
+   -n --                            normal desc
+      --newline VALUE               line1 line2
+      --callableopt VALUE           adds foo
+      --customopt VALUE             adds bar
+      --customopt-withdefault VALUE adds bar (default: foo)
   
   (some details hidden, use --verbose to show complete help)
 
diff --git a/mercurial/help.py b/mercurial/help.py
--- a/mercurial/help.py
+++ b/mercurial/help.py
@@ -20,6 +20,7 @@
     encoding,
     error,
     extensions,
+    fancyopts,
     filemerge,
     fileset,
     minirst,
@@ -84,7 +85,10 @@
         if shortopt:
             so = '-' + shortopt
         lo = '--' + longopt
-        if default:
+
+        if isinstance(default, fancyopts.customopt):
+            default = default.getdefaultvalue()
+        if default and not callable(default):
             # default is of unknown type, and in Python 2 we abused
             # the %s-shows-repr property to handle integers etc. To
             # match that behavior on Python 3, we do str(default) and
diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py
--- a/mercurial/fancyopts.py
+++ b/mercurial/fancyopts.py
@@ -208,20 +208,27 @@
     __metaclass__ = abc.ABCMeta
 
     def __init__(self, defaultvalue):
-        self.defaultvalue = defaultvalue
+        self._defaultvalue = defaultvalue
 
     def _isboolopt(self):
         return False
 
+    def getdefaultvalue(self):
+        """Returns the default value for this opt.
+
+        Subclasses should override this to return a new value if the value type
+        is mutable."""
+        return self._defaultvalue
+
     @abc.abstractmethod
     def newstate(self, oldstate, newparam, abort):
         """Adds newparam to oldstate and returns the new state.
 
         On failure, abort can be called with a string error message."""
 
 class _simpleopt(customopt):
     def _isboolopt(self):
-        return isinstance(self.defaultvalue, (bool, type(None)))
+        return isinstance(self._defaultvalue, (bool, type(None)))
 
     def newstate(self, oldstate, newparam, abort):
         return newparam
@@ -235,6 +242,9 @@
         return self.callablefn(newparam)
 
 class _listopt(customopt):
+    def getdefaultvalue(self):
+        return self._defaultvalue[:]
+
     def newstate(self, oldstate, newparam, abort):
         oldstate.append(newparam)
         return oldstate
@@ -313,7 +323,7 @@
         defmap[name] = _defaultopt(default)
 
         # copy defaults to state
-        state[name] = defmap[name].defaultvalue
+        state[name] = defmap[name].getdefaultvalue()
 
         # does it take a parameter?
         if not defmap[name]._isboolopt():



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