Merge lp:~vila/bzr/747050-config-help into lp:bzr

[Vincent Ladeuil]

Implement per-config option help

This implements help topics for registered configuration options.

I refrained from refactoring too much (still reducing the code
duplication a bit) but some love seems to be needed in this area.

I haven't deleted the text from help_topics/configuration.txt
yet, that would be better addressed when fixing bug #746993.

[Jelmer Vernooij (jelmer)]

TestConfigOptiondIndex seems like it has an extra 'd' :)

TestConfigOptionHelp seems unused.

review: Needs Fixing

[Vincent Ladeuil (vila)]

sent to pqm by email

[Vincent Ladeuil (vila)]

sent to pqm by email

[Vincent Ladeuil (vila)]

sent to pqm by email

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2011-08-13 03:39:24 +0000
+++ bzrlib/config.py	2011-08-16 07:35:26 +0000
@@ -2307,6 +2307,15 @@
     def get_default(self):
         return self.default
 
+    def get_help_text(self, additional_see_also=None, plain=True):
+        result = self.help
+        from bzrlib import help_topics
+        result += help_topics._format_see_also(additional_see_also)
+        if plain:
+            result = help_topics.help_as_plain_text(result)
+        return result
+
+
 # Predefined converters to get proper values from store
 
 def bool_from_store(unicode_str):
