Re: [PATCH v3] nmbug: Translate to Python

[David Bremner <bremner@debian.org>]

"W. Trevor King" <wking@tremily.us> writes:


> * Commands are no longer split into "most common", "other useful", and
>   "less common" sets.  If we need something like this, I'd prefer
>   workflow examples highlighting common commands in the module
>   docstring (available with 'nmbug --help').
>

I don't feel strongly about this, but I remember implementing it by
request in the first version. OTOH, I think you shortened up the main
help string when you split it.  We may want to think about a seperate
man page as a follow project.

One thing I did notice is that there is no hint to call 
nmbug {command} --help in the main docstring.


> +#!/usr/bin/env python
> +# Copyright (c) 2011 David Bremner
> +# License: same as notmuch

You should add your self, update the date, and probably explicitly 
state the license, as in Carl's patch for nmbug-status.


> +__version__ = '0.2'

Do we need/want a version distinct from that of notmuch?


> +def _hex_quote(string, safe='+@=:,'):

I'm not sure I really understand what makes a function/variable
"private" and hence prefixed with _ in your translation.


> +    status, tree, stderr = _git(

as a non-native speaker of python, I find this a bit hard to read.  How
about adding some parens to make the multiple return more clear, so

    (status, tree, stderr) = _git(


> +        for id, tags in status['deleted'].items():
same comment here.

> +
> +def merge(reference='@{upstream}'):
> +    """

I did notice that merging was noticably noisier than I remembered. 


> +    output = _collections.defaultdict(
> +        lambda : _collections.defaultdict( # {tag: status_string}
> +            lambda : ' '))  # default local status

The initial comment is confusing (to me) because it looks like code.
The two layers of defaultdict are a bit confusing too; it would help to
mention the key of the outer dictionary.

I guess it makes sense to go for a relatively straight translation as a
first step; I did wonder about whether using the python bindings to
notmuch would speed things up.  Any ideas about how to even figure out
where the bottlenecks are?

d