5 files changed, 162 insertions, 117 deletions

diff --git a/src/mailman/commands/cli_mailmanconf.py b/src/mailman/commands/cli_conf.py
index f29781473..1f8095d92 100644
--- a/src/mailman/commands/cli_mailmanconf.py
+++ b/src/mailman/commands/cli_conf.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2009-2013 by the Free Software Foundation, Inc.
+# Copyright (C) 2013 by the Free Software Foundation, Inc.
 #
 # This file is part of GNU Mailman.
 #
@@ -21,14 +21,15 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
-    'Mailmanconf'
+    'Conf'
     ]
 
 
 import sys
 
-from zope.interface import implementer
+from contextlib import closing
 from lazr.config._config import Section
+from zope.interface import implementer
 
 from mailman.config import config
 from mailman.core.i18n import _
@@ -37,92 +38,103 @@ from mailman.interfaces.command import ICLISubCommand
 
 
 @implementer(ICLISubCommand)
-class Mailmanconf:
+class Conf:
     """Print the mailman configuration."""
 
-    name = 'mailmanconf'
+    name = 'conf'
 
     def add(self, parser, command_parser):
-        self.parser = parser
         """See `ICLISubCommand`."""
+
+        self.parser = parser
         command_parser.add_argument(
             '-o', '--output',
             action='store', help=_("""\
-            File to send the output to.  If not given, standard output is
-            used."""))
+            File to send the output to.  If not given, or if '-' is given,
+            standard output is used."""))
         command_parser.add_argument(
             '-s', '--section',
             action='store', help=_("""\
-            Section to use for the lookup. If no key is given,
-            all the key-value pairs of the given section will be displayed.
+            Section to use for the lookup.  If no key is given, all the
+            key-value pairs of the given section will be displayed.
             """))
         command_parser.add_argument(
             '-k', '--key',
             action='store', help=_("""\