@@ -2351,13 +2360,12 @@
     def register_lazy(self, key, module_name, member_name):
         """Register a new option to be loaded on request.
 
-        :param key: This is the key to use to request the option later. Since
-            the registration is lazy, it should be provided and match the
-            option name.
-
-        :param module_name: The python path to the module. Such as 'os.path'.
-
-        :param member_name: The member of the module to return.  If empty or
+        :param key: the key to request the option later. Since the registration
+            is lazy, it should be provided and match the option name.
+
+        :param module_name: the python path to the module. Such as 'os.path'.
+
+        :param member_name: the member of the module to return.  If empty or 
                 None, get() will return the module itself.
         """
         super(OptionRegistry, self).register_lazy(key,
@@ -2379,7 +2387,7 @@
 
 option_registry.register(
     Option('dirstate.fdatasync', default=True, from_unicode=bool_from_store,
-           help='''
+           help='''\
 Flush dirstate changes onto physical disk?
 
 If true (default), working tree metadata changes are flushed through the

=== modified file 'bzrlib/help.py'
--- bzrlib/help.py	2011-05-18 18:32:22 +0000
+++ bzrlib/help.py	2011-08-16 07:35:26 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005-2010 Canonical Ltd
+# Copyright (C) 2005-2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -135,6 +135,7 @@
             help_topics.HelpTopicIndex(),
             _mod_commands.HelpCommandIndex(),
             plugin.PluginsHelpIndex(),
+            help_topics.ConfigOptionHelpIndex(),
             ]
 
     def _check_prefix_uniqueness(self):

=== modified file 'bzrlib/help_topics/__init__.py'
--- bzrlib/help_topics/__init__.py	2011-06-30 16:47:06 +0000
+++ bzrlib/help_topics/__init__.py	2011-08-16 07:35:26 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006-2010 Canonical Ltd
+# Copyright (C) 2006-2011 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -37,6 +37,7 @@
 
 import bzrlib
 from bzrlib import (
+    config,
     osutils,
     registry,
     )
@@ -65,7 +66,7 @@
         :param section: Section in reference manual - see SECT_* identifiers.
         """
         # The detail is stored as the 'object' and the metadata as the info
-        info=(summary,section)
+        info = (summary, section)
         super(HelpTopicRegistry, self).register(topic, detail, info=info)
 
     def register_lazy(self, topic, module_name, member_name, summary,
@@ -79,7 +80,7 @@
         :param section: Section in reference manual - see SECT_* identifiers.
         """
         # The detail is stored as the 'object' and the metadata as the info
-        info=(summary,section)
+        info = (summary, section)
         super(HelpTopicRegistry, self).register_lazy(topic, module_name,
                                                      member_name, info=info)
 
@@ -844,6 +845,15 @@
             return []
 
 
+def _format_see_also(see_also):
+    result = ''
+    if see_also:
+        result += '\n:See also: '
+        result += ', '.join(sorted(set(see_also)))
+        result += '\n'
+    return result
+
+
 class RegisteredTopic(object):
     """A help topic which has been registered in the HelpTopicRegistry.
 
@@ -867,17 +877,7 @@
             returned instead of plain text.
         """
         result = topic_registry.get_detail(self.topic)
-        # there is code duplicated here and in bzrlib/plugin.py's
-        # matching Topic code. This should probably be factored in
-        # to a helper function and a common base class.
-        if additional_see_also is not None:
-            see_also = sorted(set(additional_see_also))
-        else:
-            see_also = None
-        if see_also:
-            result += '\n:See also: '
-            result += ', '.join(see_also)
-            result += '\n'
+        result += _format_see_also(additional_see_also)
         if plain:
             result = help_as_plain_text(result)
         return result
@@ -903,3 +903,28 @@
         line = re.sub(":doc:`(.+?)-help`", r'``bzr help \1``', line)
         result.append(line)
     return "\n".join(result) + "\n"
+
+
+class ConfigOptionHelpIndex(object):
+    """A help index that returns help topics for config options."""
+
+    def __init__(self):
+        self.prefix = 'configuration/'
+
+    def get_topics(self, topic):
+        """Search for topic in the registered config options.
+
+        :param topic: A topic to search for.
+        :return: A list which is either empty or contains a single
+            config.Option entry.
+        """
+        if topic is None:
+            return []
+        elif topic.startswith(self.prefix):
+            topic = topic[len(self.prefix):]
+        if topic in config.option_registry:
+            return [config.option_registry.get(topic)]
+        else:
+            return []
+
+

=== modified file 'bzrlib/help_topics/en/configuration.txt'
--- bzrlib/help_topics/en/configuration.txt	2011-08-02 01:10:27 +0000
+++ bzrlib/help_topics/en/configuration.txt	2011-08-16 07:35:26 +0000
@@ -14,7 +14,7 @@
 
   "John Doe <[e-mail]>"
 
-See also the ``email`` configuration value.
+See also the ``email`` configuration option.
 
 BZR_PROGRESS_BAR
 ~~~~~~~~~~~~~~~~
@@ -54,7 +54,7 @@
 
 Path to the Bazaar executable to use when using the bzr+ssh protocol.
 
-See also the ``bzr_remote_path`` configuration value.
+See also the ``bzr_remote_path`` configuration option.
 
 BZR_EDITOR
 ~~~~~~~~~~

=== modified file 'bzrlib/plugin.py'
--- bzrlib/plugin.py	2011-05-31 06:15:24 +0000
+++ bzrlib/plugin.py	2011-08-16 07:35:26 +0000
@@ -506,21 +506,12 @@
             result = self.module.__doc__
         if result[-1] != '\n':
             result += '\n'
-        # there is code duplicated here and in bzrlib/help_topic.py's
-        # matching Topic code. This should probably be factored in
-        # to a helper function and a common base class.
-        if additional_see_also is not None:
-            see_also = sorted(set(additional_see_also))
-        else:
-            see_also = None
-        if see_also:
-            result += 'See also: '
-            result += ', '.join(see_also)
-            result += '\n'
+        from bzrlib import help_topics
+        result += help_topics._format_see_also(additional_see_also)
         return result
 
     def get_help_topic(self):
-        """Return the modules help topic - its __name__ after bzrlib.plugins.."""
+        """Return the module help topic: its basename."""
         return self.module.__name__[len('bzrlib.plugins.'):]
 
 

=== modified file 'bzrlib/tests/test_help.py'
--- bzrlib/tests/test_help.py	2011-06-27 15:36:58 +0000
+++ bzrlib/tests/test_help.py	2011-08-16 07:35:26 +0000
@@ -21,6 +21,7 @@
 from bzrlib import (
     builtins,
     commands,
+    config,
     errors,
     help,
     help_topics,
@@ -561,6 +562,31 @@
         self.assertEqual('', index.prefix)
 
 
+class TestConfigOptionIndex(TestHelp):
+    """Tests for the HelpCommandIndex class."""
+
+    def setUp(self):
+        super(TestConfigOptionIndex, self).setUp()
+        self.index = help_topics.ConfigOptionHelpIndex()
+
+    def test_get_topics_None(self):
+        """Searching for None returns an empty list."""
+        self.assertEqual([], self.index.get_topics(None))
+
+    def test_get_topics_no_topic(self):
+        self.assertEqual([], self.index.get_topics('nothing by this name'))
+
+    def test_prefix(self):
+        self.assertEqual('configuration/', self.index.prefix)
+
+    def test_get_topic_with_prefix(self):
+        topics = self.index.get_topics('configuration/default_format')
+        self.assertLength(1, topics)
+        opt = topics[0]
+        self.assertIsInstance(opt, config.Option)
+        self.assertEquals('default_format', opt.name)
+
+
 class TestCommandIndex(TestHelp):
     """Tests for the HelpCommandIndex class."""
 
@@ -603,16 +629,19 @@
     def test_default_search_path(self):
         """The default search path should include internal indexs."""
         indices = help.HelpIndices()
-        self.assertEqual(3, len(indices.search_path))
+        self.assertEqual(4, len(indices.search_path))
         # help topics should be searched in first.
         self.assertIsInstance(indices.search_path[0],
-            help_topics.HelpTopicIndex)
+                              help_topics.HelpTopicIndex)
         # with commands being search second.
         self.assertIsInstance(indices.search_path[1],
-            commands.HelpCommandIndex)
-        # and plugins are a third index.
+                              commands.HelpCommandIndex)
+        # plugins are a third index.
         self.assertIsInstance(indices.search_path[2],
-            plugin.PluginsHelpIndex)
+                              plugin.PluginsHelpIndex)
+        # config options are a fourth index
+        self.assertIsInstance(indices.search_path[3],
+                              help_topics.ConfigOptionHelpIndex)
 
     def test_search_for_unknown_topic_raises(self):
         """Searching for an unknown topic should raise NoHelpTopic."""

