Sort --no-foo after --foo

2 files changed, 82 insertions, 24 deletions

diff --git a/cliapp/genman.py b/cliapp/genman.py
index 319ba86..f2c81c2 100644
--- a/cliapp/genman.py
+++ b/cliapp/genman.py
@@ -30,8 +30,47 @@ class ManpageGenerator(object):
         self.cmd_synopsis = cmd_synopsis
 
     def sort_options(self, options):
-        return sorted(options,
-                      key=lambda o: (o._long_opts + o._short_opts)[0])
+        # Return the options in an option group in sorted order. This
+        # is slightly tricky. Humans have such weird opinions of what
+        # it means to be sorted. The main thing we care about is that
+        # given a boolean setting foo, the options --foo and --no-foo
+        # get sorted after each other. We do this by using the
+        # option.from_setting attribute, which might not exist. The
+        # list already includes the --no-foo options, which seems a
+        # bit silly, but there you go.
+        #
+        # Split list of options into two: one with --foo, one with
+        # --no-foo. Sort the first list. Insert options from second
+        # list into first list.
+
+        def is_neg_option(o):
+            return (hasattr(o, 'from_setting') and
+                    o._long_opts and
+                    o._long_opts[0].startswith('--no-'))
+
+        def split(options, setting):
+            before = []
+            from_same = []
+            after = []
+            for o in options:
+                s = getattr(o, 'from_setting', None)
+                if s is setting:
+                    from_same.append(o)
+                elif from_same:
+                    after.append(o)
+                else:
+                    before.append(o)
+            return before, from_same, after
+
+        neg_options = [o for o in options if is_neg_option(o)]
+        main_options = [o for o in options if o not in neg_options]
+        main_options.sort(key=lambda o: (o._long_opts + o._short_opts)[0])
+
+        for neg in neg_options:
+            before, from_same, after = split(main_options, neg.from_setting)
+            main_options = before + from_same + [neg] + after
+
+        return main_options
 
     def option_list(self, container):
         return self.sort_options(container.option_list)
diff --git a/cliapp/settings.py b/cliapp/settings.py
index 340b594..bfc6b4a 100644
--- a/cliapp/settings.py
+++ b/cliapp/settings.py
@@ -488,6 +488,16 @@ class Settings(object):
 
         config_group = option_groups[config_group_name]
 
+        # Helper to add an option and add a reference to the Setting
+        # object it is created from (or None). This allows manpage
+        # generation to recognize when --foo and --no-foo come from
+        # the same setting.
+
+        def add_option_to_group(setting, group, *args, **kwargs):
+            option = group.add_option(*args, **kwargs)
+            option.from_setting = setting
+            return option
+
         # Return help text, unless setting/option is hidden, in which
         # case return optparse.SUPPRESS_HELP.
 
@@ -504,7 +514,8 @@ class Settings(object):
                 sys.stdout.write('%s\n' % name)
             sys.exit(0)
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--dump-setting-names',
             action='callback',
             nargs=0,
@@ -517,7 +528,8 @@ class Settings(object):
             self.dump_config(sys.stdout)
             sys.exit(0)
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--dump-config',
             action='callback',
             nargs=0,
@@ -530,7 +542,8 @@ class Settings(object):
             self.config_files = []
             self._required_config_files = []
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--no-default-configs',
             action='callback',
             nargs=0,
@@ -543,7 +556,8 @@ class Settings(object):
             self.config_files.append(value)
             self._required_config_files.append(value)
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--config',
             action='callback',
             nargs=1,
@@ -559,7 +573,8 @@ class Settings(object):
                 print filename
             sys.exit(0)
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--list-config-files',
             action='callback',
             nargs=0,
@@ -570,7 +585,8 @@ class Settings(object):
 
         self._arg_synopsis = arg_synopsis
         self._cmd_synopsis = cmd_synopsis
-        p.add_option(
+        add_option_to_group(
+            None, p,
             '--generate-manpage',
             action='callback',
             nargs=1,
@@ -590,7 +606,8 @@ class Settings(object):
             sys.stdout.write(pp.format_help())
             sys.exit(0)
 
-        config_group.add_option(
+        add_option_to_group(
+            None, config_group,
             '--help-all',
             action='callback',
             help='show all options',
@@ -616,15 +633,16 @@ class Settings(object):
 
         def add_option(obj, s):
             option_names = self._option_names(s.names)
-            obj.add_option(*option_names,
-                           action='callback',
-                           callback=maybe(set_value),
-                           callback_args=(s,),
-                           type=s.type,
-                           nargs=s.nargs,
-                           choices=s.choices,
-                           help=help_text(s.help, s.hidden),
-                           metavar=s.metavar)
+            add_option_to_group(
+                s, obj, *option_names,
+                action='callback',
+                callback=maybe(set_value),
+                callback_args=(s,),
+                type=s.type,
+                nargs=s.nargs,
+                choices=s.choices,
+                help=help_text(s.help, s.hidden),
+                metavar=s.metavar)
 
         def add_negation_option(obj, s):
             option_names = self._option_names(s.names)
@@ -632,12 +650,13 @@ class Settings(object):
             neg_names = ['--no-' + x[2:] for x in long_names]
             unused_names = [x for x in neg_names
                             if x[2:] not in self._settingses]
-            obj.add_option(*unused_names,
-                           action='callback',
-                           callback=maybe(set_false),
-                           callback_args=(s,),
-                           type=s.type,
-                           help=help_text('', s.hidden))
+            add_option_to_group(
+                s, obj, *unused_names,
+                action='callback',
+                callback=maybe(set_false),
+                callback_args=(s,),
+                type=s.type,
+                help=help_text('opposite of %s' % option_names[0], s.hidden))
 
         # Add options for every setting.