* [PATCH 0/2] Add mailmap support to ref-filter
@ 2023-09-20 19:05 Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 1/2] t/t6300: introduce test_bad_atom() Kousik Sanagavarapu
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu
Add mailmap support to ref-filter, making ref-filter and pretty closer, which
is a part of the effort made to unify both ref-filter and pretty.
PATCH 1/2 - Introduces the "test_bad_atom()" function which checks for if the
given error message (either due to "err_bad_arg()" or any other err)
is correct.
PATCH 2/2 - The actual mailmap support.
Kousik Sanagavarapu (2):
t/t6300: introduce test_bad_atom()
ref-filter: add mailmap support
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 103 +++++++++++++++++++
3 files changed, 225 insertions(+), 36 deletions(-)
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] t/t6300: introduce test_bad_atom()
2023-09-20 19:05 [PATCH 0/2] Add mailmap support to ref-filter Kousik Sanagavarapu
@ 2023-09-20 19:05 ` Kousik Sanagavarapu
2023-09-20 22:56 ` Junio C Hamano
2023-09-20 19:05 ` [PATCH 2/2] ref-filter: add mailmap support Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
2 siblings, 1 reply; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma
Introduce a new function "test_bad_atom()", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
Like "test_atom()", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..15b4622f57 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
+test_bad_atom() {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
+ printf '%s\n' "$3">expect
+ test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
+ test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
+ test_cmp expect actual
+ "
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] ref-filter: add mailmap support
2023-09-20 19:05 [PATCH 0/2] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 1/2] t/t6300: introduce test_bad_atom() Kousik Sanagavarapu
@ 2023-09-20 19:05 ` Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-20 19:05 UTC (permalink / raw)
To: git; +Cc: Kousik Sanagavarapu, Christian Couder, Hariom Verma
Add mailmap support to ref-filter formats which are similar in
pretty. This support is such that the following pretty placeholders are
equivalent to the new ref-filter atoms:
%aN = authorname:mailmap
%cN = committername:mailmap
%aE = authoremail:mailmap
%aL = authoremail:mailmap,localpart
%cE = committeremail:mailmap
%cL = committeremail:mailmap,localpart
Additionally, mailmap can also be used with ":trim" option for email by
doing something like "authoremail:mailmap,trim".
The above also applies for the "tagger" atom, that is,
"taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim"
and "taggername:mailmap,localpart".
The functionality of ":trim" and ":localpart" remains the same. That is,
":trim" gives the email, but without the angle brackets and ":localpart"
gives the part of the email before the '@' character (if such a
character is not found then we directly grab everything between the
angle brackets).
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 83 ++++++++++++++++
3 files changed, 205 insertions(+), 36 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
and `date` to extract the named component. For email fields (`authoremail`,
`committeremail` and `taggeremail`), `:trim` can be appended to get the email
without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
The raw data in an object is `raw`.
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
#include "oid-array.h"
#include "repository.h"
#include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
#include "remote.h"
#include "color.h"
#include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
struct {
enum { O_SIZE, O_SIZE_DISK } option;
} objectsize;
- struct email_option {
- enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+ struct {
+ enum { N_RAW, N_MAILMAP } option;
+ } name_option;
+ struct {
+ enum {
+ EO_RAW = 0,
+ EO_TRIM = 1<<0,
+ EO_LOCALPART = 1<<1,
+ EO_MAILMAP = 1<<2,
+ } option;
} email_option;
struct {
enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int person_email_atom_parser(struct ref_format *format UNUSED,
- struct used_atom *atom,
- const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
{
if (!arg)
- atom->u.email_option.option = EO_RAW;
- else if (!strcmp(arg, "trim"))
- atom->u.email_option.option = EO_TRIM;
- else if (!strcmp(arg, "localpart"))
- atom->u.email_option.option = EO_LOCALPART;
+ atom->u.name_option.option = N_RAW;
+ else if (!strcmp(arg, "mailmap"))
+ atom->u.name_option.option = N_MAILMAP;
else
return err_bad_arg(err, atom->name, arg);
return 0;
}
+static int email_atom_option_parser(struct used_atom *atom,
+ const char **arg, struct strbuf *err)
+{
+ if (!*arg)
+ return EO_RAW;
+ if (skip_prefix(*arg, "trim", arg))
+ return EO_TRIM;
+ if (skip_prefix(*arg, "localpart", arg))
+ return EO_LOCALPART;
+ if (skip_prefix(*arg, "mailmap", arg))
+ return EO_MAILMAP;
+ return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
+{
+ for (;;) {
+ int opt = email_atom_option_parser(atom, &arg, err);
+ const char *bad_arg = arg;
+
+ if (opt < 0)
+ return err_bad_arg(err, atom->name, bad_arg);
+ atom->u.email_option.option |= opt;
+
+ if (!arg || !*arg)
+ break;
+ if (*arg == ',')
+ arg++;
+ else
+ return err_bad_arg(err, atom->name, bad_arg);
+ }
+ return 0;
+}
+
static int refname_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom,
const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
[ATOM_TYPE] = { "type", SOURCE_OBJ },
[ATOM_TAG] = { "tag", SOURCE_OBJ },
[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
- [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+ [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
- [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+ [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
- [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+ [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
return xstrdup("");
}
+static const char *find_end_of_email(const char *email, int opt)
+{
+ const char *eoemail;
+
+ if (opt & EO_LOCALPART) {
+ eoemail = strchr(email, '@');
+ if (eoemail)
+ return eoemail;
+ return strchr(email, '>');
+ }
+
+ if (opt & EO_TRIM)
+ return strchr(email, '>');
+
+ /*
+ * The option here is either the raw email option or the raw
+ * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+ * we directly grab the whole email including the closing
+ * angle brackets.
+ *
+ * If EO_MAILMAP was set with any other option (that is either
+ * EO_TRIM or EO_LOCALPART), we already grab the end of email
+ * above.
+ */
+ eoemail = strchr(email, '>');
+ if (eoemail)
+ eoemail++;
+ return eoemail;
+}
+
static const char *copy_email(const char *buf, struct used_atom *atom)
{
const char *email = strchr(buf, '<');
const char *eoemail;
+ int opt = atom->u.email_option.option;
+
if (!email)
return xstrdup("");
- switch (atom->u.email_option.option) {
- case EO_RAW:
- eoemail = strchr(email, '>');
- if (eoemail)
- eoemail++;
- break;
- case EO_TRIM:
- email++;
- eoemail = strchr(email, '>');
- break;
- case EO_LOCALPART:
+
+ if (opt & (EO_LOCALPART | EO_TRIM))
email++;
- eoemail = strchr(email, '@');
- if (!eoemail)
- eoemail = strchr(email, '>');
- break;
- default:
- BUG("unknown email option");
- }
+ eoemail = find_end_of_email(email, opt);
if (!eoemail)
return xstrdup("");
return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
v->value = 0;
}
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
/* See grab_values */
static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
{
int i;
int wholen = strlen(who);
const char *wholine = NULL;
+ const char *headers[] = { "author ", "committer ",
+ "tagger ", NULL };
for (i = 0; i < used_atom_cnt; i++) {
- const char *name = used_atom[i].name;
+ struct used_atom *atom = &used_atom[i];
+ const char *name = atom->name;
struct atom_value *v = &val[i];
+ struct strbuf mailmap_buf = STRBUF_INIT;
+
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
if (strncmp(who, name, wholen))
continue;
if (name[wholen] != 0 &&
- strcmp(name + wholen, "name") &&
+ !starts_with(name + wholen, "name") &&
!starts_with(name + wholen, "email") &&
!starts_with(name + wholen, "date"))
continue;
- if (!wholine)
+
+ if ((starts_with(name + wholen, "name") &&
+ (atom->u.name_option.option == N_MAILMAP)) ||
+ (starts_with(name + wholen, "email") &&
+ (atom->u.email_option.option & EO_MAILMAP))) {
+ if (!mailmap.items)
+ read_mailmap(&mailmap);
+ strbuf_addstr(&mailmap_buf, buf);
+ apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+ wholine = find_wholine(who, wholen, mailmap_buf.buf);
+ } else {
wholine = find_wholine(who, wholen, buf);
+ }
+
if (!wholine)
return; /* no point looking for it */
if (name[wholen] == 0)
v->s = copy_line(wholine);
- else if (!strcmp(name + wholen, "name"))
+ else if (starts_with(name + wholen, "name"))
v->s = copy_name(wholine);
else if (starts_with(name + wholen, "email"))
v->s = copy_email(wholine, &used_atom[i]);
else if (starts_with(name + wholen, "date"))
grab_date(wholine, v, name);
+
+ strbuf_release(&mailmap_buf);
}
/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 15b4622f57..1fb464ca50 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
disklen sha1:138
disklen sha256:154
EOF
+
+ # setup .mailmap
+ cat >.mailmap <<-EOF &&
+ A Thor <athor@example.com> A U Thor <author@example.com>
+ C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+ EOF
+
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
@@ -141,15 +148,31 @@ test_atom head '*objectname' ''
test_atom head '*objecttype' ''
test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
test_atom head authoremail '<author@example.com>'
test_atom head authoremail:trim 'author@example.com'
test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
test_atom head committeremail '<committer@example.com>'
test_atom head committeremail:trim 'committer@example.com'
test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
test_atom head tag ''
test_atom head tagger ''
@@ -199,22 +222,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
test_atom tag '*objecttype' 'commit'
test_atom tag author ''
test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
test_atom tag authoremail ''
test_atom tag authoremail:trim ''
test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
test_atom tag authordate ''
test_atom tag committer ''
test_atom tag committername ''
+test_atom tag committername:mailmap ''
test_atom tag committeremail ''
test_atom tag committeremail:trim ''
test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
test_atom tag committerdate ''
test_atom tag tag 'testtag'
test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
test_atom tag taggeremail '<committer@example.com>'
test_atom tag taggeremail:trim 'committer@example.com'
test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -284,9 +331,45 @@ test_bad_atom() {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+ 'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+ 'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+ 'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail: trim,foo' \
+ 'fatal: unrecognized %(committeremail) argument: trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+ 'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+ 'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+ 'fatal: unrecognized %(taggeremail) argument: foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+ 'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
'fatal: unrecognized %(taggeremail) argument: trim'
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.160.g6905eb16ce.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()
2023-09-20 19:05 ` [PATCH 1/2] t/t6300: introduce test_bad_atom() Kousik Sanagavarapu
@ 2023-09-20 22:56 ` Junio C Hamano
2023-09-20 23:09 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Junio C Hamano @ 2023-09-20 22:56 UTC (permalink / raw)
To: Kousik Sanagavarapu; +Cc: git, Christian Couder, Hariom Verma
Kousik Sanagavarapu <five231003@gmail.com> writes:
> Introduce a new function "test_bad_atom()", which is similar to
> "test_atom()" but should be used to check whether the correct error
> message is shown on stderr.
>
> Like "test_atom()", the new function takes three arguments. The three
> arguments specify the ref, the format and the expected error message
> respectively, with an optional fourth argument for tweaking
> "test_expect_*" (which is by default "success").
>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Hariom Verma <hariom18599@gmail.com>
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---
> t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 7b943fd34c..15b4622f57 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
> test_must_fail git for-each-ref --format="%(objectname:short=foo)"
> '
>
> +test_bad_atom() {
Style: have SP on both sides of "()".
> + case "$1" in
> + head) ref=refs/heads/main ;;
> + tag) ref=refs/tags/testtag ;;
> + sym) ref=refs/heads/sym ;;
> + *) ref=$1 ;;
> + esac
Somehow this indirection makes the two examples we see below harder
to understand. Why shouldn't we just write the full refname on th
command line of test_bad_atom? That would make it crystal clear
which ref each test works on. It does not help that both 'head' and
'sym' refer to a local branch (if the former referred to .git/HEAD
or .git/remotes/origin/HEAD it may have been an excellent choice of
the name, but that is not what is going on).
> + printf '%s\n' "$3">expect
Style: need SP before (but not after) '>'.
> + test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
> + test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
> + test_cmp expect actual
> + "
It is error prone to have the executable part of test_expect_{success,failure}
inside a pair of double quotes and have $variable interpolated
_before_ even the arguments to test_expect_{success,failure} are
formed. It is much more preferrable to write
test_bad_atom () {
ref=$1 format=$2
printf '%s\n' "$3" >expect
$test_do=test_expect_${4:-success}
$test_do $PREREQ "err basic atom: $ref $format" '
test_must_fail git for-each-ref \
--format="%($format)" "$ref" 2>error &&
test_cmp expect error
'
}
This is primarily because you cannot control what is in "$2" to
ensure the correctness of the test we see above locally (i.e. if
your caller has a single-quote in "$2", the shell script you create
for running test_expect_{success,failure} would be syntactically
incorrect). By enclosing the executable part inside a pair of
single quotes, and having the $variables interpolated when that
executable part is `eval`ed when test_expect_{success,failure} runs,
you will avoid such problems, and those reading the above can locally
know that you are aware of and correctly avoiding such problems.
I guess three among four problems I just pointed out you blindly
copied from test_atom. But let's not spread badness (preliminary
clean-up to existing badness would be welcome instead).
> +}
> +
> +test_bad_atom head 'authoremail:foo' \
> + 'fatal: unrecognized %(authoremail) argument: foo'
> +
> +test_bad_atom tag 'taggeremail:localpart trim' \
> + 'fatal: unrecognized %(taggeremail) argument: trim'
It is strange to see double SP before 'trim' in this error message.
Are we etching a code mistake in stone here? Wouldn't the error
message say "...argument: localpart trim" instead, perhaps?
> test_date () {
> f=$1 &&
> committer_date=$2 &&
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()
2023-09-20 22:56 ` Junio C Hamano
@ 2023-09-20 23:09 ` Junio C Hamano
2023-09-20 23:21 ` Junio C Hamano
2023-09-21 18:57 ` Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2023-09-20 23:09 UTC (permalink / raw)
To: Kousik Sanagavarapu; +Cc: git, Christian Couder, Hariom Verma
Junio C Hamano <gitster@pobox.com> writes:
>> + case "$1" in
>> + head) ref=refs/heads/main ;;
>> + tag) ref=refs/tags/testtag ;;
>> + sym) ref=refs/heads/sym ;;
>> + *) ref=$1 ;;
>> + esac
>
> Somehow this indirection makes the two examples we see below harder
> to understand. ... It does not help that both 'head' and
> 'sym' refer to a local branch ...
Ah, this "sym" thing is a (rather unnatural) symbolic ref inside
refs/heads/ hierarchy, so the naming makes halfway sense. As it is
also used in test_atom, I no longer find it all that much disturbing.
Everything else I said in my review still stands, I would think.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()
2023-09-20 22:56 ` Junio C Hamano
2023-09-20 23:09 ` Junio C Hamano
@ 2023-09-20 23:21 ` Junio C Hamano
2023-09-21 18:57 ` Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2023-09-20 23:21 UTC (permalink / raw)
To: Kousik Sanagavarapu; +Cc: git, Christian Couder, Hariom Verma
Junio C Hamano <gitster@pobox.com> writes:
>> +test_bad_atom tag 'taggeremail:localpart trim' \
>> + 'fatal: unrecognized %(taggeremail) argument: trim'
>
> It is strange to see double SP before 'trim' in this error message.
> Are we etching a code mistake in stone here? Wouldn't the error
> message say "...argument: localpart trim" instead, perhaps?
I tried. The fatal message does say ...argument: localpart trim" as
I suspected, when you ask for 'taggeremail:localpart trim'.
I think I know what is going on. With the [PATCH 1/2] as-is, this
piece does not pass. But because the error message from parsing
gets broken by [PATCH 2/2], after applying [PATCH 2/2], the error
message will become what the above test expects, hiding the new
breakage in the code. And it probably was not noticed before you
sent the patches, because you did not test [PATCH 1/2] alone.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] t/t6300: introduce test_bad_atom()
2023-09-20 22:56 ` Junio C Hamano
2023-09-20 23:09 ` Junio C Hamano
2023-09-20 23:21 ` Junio C Hamano
@ 2023-09-21 18:57 ` Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-21 18:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Christian Couder, Hariom Verma
Sorry for the late reply.
On Wed, Sep 20, 2023 at 03:56:28PM -0700, Junio C Hamano wrote:
> Kousik Sanagavarapu <five231003@gmail.com> writes:
>
> > Introduce a new function "test_bad_atom()", which is similar to
> > "test_atom()" but should be used to check whether the correct error
> > message is shown on stderr.
> >
> > Like "test_atom()", the new function takes three arguments. The three
> > arguments specify the ref, the format and the expected error message
> > respectively, with an optional fourth argument for tweaking
> > "test_expect_*" (which is by default "success").
> >
> > Mentored-by: Christian Couder <christian.couder@gmail.com>
> > Mentored-by: Hariom Verma <hariom18599@gmail.com>
> > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> > ---
> > t/t6300-for-each-ref.sh | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> > index 7b943fd34c..15b4622f57 100755
> > --- a/t/t6300-for-each-ref.sh
> > +++ b/t/t6300-for-each-ref.sh
> > @@ -267,6 +267,26 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
> > test_must_fail git for-each-ref --format="%(objectname:short=foo)"
> > '
> >
> > +test_bad_atom() {
>
> Style: have SP on both sides of "()".
>
> [...]
>
> > + printf '%s\n' "$3">expect
>
> Style: need SP before (but not after) '>'.
I'll make these style changes, they slipped by.
> > + test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
> > + test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
> > + test_cmp expect actual
> > + "
>
> It is error prone to have the executable part of test_expect_{success,failure}
> inside a pair of double quotes and have $variable interpolated
> _before_ even the arguments to test_expect_{success,failure} are
> formed. It is much more preferrable to write
>
> test_bad_atom () {
> ref=$1 format=$2
> printf '%s\n' "$3" >expect
> $test_do=test_expect_${4:-success}
>
> $test_do $PREREQ "err basic atom: $ref $format" '
> test_must_fail git for-each-ref \
> --format="%($format)" "$ref" 2>error &&
> test_cmp expect error
> '
> }
>
> This is primarily because you cannot control what is in "$2" to
> ensure the correctness of the test we see above locally (i.e. if
> your caller has a single-quote in "$2", the shell script you create
> for running test_expect_{success,failure} would be syntactically
> incorrect). By enclosing the executable part inside a pair of
> single quotes, and having the $variables interpolated when that
> executable part is `eval`ed when test_expect_{success,failure} runs,
> you will avoid such problems, and those reading the above can locally
> know that you are aware of and correctly avoiding such problems.
I see.
> I guess three among four problems I just pointed out you blindly
> copied from test_atom. But let's not spread badness (preliminary
> clean-up to existing badness would be welcome instead).
Yeah, I had copied it from test_atom. Although I didn't realize that it
was bad to implement test_bad_atom the way I did. Thanks for such a nice
explanation. So I guess we can include the test_atom cleanup in this
series?
> > +}
> > +
> > +test_bad_atom head 'authoremail:foo' \
> > + 'fatal: unrecognized %(authoremail) argument: foo'
> > +
> > +test_bad_atom tag 'taggeremail:localpart trim' \
> > + 'fatal: unrecognized %(taggeremail) argument: trim'
>
> It is strange to see double SP before 'trim' in this error message.
> Are we etching a code mistake in stone here? Wouldn't the error
> message say "...argument: localpart trim" instead, perhaps?
>
> > test_date () {
> > f=$1 &&
> > committer_date=$2 &&
So I read the the other replies and it seems that it indeed hides a
breakage and yeah I hadn't tested PATCH 1/2 independently. I'll change
this too.
Thanks
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/3] Add mailmap support to ref-filter
2023-09-20 19:05 [PATCH 0/2] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 1/2] t/t6300: introduce test_bad_atom() Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 2/2] ref-filter: add mailmap support Kousik Sanagavarapu
@ 2023-09-25 17:43 ` Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 1/3] t/t6300: cleanup test_atom Kousik Sanagavarapu
` (2 more replies)
2 siblings, 3 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu
Thanks Junio for review on the previous round.
PATCH 1/3 - Cleanup test_atom to be less error-prone
PATCH 2/3 - Fix hidden breakage
PATCH 3/3 - Unchanged
Kousik Sanagavarapu (3):
t/t6300: cleanup test_atom
t/t6300: introduce test_bad_atom
ref-filter: add mailmap support
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 123 +++++++++++++++++++++--
3 files changed, 239 insertions(+), 42 deletions(-)
Range-diff against v1:
-: ---------- > 1: b28e858e35 t/t6300: cleanup test_atom
1: 0a90b67889 ! 2: ee90d017d5 t/t6300: introduce test_bad_atom()
@@ Metadata
Author: Kousik Sanagavarapu <five231003@gmail.com>
## Commit message ##
- t/t6300: introduce test_bad_atom()
+ t/t6300: introduce test_bad_atom
- Introduce a new function "test_bad_atom()", which is similar to
+ Introduce a new function "test_bad_atom", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
- Like "test_atom()", the new function takes three arguments. The three
+ Like "test_atom", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
@@ t/t6300-for-each-ref.sh: test_expect_success 'arguments to %(objectname:short=)
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
-+test_bad_atom() {
++test_bad_atom () {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
-+ printf '%s\n' "$3">expect
-+ test_expect_${4:-success} $PREREQ "err basic atom: $1 $2" "
-+ test_must_fail git for-each-ref --format='%($2)' $ref 2>actual &&
-+ test_cmp expect actual
-+ "
++ format=$2
++ test_do=test_expect_${4:-success}
++
++ printf '%s\n' "$3" >expect
++ $test_do $PREREQ "err basic atom: $ref $format" '
++ test_must_fail git for-each-ref \
++ --format="%($format)" "$ref" 2>error &&
++ test_cmp expect error
++ '
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
-+ 'fatal: unrecognized %(taggeremail) argument: trim'
++ 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+
test_date () {
f=$1 &&
2: 63fc69f4dc ! 3: fdc14fe80b ref-filter: add mailmap support
@@ t/t6300-for-each-ref.sh: test_atom tag '*objectname' $(git rev-parse refs/tags/t
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
-@@ t/t6300-for-each-ref.sh: test_bad_atom() {
+@@ t/t6300-for-each-ref.sh: test_bad_atom () {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
@@ t/t6300-for-each-ref.sh: test_bad_atom() {
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
- 'fatal: unrecognized %(taggeremail) argument: trim'
-
+- 'fatal: unrecognized %(taggeremail) argument: localpart trim'
++ 'fatal: unrecognized %(taggeremail) argument: trim'
++
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
-+
+
test_date () {
f=$1 &&
- committer_date=$2 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/3] t/t6300: cleanup test_atom
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
@ 2023-09-25 17:43 ` Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 2/3] t/t6300: introduce test_bad_atom Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 3/3] ref-filter: add mailmap support Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
Previously, when the executable part of "test_expect_{success,failure}"
(inside "test_atom") got "eval"ed, it would have been syntactically
incorrect if the second argument ($2, which is the format) to "test_atom"
were enclosed in single quotes because the $variables would get
interpolated even before the arguments to "test_expect_{success,failure}"
are formed.
So fix this and also some style issues along the way.
Helped-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7b943fd34c..7ba9949376 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -41,25 +41,29 @@ test_expect_success setup '
git config push.default current
'
-test_atom() {
+test_atom () {
case "$1" in
head) ref=refs/heads/main ;;
tag) ref=refs/tags/testtag ;;
sym) ref=refs/heads/sym ;;
*) ref=$1 ;;
esac
+ format=$2
+ test_do=test_expect_${4:-success}
+
printf '%s\n' "$3" >expected
- test_expect_${4:-success} $PREREQ "basic atom: $1 $2" "
- git for-each-ref --format='%($2)' $ref >actual &&
+ $test_do $PREREQ "basic atom: $ref $format" '
+ git for-each-ref --format="%($format)" "$ref" >actual &&
sanitize_pgp <actual >actual.clean &&
test_cmp expected actual.clean
- "
+ '
+
# Automatically test "contents:size" atom after testing "contents"
- if test "$2" = "contents"
+ if test "$format" = "contents"
then
# for commit leg, $3 is changed there
expect=$(printf '%s' "$3" | wc -c)
- test_expect_${4:-success} $PREREQ "basic atom: $1 contents:size" '
+ $test_do $PREREQ "basic atom: $ref contents:size" '
type=$(git cat-file -t "$ref") &&
case $type in
tag)
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] t/t6300: introduce test_bad_atom
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 1/3] t/t6300: cleanup test_atom Kousik Sanagavarapu
@ 2023-09-25 17:43 ` Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 3/3] ref-filter: add mailmap support Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
Introduce a new function "test_bad_atom", which is similar to
"test_atom()" but should be used to check whether the correct error
message is shown on stderr.
Like "test_atom", the new function takes three arguments. The three
arguments specify the ref, the format and the expected error message
respectively, with an optional fourth argument for tweaking
"test_expect_*" (which is by default "success").
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
t/t6300-for-each-ref.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 7ba9949376..e4ec2926d6 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -271,6 +271,30 @@ test_expect_success 'arguments to %(objectname:short=) must be positive integers
test_must_fail git for-each-ref --format="%(objectname:short=foo)"
'
+test_bad_atom () {
+ case "$1" in
+ head) ref=refs/heads/main ;;
+ tag) ref=refs/tags/testtag ;;
+ sym) ref=refs/heads/sym ;;
+ *) ref=$1 ;;
+ esac
+ format=$2
+ test_do=test_expect_${4:-success}
+
+ printf '%s\n' "$3" >expect
+ $test_do $PREREQ "err basic atom: $ref $format" '
+ test_must_fail git for-each-ref \
+ --format="%($format)" "$ref" 2>error &&
+ test_cmp expect error
+ '
+}
+
+test_bad_atom head 'authoremail:foo' \
+ 'fatal: unrecognized %(authoremail) argument: foo'
+
+test_bad_atom tag 'taggeremail:localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+
test_date () {
f=$1 &&
committer_date=$2 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] ref-filter: add mailmap support
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 1/3] t/t6300: cleanup test_atom Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 2/3] t/t6300: introduce test_bad_atom Kousik Sanagavarapu
@ 2023-09-25 17:43 ` Kousik Sanagavarapu
2 siblings, 0 replies; 11+ messages in thread
From: Kousik Sanagavarapu @ 2023-09-25 17:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Kousik Sanagavarapu, Christian Couder,
Hariom Verma
Add mailmap support to ref-filter formats which are similar in
pretty. This support is such that the following pretty placeholders are
equivalent to the new ref-filter atoms:
%aN = authorname:mailmap
%cN = committername:mailmap
%aE = authoremail:mailmap
%aL = authoremail:mailmap,localpart
%cE = committeremail:mailmap
%cL = committeremail:mailmap,localpart
Additionally, mailmap can also be used with ":trim" option for email by
doing something like "authoremail:mailmap,trim".
The above also applies for the "tagger" atom, that is,
"taggername:mailmap", "taggeremail:mailmap", "taggeremail:mailmap,trim"
and "taggername:mailmap,localpart".
The functionality of ":trim" and ":localpart" remains the same. That is,
":trim" gives the email, but without the angle brackets and ":localpart"
gives the part of the email before the '@' character (if such a
character is not found then we directly grab everything between the
angle brackets).
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Hariom Verma <hariom18599@gmail.com>
Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Documentation/git-for-each-ref.txt | 6 +-
ref-filter.c | 152 ++++++++++++++++++++++-------
t/t6300-for-each-ref.sh | 85 +++++++++++++++-
3 files changed, 206 insertions(+), 37 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 11b2bc3121..e86d5700dd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -303,7 +303,11 @@ Fields that have name-email-date tuple as its value (`author`,
and `date` to extract the named component. For email fields (`authoremail`,
`committeremail` and `taggeremail`), `:trim` can be appended to get the email
without angle brackets, and `:localpart` to get the part before the `@` symbol
-out of the trimmed email.
+out of the trimmed email. In addition to these, the `:mailmap` option and the
+corresponding `:mailmap,trim` and `:mailmap,localpart` can be used (order does
+not matter) to get values of the name and email according to the .mailmap file
+or according to the file set in the mailmap.file or mailmap.blob configuration
+variable (see linkgit:gitmailmap[5]).
The raw data in an object is `raw`.
diff --git a/ref-filter.c b/ref-filter.c
index fae9f4b8ed..e4d3510e28 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,8 @@
#include "oid-array.h"
#include "repository.h"
#include "commit.h"
+#include "mailmap.h"
+#include "ident.h"
#include "remote.h"
#include "color.h"
#include "tag.h"
@@ -215,8 +217,16 @@ static struct used_atom {
struct {
enum { O_SIZE, O_SIZE_DISK } option;
} objectsize;
- struct email_option {
- enum { EO_RAW, EO_TRIM, EO_LOCALPART } option;
+ struct {
+ enum { N_RAW, N_MAILMAP } option;
+ } name_option;
+ struct {
+ enum {
+ EO_RAW = 0,
+ EO_TRIM = 1<<0,
+ EO_LOCALPART = 1<<1,
+ EO_MAILMAP = 1<<2,
+ } option;
} email_option;
struct {
enum { S_BARE, S_GRADE, S_SIGNER, S_KEY,
@@ -720,21 +730,55 @@ static int oid_atom_parser(struct ref_format *format UNUSED,
return 0;
}
-static int person_email_atom_parser(struct ref_format *format UNUSED,
- struct used_atom *atom,
- const char *arg, struct strbuf *err)
+static int person_name_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
{
if (!arg)
- atom->u.email_option.option = EO_RAW;
- else if (!strcmp(arg, "trim"))
- atom->u.email_option.option = EO_TRIM;
- else if (!strcmp(arg, "localpart"))
- atom->u.email_option.option = EO_LOCALPART;
+ atom->u.name_option.option = N_RAW;
+ else if (!strcmp(arg, "mailmap"))
+ atom->u.name_option.option = N_MAILMAP;
else
return err_bad_arg(err, atom->name, arg);
return 0;
}
+static int email_atom_option_parser(struct used_atom *atom,
+ const char **arg, struct strbuf *err)
+{
+ if (!*arg)
+ return EO_RAW;
+ if (skip_prefix(*arg, "trim", arg))
+ return EO_TRIM;
+ if (skip_prefix(*arg, "localpart", arg))
+ return EO_LOCALPART;
+ if (skip_prefix(*arg, "mailmap", arg))
+ return EO_MAILMAP;
+ return -1;
+}
+
+static int person_email_atom_parser(struct ref_format *format UNUSED,
+ struct used_atom *atom,
+ const char *arg, struct strbuf *err)
+{
+ for (;;) {
+ int opt = email_atom_option_parser(atom, &arg, err);
+ const char *bad_arg = arg;
+
+ if (opt < 0)
+ return err_bad_arg(err, atom->name, bad_arg);
+ atom->u.email_option.option |= opt;
+
+ if (!arg || !*arg)
+ break;
+ if (*arg == ',')
+ arg++;
+ else
+ return err_bad_arg(err, atom->name, bad_arg);
+ }
+ return 0;
+}
+
static int refname_atom_parser(struct ref_format *format UNUSED,
struct used_atom *atom,
const char *arg, struct strbuf *err)
@@ -877,15 +921,15 @@ static struct {
[ATOM_TYPE] = { "type", SOURCE_OBJ },
[ATOM_TAG] = { "tag", SOURCE_OBJ },
[ATOM_AUTHOR] = { "author", SOURCE_OBJ },
- [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ },
+ [ATOM_AUTHORNAME] = { "authorname", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_AUTHOREMAIL] = { "authoremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_AUTHORDATE] = { "authordate", SOURCE_OBJ, FIELD_TIME },
[ATOM_COMMITTER] = { "committer", SOURCE_OBJ },
- [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ },
+ [ATOM_COMMITTERNAME] = { "committername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_COMMITTEREMAIL] = { "committeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_COMMITTERDATE] = { "committerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_TAGGER] = { "tagger", SOURCE_OBJ },
- [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ },
+ [ATOM_TAGGERNAME] = { "taggername", SOURCE_OBJ, FIELD_STR, person_name_atom_parser },
[ATOM_TAGGEREMAIL] = { "taggeremail", SOURCE_OBJ, FIELD_STR, person_email_atom_parser },
[ATOM_TAGGERDATE] = { "taggerdate", SOURCE_OBJ, FIELD_TIME },
[ATOM_CREATOR] = { "creator", SOURCE_OBJ },
@@ -1486,32 +1530,49 @@ static const char *copy_name(const char *buf)
return xstrdup("");
}
+static const char *find_end_of_email(const char *email, int opt)
+{
+ const char *eoemail;
+
+ if (opt & EO_LOCALPART) {
+ eoemail = strchr(email, '@');
+ if (eoemail)
+ return eoemail;
+ return strchr(email, '>');
+ }
+
+ if (opt & EO_TRIM)
+ return strchr(email, '>');
+
+ /*
+ * The option here is either the raw email option or the raw
+ * mailmap option (that is EO_RAW or EO_MAILMAP). In such cases,
+ * we directly grab the whole email including the closing
+ * angle brackets.
+ *
+ * If EO_MAILMAP was set with any other option (that is either
+ * EO_TRIM or EO_LOCALPART), we already grab the end of email
+ * above.
+ */
+ eoemail = strchr(email, '>');
+ if (eoemail)
+ eoemail++;
+ return eoemail;
+}
+
static const char *copy_email(const char *buf, struct used_atom *atom)
{
const char *email = strchr(buf, '<');
const char *eoemail;
+ int opt = atom->u.email_option.option;
+
if (!email)
return xstrdup("");
- switch (atom->u.email_option.option) {
- case EO_RAW:
- eoemail = strchr(email, '>');
- if (eoemail)
- eoemail++;
- break;
- case EO_TRIM:
- email++;
- eoemail = strchr(email, '>');
- break;
- case EO_LOCALPART:
+
+ if (opt & (EO_LOCALPART | EO_TRIM))
email++;
- eoemail = strchr(email, '@');
- if (!eoemail)
- eoemail = strchr(email, '>');
- break;
- default:
- BUG("unknown email option");
- }
+ eoemail = find_end_of_email(email, opt);
if (!eoemail)
return xstrdup("");
return xmemdupz(email, eoemail - email);
@@ -1572,16 +1633,23 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
v->value = 0;
}
+static struct string_list mailmap = STRING_LIST_INIT_NODUP;
+
/* See grab_values */
static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
{
int i;
int wholen = strlen(who);
const char *wholine = NULL;
+ const char *headers[] = { "author ", "committer ",
+ "tagger ", NULL };
for (i = 0; i < used_atom_cnt; i++) {
- const char *name = used_atom[i].name;
+ struct used_atom *atom = &used_atom[i];
+ const char *name = atom->name;
struct atom_value *v = &val[i];
+ struct strbuf mailmap_buf = STRBUF_INIT;
+
if (!!deref != (*name == '*'))
continue;
if (deref)
@@ -1589,22 +1657,36 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void
if (strncmp(who, name, wholen))
continue;
if (name[wholen] != 0 &&
- strcmp(name + wholen, "name") &&
+ !starts_with(name + wholen, "name") &&
!starts_with(name + wholen, "email") &&
!starts_with(name + wholen, "date"))
continue;
- if (!wholine)
+
+ if ((starts_with(name + wholen, "name") &&
+ (atom->u.name_option.option == N_MAILMAP)) ||
+ (starts_with(name + wholen, "email") &&
+ (atom->u.email_option.option & EO_MAILMAP))) {
+ if (!mailmap.items)
+ read_mailmap(&mailmap);
+ strbuf_addstr(&mailmap_buf, buf);
+ apply_mailmap_to_header(&mailmap_buf, headers, &mailmap);
+ wholine = find_wholine(who, wholen, mailmap_buf.buf);
+ } else {
wholine = find_wholine(who, wholen, buf);
+ }
+
if (!wholine)
return; /* no point looking for it */
if (name[wholen] == 0)
v->s = copy_line(wholine);
- else if (!strcmp(name + wholen, "name"))
+ else if (starts_with(name + wholen, "name"))
v->s = copy_name(wholine);
else if (starts_with(name + wholen, "email"))
v->s = copy_email(wholine, &used_atom[i]);
else if (starts_with(name + wholen, "date"))
grab_date(wholine, v, name);
+
+ strbuf_release(&mailmap_buf);
}
/*
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index e4ec2926d6..00a060df0b 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
disklen sha1:138
disklen sha256:154
EOF
+
+ # setup .mailmap
+ cat >.mailmap <<-EOF &&
+ A Thor <athor@example.com> A U Thor <author@example.com>
+ C Mitter <cmitter@example.com> C O Mitter <committer@example.com>
+ EOF
+
setdate_and_increment &&
echo "Using $datestamp" > one &&
git add one &&
@@ -145,15 +152,31 @@ test_atom head '*objectname' ''
test_atom head '*objecttype' ''
test_atom head author 'A U Thor <author@example.com> 1151968724 +0200'
test_atom head authorname 'A U Thor'
+test_atom head authorname:mailmap 'A Thor'
test_atom head authoremail '<author@example.com>'
test_atom head authoremail:trim 'author@example.com'
test_atom head authoremail:localpart 'author'
+test_atom head authoremail:trim,localpart 'author'
+test_atom head authoremail:mailmap '<athor@example.com>'
+test_atom head authoremail:mailmap,trim 'athor@example.com'
+test_atom head authoremail:trim,mailmap 'athor@example.com'
+test_atom head authoremail:mailmap,localpart 'athor'
+test_atom head authoremail:localpart,mailmap 'athor'
+test_atom head authoremail:mailmap,trim,localpart,mailmap,trim 'athor'
test_atom head authordate 'Tue Jul 4 01:18:44 2006 +0200'
test_atom head committer 'C O Mitter <committer@example.com> 1151968723 +0200'
test_atom head committername 'C O Mitter'
+test_atom head committername:mailmap 'C Mitter'
test_atom head committeremail '<committer@example.com>'
test_atom head committeremail:trim 'committer@example.com'
test_atom head committeremail:localpart 'committer'
+test_atom head committeremail:localpart,trim 'committer'
+test_atom head committeremail:mailmap '<cmitter@example.com>'
+test_atom head committeremail:mailmap,trim 'cmitter@example.com'
+test_atom head committeremail:trim,mailmap 'cmitter@example.com'
+test_atom head committeremail:mailmap,localpart 'cmitter'
+test_atom head committeremail:localpart,mailmap 'cmitter'
+test_atom head committeremail:trim,mailmap,trim,trim,localpart 'cmitter'
test_atom head committerdate 'Tue Jul 4 01:18:43 2006 +0200'
test_atom head tag ''
test_atom head tagger ''
@@ -203,22 +226,46 @@ test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
test_atom tag '*objecttype' 'commit'
test_atom tag author ''
test_atom tag authorname ''
+test_atom tag authorname:mailmap ''
test_atom tag authoremail ''
test_atom tag authoremail:trim ''
test_atom tag authoremail:localpart ''
+test_atom tag authoremail:trim,localpart ''
+test_atom tag authoremail:mailmap ''
+test_atom tag authoremail:mailmap,trim ''
+test_atom tag authoremail:trim,mailmap ''
+test_atom tag authoremail:mailmap,localpart ''
+test_atom tag authoremail:localpart,mailmap ''
+test_atom tag authoremail:mailmap,trim,localpart,mailmap,trim ''
test_atom tag authordate ''
test_atom tag committer ''
test_atom tag committername ''
+test_atom tag committername:mailmap ''
test_atom tag committeremail ''
test_atom tag committeremail:trim ''
test_atom tag committeremail:localpart ''
+test_atom tag committeremail:localpart,trim ''
+test_atom tag committeremail:mailmap ''
+test_atom tag committeremail:mailmap,trim ''
+test_atom tag committeremail:trim,mailmap ''
+test_atom tag committeremail:mailmap,localpart ''
+test_atom tag committeremail:localpart,mailmap ''
+test_atom tag committeremail:trim,mailmap,trim,trim,localpart ''
test_atom tag committerdate ''
test_atom tag tag 'testtag'
test_atom tag tagger 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag taggername 'C O Mitter'
+test_atom tag taggername:mailmap 'C Mitter'
test_atom tag taggeremail '<committer@example.com>'
test_atom tag taggeremail:trim 'committer@example.com'
test_atom tag taggeremail:localpart 'committer'
+test_atom tag taggeremail:trim,localpart 'committer'
+test_atom tag taggeremail:mailmap '<cmitter@example.com>'
+test_atom tag taggeremail:mailmap,trim 'cmitter@example.com'
+test_atom tag taggeremail:trim,mailmap 'cmitter@example.com'
+test_atom tag taggeremail:mailmap,localpart 'cmitter'
+test_atom tag taggeremail:localpart,mailmap 'cmitter'
+test_atom tag taggeremail:trim,mailmap,trim,localpart,localpart 'cmitter'
test_atom tag taggerdate 'Tue Jul 4 01:18:45 2006 +0200'
test_atom tag creator 'C O Mitter <committer@example.com> 1151968725 +0200'
test_atom tag creatordate 'Tue Jul 4 01:18:45 2006 +0200'
@@ -292,8 +339,44 @@ test_bad_atom () {
test_bad_atom head 'authoremail:foo' \
'fatal: unrecognized %(authoremail) argument: foo'
+test_bad_atom head 'authoremail:mailmap,trim,bar' \
+ 'fatal: unrecognized %(authoremail) argument: bar'
+
+test_bad_atom head 'authoremail:trim,' \
+ 'fatal: unrecognized %(authoremail) argument: '
+
+test_bad_atom head 'authoremail:mailmaptrim' \
+ 'fatal: unrecognized %(authoremail) argument: trim'
+
+test_bad_atom head 'committeremail: ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail: trim,foo' \
+ 'fatal: unrecognized %(committeremail) argument: trim,foo'
+
+test_bad_atom head 'committeremail:mailmap,localpart ' \
+ 'fatal: unrecognized %(committeremail) argument: '
+
+test_bad_atom head 'committeremail:trim_localpart' \
+ 'fatal: unrecognized %(committeremail) argument: _localpart'
+
+test_bad_atom head 'committeremail:localpart,,,trim' \
+ 'fatal: unrecognized %(committeremail) argument: ,,trim'
+
+test_bad_atom tag 'taggeremail:mailmap,trim, foo ' \
+ 'fatal: unrecognized %(taggeremail) argument: foo '
+
+test_bad_atom tag 'taggeremail:trim,localpart,' \
+ 'fatal: unrecognized %(taggeremail) argument: '
+
+test_bad_atom tag 'taggeremail:mailmap;localpart trim' \
+ 'fatal: unrecognized %(taggeremail) argument: ;localpart trim'
+
test_bad_atom tag 'taggeremail:localpart trim' \
- 'fatal: unrecognized %(taggeremail) argument: localpart trim'
+ 'fatal: unrecognized %(taggeremail) argument: trim'
+
+test_bad_atom tag 'taggeremail:mailmap,mailmap,trim,qux,localpart,trim' \
+ 'fatal: unrecognized %(taggeremail) argument: qux,localpart,trim'
test_date () {
f=$1 &&
--
2.42.0.273.ge948a9aaf4
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-25 17:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-20 19:05 [PATCH 0/2] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 1/2] t/t6300: introduce test_bad_atom() Kousik Sanagavarapu
2023-09-20 22:56 ` Junio C Hamano
2023-09-20 23:09 ` Junio C Hamano
2023-09-20 23:21 ` Junio C Hamano
2023-09-21 18:57 ` Kousik Sanagavarapu
2023-09-20 19:05 ` [PATCH 2/2] ref-filter: add mailmap support Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 0/3] Add mailmap support to ref-filter Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 1/3] t/t6300: cleanup test_atom Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 2/3] t/t6300: introduce test_bad_atom Kousik Sanagavarapu
2023-09-25 17:43 ` [PATCH v2 3/3] ref-filter: add mailmap support Kousik Sanagavarapu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).