-            Key to use for the lookup. If no section is given,
-            all the key-values pair from any section matching the given key
-            will be displayed.
+            Key to use for the lookup.  If no section is given, all the
+            key-values pair from any section matching the given key will be
+            displayed.
             """))
 
     def _get_value(self, section, key):
         return getattr(getattr(config, section), key)
 
     def _print_full_syntax(self, section, key, value, output):
-        print('[{0}] {1}: {2}'.format(section, key, value), file=output)
+        print('[{}] {}: {}'.format(section, key, value), file=output)
 
     def _show_key_error(self, section, key):
-        self.parser.error('Section %s: No such key: %s' % (section, key))
+        self.parser.error('Section {}: No such key: {}'.format(section, key))
 
     def _show_section_error(self, section):
-        self.parser.error('No such section: %s' % section)
+        self.parser.error('No such section: {}'.format(section))
 
     def _print_values_for_section(self, section, output):
         current_section = getattr(config, section)
         for key in current_section:
-            if hasattr(current_section, key):
-                self._print_full_syntax(section, key, self._get_value(section, key), output)
+            self._print_full_syntax(section, key,
+                                    self._get_value(section, key), output)
 
     def _section_exists(self, section):
-        # not all the attributes in config are actual sections,
-        # so we have to additionally check a sections type
-        return hasattr(config, section) and isinstance(getattr(config, section), Section)
+        # Not all of the attributes in config are actual sections, so we have
+        # to check the section's type.
+        return (hasattr(config, section) and
+                isinstance(getattr(config, section), Section))
 
-    def process(self, args):
-        """See `ICLISubCommand`."""
-        if args.output is None:
-            output = sys.stdout
-        else:
-            # We don't need to close output because that will happen
-            # automatically when the script exits.
-            output = open(args.output, 'w')
+    def _inner_process(self, args, output):
+        # Process the command, ignoring the closing of the output file.
         section = args.section
         key = args.key
-        # Case 1: Both section and key are given, we can directly look up the value
+        # Case 1: Both section and key are given, so we can directly look up
+        # the value.
         if section is not None and key is not None:
             if not self._section_exists(section):
                 self._show_section_error(section)
             elif not hasattr(getattr(config, section), key):
                 self._show_key_error(section, key)
             else:
-                print(self._get_value(section, key))
-        # Case 2: Section is given, key is not given
+                print(self._get_value(section, key), file=output)
+        # Case 2: Section is given, key is not given.
         elif section is not None and key is None:
             if self._section_exists(section):
                 self._print_values_for_section(section, output)
             else:
                 self._show_section_error(section)
-        # Case 3: Section is not given, key is given
+        # Case 3: Section is not given, key is given.
         elif section is None and key is not None:
             for current_section in config.schema._section_schemas:
-                # We have to ensure that the current section actually exists and
-                # that it contains the given key
-                if self._section_exists(current_section) and hasattr(getattr(config, current_section), key):
-                    self._print_full_syntax(current_section, key, self._get_value(current_section, key), output)
-        # Case 4: Neither section nor key are given, 
-        # just display all the sections and their corresponding key/value pairs.
+                # We have to ensure that the current section actually exists
+                # and that it contains the given key.
+                if (self._section_exists(current_section) and
+                    hasattr(getattr(config, current_section), key)):
+                    # Then...
+                    self._print_full_syntax(
+                        current_section, key,
+                        self._get_value(current_section, key),
+                        output)
+        # Case 4: Neither section nor key are given, just display all the
+        # sections and their corresponding key/value pairs.
         elif section is None and key is None:
             for current_section in config.schema._section_schemas:
-                # However, we have to make sure that the current sections and key
-                # which are being looked up actually exist before trying to print them
+                # However, we have to make sure that the current sections and
+                # key which are being looked up actually exist before trying
+                # to print them.
                 if self._section_exists(current_section):
                     self._print_values_for_section(current_section, output)
+
+    def process(self, args):
+        """See `ICLISubCommand`."""
+        if args.output is None or args.output == '-':
+            self._inner_process(args, sys.stdout)
+        else:
+            with closing(open(args.output, 'w')) as output:
+                self._inner_process(args, output)
diff --git a/src/mailman/commands/docs/conf.rst b/src/mailman/commands/docs/conf.rst
new file mode 100644
index 000000000..6e458fb54
--- /dev/null
+++ b/src/mailman/commands/docs/conf.rst
@@ -0,0 +1,68 @@
+============================
+Display configuration values
+============================
+
+Just like the `Postfix command postconf(1)`_, the ``bin/mailman conf`` command
+lets you dump one or more Mailman configuration variables to standard output
+or a file.
+
+Mailman's configuration is divided in multiple sections which contain multiple
+key-value pairs.  The ``bin/mailman conf`` command allows you to display
+a specific key-value pair, or several key-value pairs.
+
+    >>> class FakeArgs:
+    ...     key = None
+    ...     section = None
+    ...     output = None
+    >>> from mailman.commands.cli_conf import Conf
+    >>> command = Conf()
+
+To get a list of all key-value pairs of any section, you need to call the
+command without any options.
+
+    >>> command.process(FakeArgs)
+    [logging.archiver] path: mailman.log
+    ...
+    [passwords] password_length: 8
+    ...
+    [mailman] site_owner: noreply@example.com
+    ...
+
+You can list all the key-value pairs of a specific section.
+
+    >>> FakeArgs.section = 'shell'
+    >>> command.process(FakeArgs)
+    [shell] use_ipython: no
+    [shell] banner: Welcome to the GNU Mailman shell
+    [shell] prompt: >>>
+
+You can also pass a key and display all key-value pairs matching the given
+key, along with the names of the corresponding sections.
+
+    >>> FakeArgs.section = None
+    >>> FakeArgs.key = 'path'
+    >>> command.process(FakeArgs)
+    [logging.archiver] path: mailman.log
+    [logging.locks] path: mailman.log
+    [logging.mischief] path: mailman.log
+    [logging.config] path: mailman.log
+    [logging.error] path: mailman.log
+    [logging.smtp] path: smtp.log
+    [logging.http] path: mailman.log
+    [logging.root] path: mailman.log
+    [logging.fromusenet] path: mailman.log
+    [logging.bounce] path: bounce.log
+    [logging.vette] path: mailman.log
+    [logging.runner] path: mailman.log
+    [logging.subscribe] path: mailman.log
+    [logging.debug] path: debug.log
+
+If you specify both a section and a key, you will get the corresponding value.
+
+    >>> FakeArgs.section = 'mailman'
+    >>> FakeArgs.key = 'site_owner'
+    >>> command.process(FakeArgs)
+    noreply@example.com
+
+
+.. _`Postfix command postconf(1)`: http://www.postfix.org/postconf.1.html
diff --git a/src/mailman/commands/docs/mailmanconf.rst b/src/mailman/commands/docs/mailmanconf.rst
deleted file mode 100644
index 061883bdf..000000000
--- a/src/mailman/commands/docs/mailmanconf.rst
+++ /dev/null
@@ -1,61 +0,0 @@
-==================
-Display configuration values
-==================
-
-Just like the postfix command postconf(1), mailmanconf lets you dump
-one or more mailman configuration variables. Internally, these can be
-retrieved by using the mailman.config.config object. Their structure
-is based on the schema given by src/mailman/config/schema.cfg.
-For more information on how the values are actually set, see
-src/mailman/docs/START.rst
-
-Basically, the configuration is divided in multiple sections which
-contain multiple key-value pairs. The ``bin/mailman mailmanconf``
-command allows you to display a specific or several key-value pairs.
-
-    >>> class FakeArgs:
-    ...     key = None
-    ...     section = None
-    ...     output = None
-    >>> from mailman.commands.cli_mailmanconf import Mailmanconf
-    >>> command = Mailmanconf()
-
-To get a list of all key-value pairs of any section, you need to call
-the command without any options.
-
-    >>> command.process(FakeArgs)
-    ... [logging.archiver] path: mailman.log
-    ... [logging.archiver] level: info
-    ... [logging.locks] propagate: no
-    ... [logging.locks] level: info
-    ... [passwords] configuration: python:mailman.config.passlib
-    ... etc.
-    
-You can list all the key-value pairs of a specific section.
-
-    >>> FakeArgs.section = 'mailman'
-    >>> command.process(FakeArgs)
-    ... [mailman] filtered_messages_are_preservable: no
-    ... [mailman] post_hook: 
-    ... [mailman] pending_request_life: 3d
-    ... etc.
-    
-You can also pass a key and display all key-value pairs matching
-the given key, along with the names of the corresponding sections.
-
-    >>> FakeArgs.section = 'None'
-    >>> FakeArgs.key = 'path'
-    >>> command.process(FakeArgs)
-    ... [logging.archiver] path: mailman.log
-    ... [logging.mischief] path: mailman.log
-    ... [logging.error] path: mailman.log
-    ... [logging.smtp] path: smtp.log
-    ... etc.
-    
-If you specify both a section and a key, you will get the corresponding value.
-
-    >>> FakeArgs.section = 'mailman'
-    >>> FakeArgs.key = 'site_owner'
-    >>> command.process(FakeArgs)
-    ... changeme@example.com
-    
\ No newline at end of file
diff --git a/src/mailman/commands/tests/test_mailmanconf.py b/src/mailman/commands/tests/test_conf.py
index 4a3b5df9e..bca7fe72f 100644
--- a/src/mailman/commands/tests/test_mailmanconf.py
+++ b/src/mailman/commands/tests/test_conf.py
@@ -1,4 +1,4 @@
-# Copyright (C) 2011-2013 by the Free Software Foundation, Inc.
+# Copyright (C) 2013 by the Free Software Foundation, Inc.
 #
 # This file is part of GNU Mailman.
 #
@@ -15,19 +15,25 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Test the mailmanconf subcommand."""
+"""Test the conf subcommand."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'TestConf',
     ]
 
+
+import os
 import sys
+import mock
+import tempfile
 import unittest
 
-from mailman.commands.cli_mailmanconf import Mailmanconf
-from mailman.config import config
+from mailman.commands.cli_conf import Conf
+from mailman.testing.layers import ConfigLayer
+
 
 
 class FakeArgs:
@@ -46,31 +52,50 @@ class FakeParser:
 
 
 
-class TestMailmanconf(unittest.TestCase):
-    """Test the mailmanconf subcommand."""
+class TestConf(unittest.TestCase):
+    """Test the conf subcommand."""
+
+    layer = ConfigLayer
 
     def setUp(self):
-        self.command = Mailmanconf()
+        self.command = Conf()
         self.command.parser = FakeParser()
         self.args = FakeArgs()
 
     def test_cannot_access_nonexistent_section(self):
         self.args.section = 'thissectiondoesnotexist'
         self.args.key = None
-        try:
+        with self.assertRaises(SystemExit):
             self.command.process(self.args)
-        except SystemExit:
-            pass
         self.assertEqual(self.command.parser.message,
                          'No such section: thissectiondoesnotexist')
 
     def test_cannot_access_nonexistent_key(self):
         self.args.section = "mailman"
         self.args.key = 'thiskeydoesnotexist'
-        try:
+        with self.assertRaises(SystemExit):
             self.command.process(self.args)
-        except SystemExit:
-            pass
         self.assertEqual(self.command.parser.message,
                          'Section mailman: No such key: thiskeydoesnotexist')
-        
\ No newline at end of file
+
+    def test_output_to_explicit_stdout(self):
+        self.args.output = '-'
+        self.args.section = 'shell'
+        self.args.key = 'use_ipython'
+        with mock.patch('sys.stdout') as mock_object:
+            self.command.process(self.args)
+        mock_object.write.assert_has_calls(
+            [mock.call('no'), mock.call('\n')])
+
+    def test_output_to_file(self):
+        self.args.section = 'shell'
+        self.args.key = 'use_ipython'
+        fd, filename = tempfile.mkstemp()
+        try:
+            self.args.output = filename
+            self.command.process(self.args)
+            with open(filename, 'r') as fp:
+                contents = fp.read()
+        finally:
+            os.remove(filename)
+        self.assertEqual(contents, 'no\n')
diff --git a/src/mailman/docs/ACKNOWLEDGMENTS.rst b/src/mailman/docs/ACKNOWLEDGMENTS.rst
index e0929e2c4..55a746ac7 100644
--- a/src/mailman/docs/ACKNOWLEDGMENTS.rst
+++ b/src/mailman/docs/ACKNOWLEDGMENTS.rst
@@ -246,6 +246,7 @@ left off the list!
 * Pasi Sjöholm
 * Chris Snell
 * Mikhail Sobolev
+* David Soto
 * Greg Stein
 * Dale Stimson
 * Students of HIT <mailman-cn@mail.cs.hit.edu.cn>