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

Hi Ævar,

On Tue, 5 Jan 2021 at 14:32, Ævar Arnfjörð Bjarmason [off-list ref] 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@company.xx>

[off-list ref]" >> internal_mailmap/.mailmap &&
        echo "Some Dude [off-list ref]         nick1
[off-list ref]" >> internal_mailmap/.mailmap &&
+       echo "# Comment [off-list ref]         nick1 [off-list ref]"

quoted

internal_mailmap/.mailmap &&

        echo "Other Author [off-list ref]   nick2
[off-list ref]" >> internal_mailmap/.mailmap &&
        echo "Other Author [off-list ref]
[off-list ref]" >> internal_mailmap/.mailmap &&
        echo "Santa Claus [off-list ref] [off-list ref]"

quoted

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