Re: [PATCH] t2400: Fix test failures when using grep 2.5

From: Jacob Abel <jacobabel@nullpo.dev>
Date: Jul 18, 2023, 00:44 UTC
Message-ID: <dyzkftugvd5b4f4wxsg6773fkrdrnbync6idvvi6h7cuuto36w@dbzjnkj3mh2l>
In-Reply-To: <3f3a3f5b-70fd-ec3f-acbb-d585b5eb6cbc@gmail.com>

On 23/07/16 04:34PM, Phillip Wood wrote:

Show 7 quoted lines

> Hi Jacob
> 
> [...]
> 
> One thing I forgot to mention was that I think it would be better to
> explain in the commit message that "\s" etc. are not part of POSIX EREs
> and that is why they do not work.

Noted. Will do.

> [...]
> 
> Oh so we need to search for a space followed by a tab after "hint:"
> then.

Okay. I think `\t` is PCRE so I'll just update the string in `builtin/worktree.c` so we can just do `[ ]+` instead.

> As an aside we often just use four spaces to indent commands in
> advice messages (see the output of git -C .. grep '"    git' \*.c)

Apologies. When writing up that original patchset I based the formatting of the advice based on the ones in `builtin/add.c` which seems to also use `\t`.

Show 6 quoted lines

> 
> > So I just went with `[[:space:]]+` as I
> > didn't want to have to worry about whether some platforms expand the
> > tab to spaces or how many spaces.
> 
> Is that a thing?

It might be? I know copying text through tmux tends to expand tabs to spaces for me so I figured some other tools or those same tools on different platforms might do things like that as well. To be honest I have no idea and figured that I'd just CYA by making it work in the case that it did than trying to guarantee that it wouldn't happen.

Show 10 quoted lines

> > [...]
> >
> > If there is documentation (or discussions) on how to use this backend
> > properly I'd appreciate a link and I can try workshopping a better
> > solution then. The warning included in the original patchset reads
> > from that HEAD file as well so it would also need to be adapted.
> 
> I'm afraid I don't have anything specific, there were some patches a
> while ago such as dd8468ef00 (t5601: read HEAD using rev-parse,
> 2021-05-31) that stopped reading HEAD from the filesystem.

Noted.

Show 7 quoted lines

> > [...]
> 
> I think it might be better to just diagnose if HEAD is a dangling
> symbolic-ref or contains an invalid oid and leave it at that. See the
> documentation in refs.h for refs_resolve_ref_unsafe() for how to check
> if HEAD is a dangling symbolic ref - if rego_get_oid(repo, "HEAD") fails
> and it is not a dangling symbolic ref then it contains an invalid oid.

Understood. I'll start working on a separate patch to update that warning once this patch settles then.

> 
> [...]