Re: [PATCH v3 1/3] Add tests for describe with --work-tree

From: Jeff King <peff@peff.net>
Date: Jan 30, 2019, 12:43 UTC
Message-ID: <20190130124331.GA2273@sigill.intra.peff.net>
In-Reply-To: <CA+xP2Sb1--LQJs+GXqKVE3FjNHujSn6Q-Ow=-LEHtRLcFJJMGA@mail.gmail.com>

On Wed, Jan 30, 2019 at 11:23:14AM +0100, Sebastian Staudt wrote:

Show 11 quoted lines

> > >> +    grep 'A-\d\+-g[0-9a-f]\+' '$TRASH_DIRECTORY/out'
> > >
> > > Using "\d" isn't portable.
> >
> > True, but not just \d.  I think using \ before special characters to
> > force an otherwise basic regular expression to be ERE (i.e. \+ at
> > the end) is a GNUism.
> >
> 
> I guess I’ll use the even broader but apparently more portable A-*[0-9a-f]
> then. It’s used in the other checks, so this should be OK?

Yeah, that is OK, as long as you use it with `case` like check_describe does. Because it's a glob and not a regex, you do not have to worry about the anchoring issue.

Or if you mean to do an anchored but more general regex like:

  ^A-.*[0-9a-f]$

that is OK, too.

Show 8 quoted lines

> > > If that's indeed what we're checking, then an easier check is perhaps:
> > >
> > >   ! grep dirty ...
> >
> > Good.
> 
> This was copied and pasted from the existing check for describe with a
> clean working tree. So this should be changed, too.

I do think that makes what we're checking more obvious, so I wouldn't mind seeing the other tests converted to use this. But it may be hard if they are relying on check_describe().

I'm OK with anything that works and is robust. :)

-Peff