| Message ID | xmqq5z9b91o3.fsf_-_@gitster.c.googlers.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e9bd00ab2dad0148c8643a773ea2e68d624f5624 |
| Headers | show |
| Series | ident: say whose identity is missing when giving user.name hint | expand |
On Fri, Aug 21, 2020 at 4:36 PM Junio C Hamano <gitster@pobox.com> wrote: > When the author or the committer identity is missing when required, > "git" errors out with a message that suggests to set these > configuration variables at the per-user level as the easiest way > forward. This message is given to a brand-new user, whose > ~/.gitconfig hasn't been configured for user.name and user.email, > who runs "git commit --author=...", too, but such a user may find it > confusing ("why? I just gave you a name and e-mail"). > > State whose identity is missing as the reason why we are erroring > out, when we give the hint, to help reduce the confusion. I had trouble following the first paragraph due to the run-on nature of the second sentence. Perhaps the entire message could be rewritten something like this: If `user.name` and `user.email` have not been configured and the user invokes: git commit --author=... without without specifying `--committer=`, then Git errors out with a message asking the user to configure `user.name` and `user.email` but doesn't tell the user which attribution was missing. This can be confusing for a user new to Git who isn't aware of the distinction between user, author, and committer. Give such users a bit more help by extending the error message to also say which attribution is expected.
Eric Sunshine <sunshine@sunshineco.com> writes: > If `user.name` and `user.email` have not been configured and the > user invokes: > > git commit --author=... > > without without specifying `--committer=`, then Git errors out > with a message asking the user to configure `user.name` and > `user.email` but doesn't tell the user which attribution was > missing. This can be confusing for a user new to Git who isn't > aware of the distinction between user, author, and committer. > Give such users a bit more help by extending the error message to > also say which attribution is expected. Much easier to read. Will steal.
One nit though: There is no `--committer` flag for `git commit`, unless this has changed after the v2.28.0 release: ``` $ g commit -m message --committer someone@somemail.com error: unknown option `committer' $ git --version git version 2.28.0 ``` On Fri, Aug 21, 2020 at 5:13 PM Junio C Hamano <gitster@pobox.com> wrote: > > Eric Sunshine <sunshine@sunshineco.com> writes: > > > If `user.name` and `user.email` have not been configured and the > > user invokes: > > > > git commit --author=... > > > > without without specifying `--committer=`, then Git errors out > > with a message asking the user to configure `user.name` and > > `user.email` but doesn't tell the user which attribution was > > missing. This can be confusing for a user new to Git who isn't > > aware of the distinction between user, author, and committer. > > Give such users a bit more help by extending the error message to > > also say which attribution is expected. > > Much easier to read. Will steal. >
On Fri, Aug 21, 2020 at 5:31 PM Alvaro Aleman <aaleman@redhat.com> wrote:
> One nit though: There is no `--committer` flag for `git commit`,
Indeed, that `--committer=` was a last-second edit (without checking
docs). How about this?
If `user.name` and `user.email` have not been configured and the
user invokes:
git commit --author=...
without specifying the committer, then Git errors out with a
message asking the user to configure `user.name` and `user.email`
but doesn't tell the user which attribution was missing. This can
be confusing for a user new to Git who isn't aware of the
distinction between user, author, and committer. Give such users
a bit more help by extending the error message to also say which
attribution is expected.
Eric Sunshine <sunshine@sunshineco.com> writes: > On Fri, Aug 21, 2020 at 5:31 PM Alvaro Aleman <aaleman@redhat.com> wrote: >> One nit though: There is no `--committer` flag for `git commit`, > > Indeed, that `--committer=` was a last-second edit (without checking > docs). Yeah, I forgot that we deliberately omit the command line option for the committer info. > How about this? > > If `user.name` and `user.email` have not been configured and the > user invokes: > > git commit --author=... > > without specifying the committer, then Git errors out with a > message asking the user to configure `user.name` and `user.email` > but doesn't tell the user which attribution was missing. This can > be confusing for a user new to Git who isn't aware of the > distinction between user, author, and committer. Give such users > a bit more help by extending the error message to also say which > attribution is expected. OK.
diff --git a/ident.c b/ident.c index e666ee4e59..813741c06c 100644 --- a/ident.c +++ b/ident.c @@ -345,18 +345,32 @@ int split_ident_line(struct ident_split *split, const char *line, int len) return 0; } -static const char *env_hint = -N_("\n" - "*** Please tell me who you are.\n" - "\n" - "Run\n" - "\n" - " git config --global user.email \"you@example.com\"\n" - " git config --global user.name \"Your Name\"\n" - "\n" - "to set your account\'s default identity.\n" - "Omit --global to set the identity only in this repository.\n" - "\n"); + +static void ident_env_hint(enum want_ident whose_ident) +{ + switch (whose_ident) { + case WANT_AUTHOR_IDENT: + fputs(_("Author identity unknown\n"), stderr); + break; + case WANT_COMMITTER_IDENT: + fputs(_("Committer identity unknown\n"), stderr); + break; + default: + break; + } + + fputs(_("\n" + "*** Please tell me who you are.\n" + "\n" + "Run\n" + "\n" + " git config --global user.email \"you@example.com\"\n" + " git config --global user.name \"Your Name\"\n" + "\n" + "to set your account\'s default identity.\n" + "Omit --global to set the identity only in this repository.\n" + "\n"), stderr); +} const char *fmt_ident(const char *name, const char *email, enum want_ident whose_ident, const char *date_str, int flag) @@ -375,12 +389,12 @@ const char *fmt_ident(const char *name, const char *email, if (!email) { if (strict && ident_use_config_only && !(ident_config_given & IDENT_MAIL_GIVEN)) { - fputs(_(env_hint), stderr); + ident_env_hint(whose_ident); die(_("no email was given and auto-detection is disabled")); } email = ident_default_email(); if (strict && default_email_is_bogus) { - fputs(_(env_hint), stderr); + ident_env_hint(whose_ident); die(_("unable to auto-detect email address (got '%s')"), email); } } @@ -397,13 +411,13 @@ const char *fmt_ident(const char *name, const char *email, if (!name) { if (strict && ident_use_config_only && !(ident_config_given & IDENT_NAME_GIVEN)) { - fputs(_(env_hint), stderr); + ident_env_hint(whose_ident); die(_("no name was given and auto-detection is disabled")); } name = ident_default_name(); using_default = 1; if (strict && default_name_is_bogus) { - fputs(_(env_hint), stderr); + ident_env_hint(whose_ident); die(_("unable to auto-detect name (got '%s')"), name); } } @@ -411,7 +425,7 @@ const char *fmt_ident(const char *name, const char *email, struct passwd *pw; if (strict) { if (using_default) - fputs(_(env_hint), stderr); + ident_env_hint(whose_ident); die(_("empty ident name (for <%s>) not allowed"), email); } pw = xgetpwuid_self(NULL); diff --git a/t/t7518-ident-corner-cases.sh b/t/t7518-ident-corner-cases.sh index b22f631261..dc3e9c8c88 100755 --- a/t/t7518-ident-corner-cases.sh +++ b/t/t7518-ident-corner-cases.sh @@ -29,7 +29,18 @@ test_expect_success 'empty configured name does not auto-detect' ' sane_unset GIT_AUTHOR_NAME && test_must_fail \ git -c user.name= commit --allow-empty -m foo 2>err && - test_i18ngrep "empty ident name" err + test_i18ngrep "empty ident name" err && + test_i18ngrep "Author identity unknown" err + ) +' + +test_expect_success 'empty configured name does not auto-detect for committer' ' + ( + sane_unset GIT_COMMITTER_NAME && + test_must_fail \ + git -c user.name= commit --allow-empty -m foo 2>err && + test_i18ngrep "empty ident name" err && + test_i18ngrep "Committer identity unknown" err ) '
OK, so here is essentially the same patch but with a bit of cleaning up, with a test update and a proposed log message. -- >8 -- When the author or the committer identity is missing when required, "git" errors out with a message that suggests to set these configuration variables at the per-user level as the easiest way forward. This message is given to a brand-new user, whose ~/.gitconfig hasn't been configured for user.name and user.email, who runs "git commit --author=...", too, but such a user may find it confusing ("why? I just gave you a name and e-mail"). State whose identity is missing as the reason why we are erroring out, when we give the hint, to help reduce the confusion. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- ident.c | 48 ++++++++++++++++++++++------------- t/t7518-ident-corner-cases.sh | 13 +++++++++- 2 files changed, 43 insertions(+), 18 deletions(-)