Re: [PATCH] shortlog: remove unused(?) "repo-abbrev" feature

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ævar,

On Tue, 5 Jan 2021 at 14:32, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>  {
>         char *name1 = NULL, *email1 = NULL, *name2 = NULL, *email2 = NULL;
> -       if (buffer[0] == '#') {
> -               static const char abbrev[] = "# repo-abbrev:";
> -               int abblen = sizeof(abbrev) - 1;
> -               int len = strlen(buffer);
>
> -               if (!repo_abbrev)
> -                       return;
> -
> -               if (len && buffer[len - 1] == '\n')
> -                       buffer[--len] = 0;
> -               if (!strncmp(buffer, abbrev, abblen)) {
> -                       char *cp;
> -
> -                       free(*repo_abbrev);
> -
> -                       for (cp = buffer + abblen; isspace(*cp); cp++)
> -                               ; /* nothing */
> -                       *repo_abbrev = xstrdup(cp);
> -               }
> -               return;
> -       }
>         if ((name2 = parse_name_and_email(buffer, &name1, &email1, 0)) != NULL)
>                 parse_name_and_email(name2, &name2, &email2, 1);

I think this is a tiny bit too aggressive -- it stops recognizing and
skipping comments. For example, this whitespace-damaged diff:

diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 586c3a86b1..4d461ad343 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -373,6 +373,7 @@ test_expect_success 'Shortlog output (complex mapping)' '
        echo "Committed <$GIT_COMMITTER_EMAIL>" > internal_mailmap/.mailmap &&
        echo "<cto@xxxxxxxxxx>
<cto@xxxxxxxxxxx>" >> internal_mailmap/.mailmap &&
        echo "Some Dude <some@xxxxxxx>         nick1
<bugs@xxxxxxxxxx>" >> internal_mailmap/.mailmap &&
+       echo "# Comment <no@xxxxxxx>         nick1 <bugs@xxxxxxxxxx>"
>> internal_mailmap/.mailmap &&
        echo "Other Author <other@xxxxxxxxx>   nick2
<bugs@xxxxxxxxxx>" >> internal_mailmap/.mailmap &&
        echo "Other Author <other@xxxxxxxxx>
<nick2@xxxxxxxxxx>" >> internal_mailmap/.mailmap &&
        echo "Santa Claus <santa.claus@xxxxxxxxxxxx> <me@xxxxxxxxxx>"
>> internal_mailmap/.mailmap &&

... which passes before this, makes the test fail after this patch. It
seems our test coverage for comments is basically zero here. It might
make sense to first introduce some testing around comments (maybe not in
this "complex mapping" test, though) before doing this patch you're
posting here, but keeping something like

       if (buffer[0] == '#')
               return;

Martin




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux