From: Phillip Wood <phillip.wood123@gmail•com>
To: Jacob Abel <jacobabel@nullpo•dev>, phillip.wood@dunelm•org.uk
Cc: git@vger•kernel.org
Subject: Re: [PATCH] t2400: Fix test failures when using grep 2.5
Date: Tue, 18 Jul 2023 14:36:10 +0100 [thread overview]
Message-ID: <44671697-e9e6-d75a-30b9-7dccffcc792a@gmail.com> (raw)
In-Reply-To: <dyzkftugvd5b4f4wxsg6773fkrdrnbync6idvvi6h7cuuto36w@dbzjnkj3mh2l>
On 18/07/2023 01:44, Jacob Abel wrote:
> On 23/07/16 04:34PM, Phillip Wood wrote:
>> 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`.
The existing code is not consistent on this point but I think there are
more instances of " " than "\t". Using " " makes the indentation
consistent as the "hint: " prefix is translated so we don't know how far
the next tab stop will be from the end of the prefix.
>>
>>> 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.
In the test we are redirecting the output to a file so things like tmux
do not come into play. I think it would be a bit odd for the system libc
to convert tabs to spaces.
>>> [...]
>>
>> 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.
That's great. I think just telling the user something like
branch 'main' does not exist
when HEAD contains the dangling symbolic ref "refs/heads/main" and
HEAD is corrupt
when it is not a symbolic ref and repo_get_oid() fails would be fine.
Best Wishes
Phillip
next prev parent reply other threads:[~2023-07-18 13:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-15 2:55 [PATCH] t2400: Fix test failures when using grep 2.5 Jacob Abel
2023-07-15 8:59 ` Phillip Wood
2023-07-15 23:15 ` Jacob Abel
2023-07-16 1:08 ` Junio C Hamano
2023-07-16 2:55 ` Jacob Abel
2023-07-16 15:34 ` Phillip Wood
2023-07-17 2:38 ` Junio C Hamano
2023-07-18 0:44 ` Jacob Abel
2023-07-18 13:36 ` Phillip Wood [this message]
2023-07-21 4:35 ` Jacob Abel
2023-07-15 23:36 ` Jacob Abel
2023-07-16 3:38 ` [PATCH v2] " Jacob Abel
2023-07-21 4:40 ` [PATCH v3 0/3] " Jacob Abel
2023-07-21 4:40 ` [PATCH v3 1/3] t2400: drop no-op `--sq` from rev-parse call Jacob Abel
2023-07-21 4:40 ` [PATCH v3 2/3] builtin/worktree.c: convert tab in advice to space Jacob Abel
2023-07-21 4:40 ` [PATCH v3 3/3] t2400: rewrite regex to avoid unintentional PCRE Jacob Abel
2023-07-21 15:16 ` Junio C Hamano
2023-07-21 15:49 ` Junio C Hamano
2023-07-22 2:36 ` Jacob Abel
2023-07-26 21:42 ` [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5 Jacob Abel
2023-07-26 21:42 ` [PATCH v4 1/3] t2400: drop no-op `--sq` from rev-parse call Jacob Abel
2023-07-26 21:42 ` [PATCH v4 2/3] builtin/worktree.c: convert tab in advice to space Jacob Abel
2023-07-26 21:42 ` [PATCH v4 3/3] t2400: rewrite regex to avoid unintentional PCRE Jacob Abel
2023-07-26 22:09 ` [PATCH v4 0/3] t2400: Fix test failures when using grep 2.5 Junio C Hamano
2023-07-28 13:09 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=44671697-e9e6-d75a-30b9-7dccffcc792a@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=jacobabel@nullpo$(echo .)dev \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox