* [PATCH 0/1] add author and committer configuration settings @ 2018-12-19 18:39 William Hubbs 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs 2018-12-19 21:46 ` [PATCH 0/1] add " Jonathan Nieder 0 siblings, 2 replies; 6+ messages in thread From: William Hubbs @ 2018-12-19 18:39 UTC (permalink / raw) To: git; +Cc: williamh, chutzpah Hi all, this is my first patch for git, so please be gentle. ;-) I am in a situation where I need to use different authorship information for some repositories I commit to. Git already supports setting different authorship and committer information with environment variables, but this setting is global so if you want to change it per repository you need to use a separate tool. This patch adds support to git config for author.email, author.name, committer.email and committer.name settings so this information can be set per repository. It applies to current master. Thanks much for your reviews, and I would like to publically thank dscho from #git-devel for assisting me in preparing this patch. Also, since I use a screen reader, it would be very helpful if you put your comments in your replies as close to the affected code as possible, preferably directly below it. William Hubbs (1): Add author and committer configuration settings Documentation/config/user.txt | 20 +++++++++++ builtin/commit.c | 2 +- cache.h | 5 ++- config.c | 6 ++++ ident.c | 68 ++++++++++++++++++++++++++++++++--- log-tree.c | 3 +- sequencer.c | 3 +- 7 files changed, 97 insertions(+), 10 deletions(-) -- 2.19.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Add author and committer configuration settings 2018-12-19 18:39 [PATCH 0/1] add author and committer configuration settings William Hubbs @ 2018-12-19 18:39 ` William Hubbs 2018-12-19 20:05 ` John Passaro ` (2 more replies) 2018-12-19 21:46 ` [PATCH 0/1] add " Jonathan Nieder 1 sibling, 3 replies; 6+ messages in thread From: William Hubbs @ 2018-12-19 18:39 UTC (permalink / raw) To: git; +Cc: williamh, chutzpah The author.email, author.name, committer.email and committer.name settings are analogous to the GIT_AUTHOR_* and GIT_COMMITTER_* environment variables, but for the git config system. This allows them to be set separately for each repository. Signed-off-by: William Hubbs <williamh@gentoo.org> --- Documentation/config/user.txt | 20 +++++++++++ builtin/commit.c | 2 +- cache.h | 5 ++- config.c | 6 ++++ ident.c | 68 ++++++++++++++++++++++++++++++++--- log-tree.c | 3 +- sequencer.c | 3 +- 7 files changed, 97 insertions(+), 10 deletions(-) diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt index b5b2ba1199..6ba7002252 100644 --- a/Documentation/config/user.txt +++ b/Documentation/config/user.txt @@ -1,3 +1,23 @@ +author.email:: +Your email address to be recorded on the author line of any newly +created commits. +If this is not set, we use user.email. + +author.name:: +Your full name to be recorded on the author line of any newly +created commits. +If this is not set, we use user.name. + +committer.email:: +Your email address to be recorded on the committer line of any newly +created commits. +If this is not set, we use user.email. + +committer.name:: +Your full name to be recorded on the committer line of any newly +created commits. +If this is not set, we use user.name. + user.email:: Your email address to be recorded in any newly created commits. Can be overridden by the `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL`, and diff --git a/builtin/commit.c b/builtin/commit.c index c021b119bb..49a97adeb8 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -607,7 +607,7 @@ static void determine_author_info(struct strbuf *author_ident) set_ident_var(&date, strbuf_detach(&date_buf, NULL)); } - strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT)); + strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT|IDENT_AUTHOR)); assert_split_ident(&author, author_ident); export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0); export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0); diff --git a/cache.h b/cache.h index ca36b44ee0..0ee87f22a9 100644 --- a/cache.h +++ b/cache.h @@ -1479,10 +1479,13 @@ int date_overflows(timestamp_t date); #define IDENT_STRICT 1 #define IDENT_NO_DATE 2 #define IDENT_NO_NAME 4 +#define IDENT_AUTHOR 8 +#define IDENT_COMMITTER 16 + extern const char *git_author_info(int); extern const char *git_committer_info(int); extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int); -extern const char *fmt_name(const char *name, const char *email); +extern const char *fmt_committer_name(void); extern const char *ident_default_name(void); extern const char *ident_default_email(void); extern const char *git_editor(void); diff --git a/config.c b/config.c index ff521eb27a..4bd5920dea 100644 --- a/config.c +++ b/config.c @@ -1484,6 +1484,12 @@ int git_default_config(const char *var, const char *value, void *cb) return 0; } + if (starts_with(var, "author.")) + return git_ident_config(var, value, cb); + + if (starts_with(var, "committer.")) + return git_ident_config(var, value, cb); + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/ident.c b/ident.c index 33bcf40644..3da96ebbef 100644 --- a/ident.c +++ b/ident.c @@ -11,6 +11,10 @@ static struct strbuf git_default_name = STRBUF_INIT; static struct strbuf git_default_email = STRBUF_INIT; static struct strbuf git_default_date = STRBUF_INIT; +static struct strbuf git_author_name = STRBUF_INIT; +static struct strbuf git_author_email = STRBUF_INIT; +static struct strbuf git_committer_name = STRBUF_INIT; +static struct strbuf git_committer_email = STRBUF_INIT; static int default_email_is_bogus; static int default_name_is_bogus; @@ -361,7 +365,15 @@ const char *fmt_ident(const char *name, const char *email, int strict = (flag & IDENT_STRICT); int want_date = !(flag & IDENT_NO_DATE); int want_name = !(flag & IDENT_NO_NAME); + int want_author = (flag & IDENT_AUTHOR); + int want_committer = (flag & IDENT_COMMITTER); + if (!email) { + if (want_author && git_author_email.len) + email = git_author_email.buf; + else if (want_committer && git_committer_email.len) + email = git_committer_email.buf; + } if (!email) { if (strict && ident_use_config_only && !(ident_config_given & IDENT_MAIL_GIVEN)) { @@ -377,6 +389,12 @@ const char *fmt_ident(const char *name, const char *email, if (want_name) { int using_default = 0; + if (!name) { + if (want_author && git_author_name.len) + name = git_author_name.buf; + else if (want_committer && git_committer_name.len) + name = git_committer_name.buf; + } if (!name) { if (strict && ident_use_config_only && !(ident_config_given & IDENT_NAME_GIVEN)) { @@ -425,9 +443,11 @@ const char *fmt_ident(const char *name, const char *email, return ident.buf; } -const char *fmt_name(const char *name, const char *email) +const char *fmt_committer_name(void) { - return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE); + char *name = getenv("GIT_COMMITTER_NAME"); + char *email = getenv("GIT_COMMITTER_EMAIL"); + return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE|IDENT_COMMITTER); } const char *git_author_info(int flag) @@ -439,7 +459,7 @@ const char *git_author_info(int flag) return fmt_ident(getenv("GIT_AUTHOR_NAME"), getenv("GIT_AUTHOR_EMAIL"), getenv("GIT_AUTHOR_DATE"), - flag); + flag|IDENT_AUTHOR); } const char *git_committer_info(int flag) @@ -451,7 +471,7 @@ const char *git_committer_info(int flag) return fmt_ident(getenv("GIT_COMMITTER_NAME"), getenv("GIT_COMMITTER_EMAIL"), getenv("GIT_COMMITTER_DATE"), - flag); + flag|IDENT_COMMITTER); } static int ident_is_sufficient(int user_ident_explicitly_given) @@ -480,6 +500,46 @@ int git_ident_config(const char *var, const char *value, void *data) return 0; } + if (!strcmp(var, "author.name")) { + if (!value) + return config_error_nonbool(var); + strbuf_reset(&git_author_name); + strbuf_addstr(&git_author_name, value); + author_ident_explicitly_given |= IDENT_NAME_GIVEN; + ident_config_given |= IDENT_NAME_GIVEN; + return 0; + } + + if (!strcmp(var, "author.email")) { + if (!value) + return config_error_nonbool(var); + strbuf_reset(&git_author_email); + strbuf_addstr(&git_author_email, value); + author_ident_explicitly_given |= IDENT_MAIL_GIVEN; + ident_config_given |= IDENT_MAIL_GIVEN; + return 0; + } + + if (!strcmp(var, "committer.name")) { + if (!value) + return config_error_nonbool(var); + strbuf_reset(&git_committer_name); + strbuf_addstr(&git_committer_name, value); + committer_ident_explicitly_given |= IDENT_NAME_GIVEN; + ident_config_given |= IDENT_NAME_GIVEN; + return 0; + } + + if (!strcmp(var, "committer.email")) { + if (!value) + return config_error_nonbool(var); + strbuf_reset(&git_committer_email); + strbuf_addstr(&git_committer_email, value); + committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; + ident_config_given |= IDENT_MAIL_GIVEN; + return 0; + } + if (!strcmp(var, "user.name")) { if (!value) return config_error_nonbool(var); diff --git a/log-tree.c b/log-tree.c index 10680c139e..6760a2e9c4 100644 --- a/log-tree.c +++ b/log-tree.c @@ -687,8 +687,7 @@ void show_log(struct rev_info *opt) */ if (ctx.need_8bit_cte >= 0 && opt->add_signoff) ctx.need_8bit_cte = - has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"), - getenv("GIT_COMMITTER_EMAIL"))); + has_non_ascii(fmt_committer_name()); ctx.date_mode = opt->date_mode; ctx.date_mode_explicit = opt->date_mode_explicit; ctx.abbrev = opt->diffopt.abbrev; diff --git a/sequencer.c b/sequencer.c index e1a4dd15f1..f357defda5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4036,8 +4036,7 @@ void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag) int has_footer; strbuf_addstr(&sob, sign_off_header); - strbuf_addstr(&sob, fmt_name(getenv("GIT_COMMITTER_NAME"), - getenv("GIT_COMMITTER_EMAIL"))); + strbuf_addstr(&sob, fmt_committer_name()); strbuf_addch(&sob, '\n'); if (!ignore_footer) -- 2.19.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Add author and committer configuration settings 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs @ 2018-12-19 20:05 ` John Passaro 2018-12-21 12:15 ` Johannes Schindelin 2019-01-02 22:57 ` Junio C Hamano 2 siblings, 0 replies; 6+ messages in thread From: John Passaro @ 2018-12-19 20:05 UTC (permalink / raw) To: William Hubbs; +Cc: git, chutzpah On Wed, Dec 19, 2018 at 2:19 PM William Hubbs <williamh@gentoo.org> wrote: > > The author.email, author.name, committer.email and committer.name > settings are analogous to the GIT_AUTHOR_* and GIT_COMMITTER_* > environment variables, but for the git config system. This allows them > to be set separately for each repository. > Great! I didn't realize this wasn't already supported... However your patch does seem to be missing tests. t/t7517-per-repo-email.sh would appear to be a logical place to add them. > Signed-off-by: William Hubbs <williamh@gentoo.org> > --- > Documentation/config/user.txt | 20 +++++++++++ > builtin/commit.c | 2 +- > cache.h | 5 ++- > config.c | 6 ++++ > ident.c | 68 ++++++++++++++++++++++++++++++++--- > log-tree.c | 3 +- > sequencer.c | 3 +- > 7 files changed, 97 insertions(+), 10 deletions(-) > > diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt > index b5b2ba1199..6ba7002252 100644 > --- a/Documentation/config/user.txt > +++ b/Documentation/config/user.txt > @@ -1,3 +1,23 @@ > +author.email:: > +Your email address to be recorded on the author line of any newly > +created commits. > +If this is not set, we use user.email. > + > +author.name:: > +Your full name to be recorded on the author line of any newly > +created commits. > +If this is not set, we use user.name. > + > +committer.email:: > +Your email address to be recorded on the committer line of any newly > +created commits. > +If this is not set, we use user.email. > + > +committer.name:: > +Your full name to be recorded on the committer line of any newly > +created commits. > +If this is not set, we use user.name. > + > user.email:: > Your email address to be recorded in any newly created commits. > Can be overridden by the `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL`, and I think it would be wise to mention the new config items under user.email and user.name as well. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Add author and committer configuration settings 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs 2018-12-19 20:05 ` John Passaro @ 2018-12-21 12:15 ` Johannes Schindelin 2019-01-02 22:57 ` Junio C Hamano 2 siblings, 0 replies; 6+ messages in thread From: Johannes Schindelin @ 2018-12-21 12:15 UTC (permalink / raw) To: William Hubbs; +Cc: git, chutzpah Hi William, thank you for putting this together. On Wed, 19 Dec 2018, William Hubbs wrote: > -extern const char *fmt_name(const char *name, const char *email); > +extern const char *fmt_committer_name(void); If it would not be too much trouble for you, could I ask you to split this change out into a separate commit (which would be the first of now two patches)? It could have a commit message like this: ident: rename fmt_name() to fmt_committer_name() Ever since 4c28e4ada03f (commit: die before asking to edit the log message, 2010-12-20), all remaining callers of that function want to format the committer name. To simplify the code, therefore, we rename the function and move the getenv() call into it. Ciao, Dscho ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Add author and committer configuration settings 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs 2018-12-19 20:05 ` John Passaro 2018-12-21 12:15 ` Johannes Schindelin @ 2019-01-02 22:57 ` Junio C Hamano 2 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2019-01-02 22:57 UTC (permalink / raw) To: William Hubbs; +Cc: git, chutzpah William Hubbs <williamh@gentoo.org> writes: > Subject: Re: [PATCH 1/1] Add author and committer configuration settings Perhaps something like this Subject: config: allow giving separate author and committer idents would fit better in "git shortlog --no-merges" output. > The author.email, author.name, committer.email and committer.name > settings are analogous to the GIT_AUTHOR_* and GIT_COMMITTER_* > environment variables, but for the git config system. This allows them > to be set separately for each repository. Don't indent the whole proposed log message. > diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt > index b5b2ba1199..6ba7002252 100644 > --- a/Documentation/config/user.txt > +++ b/Documentation/config/user.txt > @@ -1,3 +1,23 @@ > +author.email:: > +Your email address to be recorded on the author line of any newly > +created commits. > +If this is not set, we use user.email. "author line" is a bit too technical and appropriate only to those who are familiar with "git cat-file commit" output. How about phrasing it a bit differently, like author.email:: The email-address used for the author of newly created commits. Defaults to the value of the `GIT_AUTHOR_EMAIL` environment variable, or if it is not set, the `user.email` configuration variable. Likewise for the other three variables. > user.email:: > Your email address to be recorded in any newly created commits. > Can be overridden by the `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL`, and As you can see, the enumeration list in this file is formatted by indenting the definition body. > diff --git a/builtin/commit.c b/builtin/commit.c > index c021b119bb..49a97adeb8 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -607,7 +607,7 @@ static void determine_author_info(struct strbuf *author_ident) > set_ident_var(&date, strbuf_detach(&date_buf, NULL)); > } > > - strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT)); > + strbuf_addstr(author_ident, fmt_ident(name, email, date, IDENT_STRICT|IDENT_AUTHOR)); That's now a bit overly long line. > assert_split_ident(&author, author_ident); > export_one("GIT_AUTHOR_NAME", author.name_begin, author.name_end, 0); > export_one("GIT_AUTHOR_EMAIL", author.mail_begin, author.mail_end, 0); > diff --git a/cache.h b/cache.h > index ca36b44ee0..0ee87f22a9 100644 > --- a/cache.h > +++ b/cache.h > @@ -1479,10 +1479,13 @@ int date_overflows(timestamp_t date); > #define IDENT_STRICT 1 > #define IDENT_NO_DATE 2 > #define IDENT_NO_NAME 4 > +#define IDENT_AUTHOR 8 > +#define IDENT_COMMITTER 16 > extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int); It is wrong to pass "do we want the author, or the committer, name?" information in the same flag parameter to fmt_ident(), and it is wrong to introduce IDENT_AUTHOR/COMMITTER bits as if they belong to the existing four. For one thing, unlike these other bits, these two are not independent bits. It would be an error for a caller to pass neither bits, or both bits at the same time. One way to do it better may be to pass it as another parameter, e.g. enum want_ident { WANT_AUTHOR_IDENT, WANT_COMMITTER_IDENT }; const char *fmt_ident(const char *name, const char *email, enum want_ident whose_ident, const char *date_str, int flags); In the remainder of the review, I'd give update suggestions based on this function signature. > diff --git a/ident.c b/ident.c > index 33bcf40644..3da96ebbef 100644 > --- a/ident.c > +++ b/ident.c > @@ -11,6 +11,10 @@ > static struct strbuf git_default_name = STRBUF_INIT; > static struct strbuf git_default_email = STRBUF_INIT; > static struct strbuf git_default_date = STRBUF_INIT; > +static struct strbuf git_author_name = STRBUF_INIT; > +static struct strbuf git_author_email = STRBUF_INIT; > +static struct strbuf git_committer_name = STRBUF_INIT; > +static struct strbuf git_committer_email = STRBUF_INIT; > static int default_email_is_bogus; > static int default_name_is_bogus; > > @@ -361,7 +365,15 @@ const char *fmt_ident(const char *name, const char *email, > int strict = (flag & IDENT_STRICT); > int want_date = !(flag & IDENT_NO_DATE); > int want_name = !(flag & IDENT_NO_NAME); > + int want_author = (flag & IDENT_AUTHOR); > + int want_committer = (flag & IDENT_COMMITTER); > > + if (!email) { > + if (want_author && git_author_email.len) > + email = git_author_email.buf; > + else if (want_committer && git_committer_email.len) > + email = git_committer_email.buf; > + } > if (!email) { > if (strict && ident_use_config_only > && !(ident_config_given & IDENT_MAIL_GIVEN)) { > @@ -377,6 +389,12 @@ const char *fmt_ident(const char *name, const char *email, > > if (want_name) { > int using_default = 0; > + if (!name) { > + if (want_author && git_author_name.len) > + name = git_author_name.buf; > + else if (want_committer && git_committer_name.len) > + name = git_committer_name.buf; > + } > if (!name) { > if (strict && ident_use_config_only > && !(ident_config_given & IDENT_NAME_GIVEN)) { The reviewer's interest here is to see how "author.name trumps user.name" precedence is implemented; by mucking with "name" before the code that deals with the ident_default_name(), which yields git_default_name taken from user.name, the code gives precedence to these newly introduced variables. Which makes sense. > @@ -425,9 +443,11 @@ const char *fmt_ident(const char *name, const char *email, > return ident.buf; > } > > -const char *fmt_name(const char *name, const char *email) > +const char *fmt_committer_name(void) > { > - return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE); > + char *name = getenv("GIT_COMMITTER_NAME"); > + char *email = getenv("GIT_COMMITTER_EMAIL"); > + return fmt_ident(name, email, NULL, IDENT_STRICT | IDENT_NO_DATE|IDENT_COMMITTER); > } OK, we are lucky that no existing caller use fmt_name() with author information, I guess? The resulting codebase does invite a question "why don't we have fmt_author_name() at all?", which is somewhat disturbing. As we are going to change this function *and* all of its callers anyway, perhaps we can generalize it with minimum effort, like so: const char *fmt_name(enum want_ident whose_ident) { switch (whose_ident) { case WANT_AUTHOR_IDENT: name = getenv("GIT_AUTHOR_NAME"); email = getenv("GIT_COMMITTER_NAME"); break; case WANT_COMMITTER_IDENT: ... } return fmt_ident(name, email, whose_ident, NULL, IDENT_STRICT | IDENT_NO_DATE); } Thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/1] add author and committer configuration settings 2018-12-19 18:39 [PATCH 0/1] add author and committer configuration settings William Hubbs 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs @ 2018-12-19 21:46 ` Jonathan Nieder 1 sibling, 0 replies; 6+ messages in thread From: Jonathan Nieder @ 2018-12-19 21:46 UTC (permalink / raw) To: William Hubbs; +Cc: git, chutzpah Hi, William Hubbs wrote: > this is my first patch for git, so please be gentle. ;-) Thanks for contributing! > I am in a situation where I need to use different authorship information > for some repositories I commit to. > > Git already supports setting different authorship and committer > information with environment variables, but this setting is global so if > you want to change it per repository you need to use a separate tool. > > This patch adds support to git config for author.email, author.name, > committer.email and committer.name settings so this information > can be set per repository. It applies to current master. The above information (except for "It applies to current master") is very useful to have when looking back at the change in history. When sending the next version of this patch in response to others' comments, can you replace the commit message with something like it? In other words, it is very useful for the commit message to include this kind of information about the "why" behind a change, beyond the "what" that the patch itself already provides. > Thanks much for your reviews, and I would like to publically thank dscho > from #git-devel for assisting me in preparing this patch. > > Also, since I use a screen reader, it would be very helpful if you put > your comments in your replies as close to the affected code as possible, > preferably directly below it. Fortunately, this is already common practice here, but the reminder is very welcome. By the way, if you have other feedback about Git accessibility through a screen reader (both the project and the tool), I would be very interested to hear. Presumably in a new thread. :) Thanks and hope that helps, Jonathan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-02 22:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-19 18:39 [PATCH 0/1] add author and committer configuration settings William Hubbs 2018-12-19 18:39 ` [PATCH 1/1] Add " William Hubbs 2018-12-19 20:05 ` John Passaro 2018-12-21 12:15 ` Johannes Schindelin 2019-01-02 22:57 ` Junio C Hamano 2018-12-19 21:46 ` [PATCH 0/1] add " Jonathan Nieder
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.