Add support for multiple output formats

Three are provided:

- table (default, same as what we currently have)
- simple (less markup than default but still legible)
- csv (comma-separated output)

This can be configured via git-config, an environment option, or a
command line argument.

Signed-off-by: Stephen Finucane <stephen@that.guru>

Author: stephenfin

git_pw/bundle.py

@@ -6,7 +6,6 @@
 import sys
 
 import click
-from tabulate import tabulate
 
 from git_pw import api
 from git_pw import utils
@@ -82,8 +81,9 @@ def download_cmd(bundle_id, output):
 
 
 @click.command(name='show')
+@utils.format_options
 @click.argument('bundle_id')
-def show_cmd(bundle_id):
+def show_cmd(fmt, bundle_id):
     """Show information about bundle.
 
     Retrieve Patchwork metadata for a bundle.
@@ -108,8 +108,7 @@ def _format_patch(patch):
         output.append((prefix, _format_patch(patch)))
         prefix = ''
 
-    # TODO(stephenfin): We might want to make this machine readable?
-    click.echo(tabulate(output, ['Property', 'Value'], tablefmt='psql'))
+    utils.echo(output, ['Property', 'Value'], fmt=fmt)
 
 
 @click.command(name='list')
@@ -124,9 +123,10 @@ def _format_patch(patch):
 @click.option('--sort', metavar='FIELD', default='name', type=click.Choice(
                   ['id', '-id', 'name', '-name']),
               help='Sort output on given field.')
+@utils.format_options
 @click.argument('name', required=False)
 @api.validate_multiple_filter_support
-def list_cmd(owner, limit, page, sort, name):
+def list_cmd(owner, limit, page, sort, fmt, name):
     """List bundles.
 
     List bundles on the Patchwork instance.
@@ -163,4 +163,4 @@ def list_cmd(owner, limit, page, sort, name):
         'yes' if bundle.get('public') else 'no',
     ] for bundle in bundles]
 
-    utils.echo_via_pager(tabulate(output, headers, tablefmt='psql'))
+    utils.echo_via_pager(output, headers, fmt=fmt)

git_pw/patch.py

@@ -7,7 +7,6 @@
 
 import arrow
 import click
-from tabulate import tabulate
 
 from git_pw import api
 from git_pw import utils
@@ -88,7 +87,7 @@ def download_cmd(patch_id, output, fmt):
         LOG.info('Downloaded patch to %s', path)
 
 
-def _show_patch(patch):
+def _show_patch(patch, fmt):
 
     def _format_series(series):
         return '%-4d %s' % (series.get('id'), series.get('name') or '-')
@@ -113,13 +112,13 @@ def _format_series(series):
         output.append((prefix, _format_series(series)))
         prefix = ''
 
-    # TODO(stephenfin): We might want to make this machine readable?
-    click.echo(tabulate(output, ['Property', 'Value'], tablefmt='psql'))
+    utils.echo(output, ['Property', 'Value'], fmt=fmt)
 
 
 @click.command(name='show')
+@utils.format_options
 @click.argument('patch_id', type=click.INT)
-def show_cmd(patch_id):
+def show_cmd(fmt, patch_id):
     """Show information about patch.
 
     Retrieve Patchwork metadata for a patch.
@@ -128,7 +127,7 @@ def show_cmd(patch_id):
 
     patch = api.detail('patches', patch_id)
 
-    _show_patch(patch)
+    _show_patch(patch, fmt)
 
 
 @click.command(name='update')
@@ -143,7 +142,8 @@ def show_cmd(patch_id):
               'either a username or a user\'s email address.')
 @click.option('--archived', metavar='ARCHIVED', type=click.BOOL,
               help='Set the patch archived state.')
-def update_cmd(patch_ids, commit_ref, state, delegate, archived):
+@utils.format_options
+def update_cmd(patch_ids, commit_ref, state, delegate, archived, fmt):
     """Update one or more patches.
 
     Updates one or more Patches on the Patchwork instance. Some operations may
@@ -174,7 +174,7 @@ def update_cmd(patch_ids, commit_ref, state, delegate, archived):
 
         patch = api.update('patches', patch_id, data)
 
-        _show_patch(patch)
+        _show_patch(patch, fmt)
 
 
 @click.command(name='list')
@@ -199,9 +199,11 @@ def update_cmd(patch_ids, commit_ref, state, delegate, archived):
 @click.option('--sort', metavar='FIELD', default='-date', type=click.Choice(
                   ['id', '-id', 'name', '-name', 'date', '-date']),
               help='Sort output on given field.')
+@utils.format_options
 @click.argument('name', required=False)
 @api.validate_multiple_filter_support
-def list_cmd(state, submitter, delegate, archived, limit, page, sort, name):
+def list_cmd(state, submitter, delegate, archived, limit, page, sort, fmt,
+             name):
     """List patches.
 
     List patches on the Patchwork instance.
@@ -256,4 +258,4 @@ def list_cmd(state, submitter, delegate, archived, limit, page, sort, name):
          if patch.get('delegate') else ''),
     ] for patch in patches]
 
-    utils.echo_via_pager(tabulate(output, headers, tablefmt='psql'))
+    utils.echo_via_pager(output, headers, fmt=fmt)

git_pw/series.py

@@ -7,7 +7,6 @@
 
 import arrow
 import click
-from tabulate import tabulate
 
 from git_pw import api
 from git_pw import utils
@@ -62,8 +61,9 @@ def download_cmd(series_id, output):
 
 
 @click.command(name='show')
+@utils.format_options
 @click.argument('series_id', type=click.INT)
-def show_cmd(series_id):
+def show_cmd(fmt, series_id):
     """Show information about series.
 
     Retrieve Patchwork metadata for a series.
@@ -95,8 +95,7 @@ def _format_submission(submission):
         output.append((prefix, _format_submission(patch)))
         prefix = ''
 
-    # TODO(stephenfin): We might want to make this machine readable?
-    click.echo(tabulate(output, ['Property', 'Value'], tablefmt='psql'))
+    utils.echo(output, ['Property', 'Value'], fmt=fmt)
 
 
 @click.command(name='list')
@@ -111,9 +110,10 @@ def _format_submission(submission):
 @click.option('--sort', metavar='FIELD', default='-date', type=click.Choice(
                   ['id', '-id', 'name', '-name', 'date', '-date']),
               help='Sort output on given field.')
+@utils.format_options
 @click.argument('name', required=False)
 @api.validate_multiple_filter_support
-def list_cmd(submitter, limit, page, sort, name):
+def list_cmd(submitter, limit, page, sort, fmt, name):
     """List series.
 
     List series on the Patchwork instance.
@@ -152,4 +152,4 @@ def list_cmd(submitter, limit, page, sort, name):
                      series_.get('submitter').get('email'))
     ] for series_ in series]
 
-    utils.echo_via_pager(tabulate(output, headers, tablefmt='psql'))
+    utils.echo_via_pager(output, headers, fmt=fmt)

git_pw/utils.py

@@ -7,6 +7,10 @@
 import subprocess
 import sys
 
+import click
+from tabulate import tabulate
+
+
 if sys.version_info < (3, 0):
     _text = unicode  # noqa
 else:
@@ -48,6 +52,25 @@ def git_am(mbox, args):
         sys.exit(exc.returncode)
 
 
+def _tabulate(output, headers, fmt):
+    fmt = fmt or git_config('pw.format') or 'table'
+
+    if fmt == 'table':
+        return tabulate(output, headers, tablefmt='psql')
+    elif fmt == 'simple':
+        return tabulate(output, headers, tablefmt='simple')
+    elif fmt == 'csv':
+        result = []
+        result.append(','.join(headers))
+        for item in output:
+            result.append(
+                ','.join(_text(x if x is not None else '') for x in item))
+        return '\n'.join(result)
+
+    print('pw.format must be one of: table, simple, csv')
+    sys.exit(1)
+
+
 def _is_ascii_encoding(encoding):
     """Checks if a given encoding is ascii."""
     try:
@@ -92,7 +115,7 @@ def _echo_via_pager(pager, output):
             break
 
 
-def echo_via_pager(output):
+def echo_via_pager(output, headers, fmt):
     """Echo using git's default pager.
 
     Wrap ``click.echo_via_pager``, setting some environment variables in the
