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

Tessa Starkey testarkey at gmail.com
Tue Feb 2 22:09:52 CST 2010


Thanks for the comments. Responses inline.

On Tue, Feb 2, 2010 at 6:20 PM, Greg Ward <greg-hg at gerg.ca> wrote:

> 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?
>
>
    The reason that glob_exclude is a separate command is to distinguish
between entries that use the characters '*' and '?' as wildcards and file
names that actually contain these characters. On the off chance that someone
actually names a file '*.py', they can exclude that file without exlcuding
all .py files.



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


> >       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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20100202/1f81af02/attachment.htm>


More information about the Mercurial-devel mailing list