[PATCH 1 of 2] convert: Support glob exclude patterns

[Greg Ward]

On Tue, Feb 2, 2010 at 2:07 PM, Tessa Starkey <testarkey at gmail.com> wrote:
> # HG changeset patch
> # User Tessa Starkey <testarkey at gmail.com>
> # Date 1265132627 18000
> # Node ID 052ab4a8d877c59c398a7ed2f52644387dc56d9b
> # Parent  d9aa5b368e36c10d2c29411772fef9fd339c2e9f
> convert: Support glob exclude patterns

That sounds useful.

> --- a/hgext/convert/__init__.py
> +++ b/hgext/convert/__init__.py
> @@ -84,15 +84,19 @@
>
>       exclude path/to/file
>
> +      glob_exclude path/to/file
> +

Why does this require a new command?  Is there something wrong with
supporting globs in the existing 'exclude' command?

Also, I'm pretty sure someone (whose name might be Matt) will object
to the underscore.

>       rename from/file to/file
>
> -    The 'include' directive causes a file, or all files under a
> -    directory, to be included in the destination repository, and the
> -    exclusion of all other files and directories not explicitly
> -    included. The 'exclude' directive causes files or directories to
> -    be omitted. The 'rename' directive renames a file or directory. To
> -    rename from a subdirectory into the root of the repository, use
> -    '.' as the path to rename to.
> +    The 'include' directive causes a file, or all files under
> +    a directory, to be included in the destination repository,
> +    and the exclusion of all other files and directories not
> +    explicitly included. The 'exclude' directive causes files
> +    or directories to be omitted. The 'glob_exclude' directive also
> +    causes files or directories to be omitted and allows the use of
> +    glob wildcards '*' and '?'.The 'rename' directive renames a file
> +    or directory. To rename from a subdirectory into the root of the
> +    repository, use '.' as the path to rename to.

Careful: you seem to have wrapped the text to a slightly narrower line
length, which makes the patch larger than it needs to be and therefore
harder to review.

>  def rpairs(name):
> +    """ yields all suffix-prefix pairs of the given file path"""
>     e = len(name)
>     while e != -1:
> -        yield name[:e], name[e + 1:]
> +        yield name[:e], name[e+1:]

Please avoid making cosmetic changes at the same time as adding
functionality.  As above, it makes the patch longer and harder to
review.

The actual change itself seems OK to me, although I'm not very
familiar with the code in question and have not tested your patch.

Thanks for your submission!

Greg