=== modified file 'bzrlib/tests/test_plugins.py'
--- bzrlib/tests/test_plugins.py	2011-05-16 13:49:58 +0000
+++ bzrlib/tests/test_plugins.py	2011-08-16 07:35:26 +0000
@@ -615,15 +615,16 @@
     def test_get_help_text_with_additional_see_also(self):
         mod = FakeModule('two lines of help\nand more', 'demo')
         topic = plugin.ModuleHelpTopic(mod)
-        self.assertEqual("two lines of help\nand more\nSee also: bar, foo\n",
-            topic.get_help_text(['foo', 'bar']))
+        self.assertEqual("two lines of help\nand more\n\n:See also: bar, foo\n",
+                         topic.get_help_text(['foo', 'bar']))
 
     def test_get_help_topic(self):
         """The help topic for a plugin is its module name."""
         mod = FakeModule('two lines of help\nand more', 'bzrlib.plugins.demo')
         topic = plugin.ModuleHelpTopic(mod)
         self.assertEqual('demo', topic.get_help_topic())
-        mod = FakeModule('two lines of help\nand more', 'bzrlib.plugins.foo_bar')
+        mod = FakeModule('two lines of help\nand more',
+                         'bzrlib.plugins.foo_bar')
         topic = plugin.ModuleHelpTopic(mod)
         self.assertEqual('foo_bar', topic.get_help_topic())
 

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-08-15 13:49:38 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-08-16 07:35:26 +0000
@@ -69,6 +69,9 @@
   while --match-message, --match-author, --match-committer and
   --match-bugs match each of those fields.
 
+* ``bzr help configuration/<option>`` display the help for ``option`` for
+  all registered configuration options. (Vincent Ladeuil, #747050)
+
 * Relative local paths can now be specified in URL syntax by using the
   "file:" prefix.  (Jelmer Vernooij)