@@ -102,6 +125,8 @@ def echo_via_pager(output):
         then ``core.pager`` configuration, then ``$PAGER``, and then the
         default chosen at compile time (usually ``less``).
     """
+    output = _tabulate(output, headers, fmt)
+
     pager = os.environ.get('GIT_PAGER', None)
     if pager:
         _echo_via_pager(pager, output)
@@ -118,3 +143,17 @@ def echo_via_pager(output):
         return
 
     _echo_via_pager('less', output)
+
+
+def echo(output, headers, fmt):
+    click.echo(_tabulate(output, headers, fmt))
+
+
+def format_options(f):
+    """Shared output format options."""
+    f = click.option('--format', '-f', 'fmt', envvar='PW_FORMAT',
+                     default=None,
+                     type=click.Choice(['simple', 'table', 'csv']),
+                     help='Output format. Defaults to table.')(f)
+
+    return f

releasenotes/notes/issue-29-884269fdf35f64b2.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    Many commands now take a ``--format``/``-f`` parameter, which can be used
+    to control the output format. Three formats are currently supported:
+
+    - ``table`` (default)
+    - ``simple`` (a version of table with less markup)
+    - ``csv`` (comma-separated output)

tests/test_bundle.py

@@ -176,9 +176,9 @@ def test_show(self, mock_get_bundle):
         mock_get_bundle.assert_called_once_with('123')
 
 
-@mock.patch('git_pw.utils.echo_via_pager', new=mock.Mock)
 @mock.patch('git_pw.api.version', return_value=(1, 0))
 @mock.patch('git_pw.api.index')
+@mock.patch('git_pw.utils.echo_via_pager')
 class ListTestCase(unittest.TestCase):
 
     @staticmethod
@@ -194,7 +194,7 @@ def _get_users(**kwargs):
         rsp.update(**kwargs)
         return rsp
 
-    def test_list(self, mock_index, mock_version):
+    def test_list(self, mock_echo, mock_index, mock_version):
         """Validate standard behavior."""
 
         rsp = [self._get_bundle()]
@@ -208,7 +208,22 @@ def test_list(self, mock_index, mock_version):
             ('q', None), ('page', None), ('per_page', None),
             ('order', 'name')])
 
-    def test_list_with_filters(self, mock_index, mock_version):
+    def test_list_with_formatting(self, mock_echo, mock_index, mock_version):
+        """Validate behavior with formatting applied."""
+
+        rsp = [self._get_bundle()]
+        mock_index.return_value = rsp
+
+        runner = CLIRunner()
+        result = runner.invoke(bundle.list_cmd, [
+            '--format', 'simple'])
+
+        assert result.exit_code == 0, result
+
+        mock_echo.assert_called_once_with(mock.ANY, mock.ANY,
+                                          fmt='simple')
+
+    def test_list_with_filters(self, mock_echo, mock_index, mock_version):
         """Validate behavior with filters applied.
 
         Apply all filters, including those for pagination.
@@ -233,7 +248,7 @@ def test_list_with_filters(self, mock_index, mock_version):
         mock_index.assert_has_calls(calls)
 
     @mock.patch('git_pw.api.LOG')
-    def test_list_with_wildcard_filters(self, mock_log, mock_index,
+    def test_list_with_wildcard_filters(self, mock_log, mock_echo, mock_index,
                                         mock_version):
         """Validate behavior with a "wildcard" filter.
 
@@ -251,7 +266,7 @@ def test_list_with_wildcard_filters(self, mock_log, mock_index,
         assert mock_log.warning.called
 
     @mock.patch('git_pw.api.LOG')
-    def test_list_with_multiple_filters(self, mock_log, mock_index,
+    def test_list_with_multiple_filters(self, mock_log, mock_echo, mock_index,
                                         mock_version):
         """Validate behavior with use of multiple filters.
 
@@ -271,7 +286,8 @@ def test_list_with_multiple_filters(self, mock_log, mock_index,
         assert mock_log.warning.called
 
     @mock.patch('git_pw.api.LOG')
-    def test_list_api_v1_1(self, mock_log, mock_index, mock_version):
+    def test_list_api_v1_1(self, mock_log, mock_echo, mock_index,
+                           mock_version):
         """Validate behavior with API v1.1."""
 
         mock_version.return_value = (1, 1)