[PATCH 1 of 3] record: refactor the record extension's prompt()

[Patrick Mézard]

Le 05/01/10 01:44, Steve Losh a écrit :
> # HG changeset patch
> # User Steve Losh <steve at stevelosh.com>
> # Date 1262652127 18000
> # Node ID 1fa39ad1ca75699329f538aae321fcdcfbb5dfda
> # Parent  f5e55f1ca92738458295ca9f82f31c54a89babbb
> record: refactor the record extension's prompt()
> 
> This changeset refactors record.filterpatch.prompt() to be more flexible when
> defining options.

I know you are not looking for formal patch review, but it is usually easier to fix style issues sooner than later, so here are a couple of remarks:

> diff --git a/hgext/record.py b/hgext/record.py
> --- a/hgext/record.py
> +++ b/hgext/record.py
> @@ -284,34 +284,53 @@
>              return resp_file[0]
>          while True:
>              resps = _('[Ynsfdaq?]')
> -            choices = (_('&Yes, record this change'),
> -                    _('&No, skip this change'),
> -                    _('&Skip remaining changes to this file'),
> -                    _('Record remaining changes to this &file'),
> -                    _('&Done, skip remaining changes and files'),
> -                    _('Record &all changes to all remaining files'),
> -                    _('&Quit, recording no changes'),
> -                    _('&?'))
> +            all_choices = {

No underscores please

http://mercurial.selenic.com/wiki/BasicCodingStyle

> +                'yes': _('&Yes, record this change'),
> +                'no': _('&No, skip this change'),
> +                'skip': _('&Skip remaining changes to this file'),
> +                'file': _('Record remaining changes to this &file'),
> +                'done': _('&Done, skip remaining changes and files'),
> +                'all': _('Record &all changes to all remaining files'),
> +                'quit': _('&Quit, recording no changes'),
> +                'help': _('&?'),
> +            }
> +            choices = (all_choices['yes'],
> +                    all_choices['no'],
> +                    all_choices['skip'],
> +                    all_choices['file'],
> +                    all_choices['done'],
> +                    all_choices['all'],
> +                    all_choices['quit'],
> +                    all_choices['help'],)

I think you can build a tuple list first for ordering and build the dictionary from it.

>              r = ui.promptchoice("%s %s" % (query, resps), choices)
> -            if r == 7: # ?
> +            choice = [k for k in all_choices if all_choices[k] == choices[r]][0]
> +            if choice == 'help':
>                  doc = gettext(record.__doc__)
>                  c = doc.find(_('y - record this change'))
> -                for l in doc[c:].splitlines():
> +                doc_lines = doc[c:].splitlines()
> +                if not at_hunk:
> +                    doc_lines = [l for l in doc_lines if not l.startswith('p - split')]
> +                else:
> +                    doc_lines = filter(
> +                        lambda l: l.replace('(if you are currently at a hunk)', ''),
> +                        doc_lines
> +                    )

We never indent that way. What about:

docfilter = lambda l: l.replace('(if you are currently at a hunk)', '')
doc_lines = filter(docfilter, doc_lines)

?

Also, "p - split this hunk (if you are currently at a hunk)" is likely to be translated, so the prefix has to be defined differently.

--
Patrick Mézard