git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>,
	git-for-windows@googlegroups.com, git@vger.kernel.org,
	git-packagers@googlegroups.com,
	Ariadne Conill <ariadne@dereferenced.org>
Subject: Re: Git for Windows v2.23.0-rc0, was Re: [ANNOUNCE] Git v2.23.0-rc0
Date: Wed, 31 Jul 2019 17:21:25 -0700
Message-ID: <20190801002125.GA176307@google.com> (raw)
In-Reply-To: <20190731231848.GC1933@sigill.intra.peff.net>

Jeff King wrote:

> I think part of what my annoyance is responding to (and your willingness
> to just squelch this for everybody) is that switching this particular
> default isn't really that big a deal, that it requires annoying people
> on every single "git log" invocation. Perhaps we would be better
> squelching it entirely and just putting a mention in the release notes.

Yes.

Although as Dscho mentions, it's particularly irritating because it is
not part of the paginated output.

I wonder if the ideal might not be to trigger it more selectively, when
the output actually changed due to a reflog entry.  I mean something
like

	commit 393a9dd0f9762c69f753a8fa0bc89c203c6b4e9e (HEAD, origin/foo, other/pu)
	Merge: 18598e40e6 1eba6eb1c2
	Author: A U Thor <author@example.com> (see "git help mailmap")
	Date:   Tue Jul 30 15:05:41 2019 -0700

	    Merge branch 'jt/fetch-cdn-offload' into foo

The message

	warning: log.mailmap is not set; its implicit value will change in an
	upcoming release. To squelch this message and preserve current
	behaviour, set the log.mailmap configuration value to false.

	To squelch this message and adopt the new behaviour now, set the
	log.mailmap configuration value to true.

is *particularly* unactionable in the current state where we're not
rewriting authors.  I think we should bite the bullet and just flip
the default to "true", with the config as an escape hatch to allow
going back to the old behavior.

Is it too late in the release cycle to do that?  If not, we can do

-- >8 --
Subject: log: use mailmap by default in interactive use

In f0596ecc8de9 (log: add warning for unspecified log.mailmap setting,
2019-07-15), we prepared for a future where "git log" uses the mailmap
by default, using the following conditions:

 1. If log.mailmap or --[no-]use-mailmap is set explicitly explicitly,
    respect that.  Otherwise:

 2. If output is not going to a terminal or pager, we might be in a
    script.  Match historical behavior by not using the mailmap.
    Otherwise:

 3. If the output format was specified using --pretty, we might be in
    a script that produces output intended for copy/paste.  Match
    historical behavior by not using the mailmap.  Otherwise:

 4. This is an interactive use, where we will be able to change the
    default behavior.  Print a warning about the upcoming change
    but don't use the mailmap yet.

In practice, the case (4) turns out to be irritating.  It prints
before pager setup, so it just flashes on the screen.  It appears on
every "git log" invocation, even when the mailmap would not result in
the output changing.  The change is not meaningful to most people, and
the new default of --use-mailmap is likely to be preferable for all of
them.

Let's bite the bullet and jump straight to --use-mailmap in case (4).

While at it, add a new log.mailmap setting "auto" that can be used to
explicitly request the new automatic behavior (so that e.g. if
log.mailmap is set to "true" system-side, I can set it to "auto" in my
per-user configuration).

Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/RelNotes/2.23.0.txt |  6 ++----
 Documentation/config/log.txt      |  4 +++-
 builtin/log.c                     | 23 ++++++-----------------
 t/t4203-mailmap.sh                | 20 ++++++++++++++++++++
 t/t7006-pager.sh                  |  2 --
 5 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/Documentation/RelNotes/2.23.0.txt b/Documentation/RelNotes/2.23.0.txt
index 9896599b2f..b4765a5472 100644
--- a/Documentation/RelNotes/2.23.0.txt
+++ b/Documentation/RelNotes/2.23.0.txt
@@ -91,10 +91,8 @@ UI, Workflows & Features
    commit-graph files now, which allows the commit-graph files to be
    updated incrementally.
 
- * The "git log" command learns to issue a warning when log.mailmap
-   configuration is not set and --[no-]mailmap option is not used, to
-   prepare users for future versions of Git that uses the mailmap by
-   default.
+ * The "git log" command learned to use --mailmap by default when
+   used interactively without a --pretty format option.
 
  * "git range-diff" output has been tweaked for easier identification
    of which part of what file the patch shown is about.
diff --git a/Documentation/config/log.txt b/Documentation/config/log.txt
index 7798e10cb0..739ea298aa 100644
--- a/Documentation/config/log.txt
+++ b/Documentation/config/log.txt
@@ -41,4 +41,6 @@ log.showSignature::
 log.mailmap::
 	If true, makes linkgit:git-log[1], linkgit:git-show[1], and
 	linkgit:git-whatchanged[1] assume `--use-mailmap`, otherwise
-	assume `--no-use-mailmap`. False by default.
+	assume `--no-use-mailmap`. The default is `auto`, which means
+	to use mailmap if the output is going to a terminal and no
+	`--pretty` format has been specified.
diff --git a/builtin/log.c b/builtin/log.c
index 1cf9e37736..bafce51d01 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -156,16 +156,6 @@ static void cmd_log_init_defaults(struct rev_info *rev)
 		parse_date_format(default_date_mode, &rev->date_mode);
 }
 
-static char warn_unspecified_mailmap_msg[] =
-N_("log.mailmap is not set; its implicit value will change in an\n"
-   "upcoming release. To squelch this message and preserve current\n"
-   "behaviour, set the log.mailmap configuration value to false.\n"
-   "\n"
-   "To squelch this message and adopt the new behaviour now, set the\n"
-   "log.mailmap configuration value to true.\n"
-   "\n"
-   "See 'git help config' and search for 'log.mailmap' for further information.");
-
 static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 			 struct rev_info *rev, struct setup_revision_opt *opt)
 {
@@ -214,12 +204,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 	memset(&w, 0, sizeof(w));
 	userformat_find_requirements(NULL, &w);
 
-	if (mailmap < 0) {
-		if (session_is_interactive() && !rev->pretty_given)
-			warning("%s\n", _(warn_unspecified_mailmap_msg));
-
-		mailmap = 0;
-	}
+	if (mailmap < 0)
+		mailmap = session_is_interactive() && !rev->pretty_given;
 
 	if (!rev->show_notes_given && (!rev->pretty_given || w.notes))
 		rev->show_notes = 1;
@@ -477,7 +463,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
 	if (skip_prefix(var, "color.decorate.", &slot_name))
 		return parse_decorate_color_config(var, slot_name, value);
 	if (!strcmp(var, "log.mailmap")) {
-		use_mailmap_config = git_config_bool(var, value);
+		if (value && !strcasecmp(value, "auto"))
+			use_mailmap_config = -1;
+		else
+			use_mailmap_config = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "log.showsignature")) {
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 43b1522ea2..e398bc803a 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -3,6 +3,7 @@
 test_description='.mailmap configurations'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-terminal.sh
 
 fuzz_blame () {
 	sed "
@@ -442,6 +443,25 @@ test_expect_success 'Log output with log.mailmap' '
 	test_cmp expect actual
 '
 
+test_expect_success TTY 'Use mailmap by default when writing to terminal' '
+	test_terminal git log | grep Author >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success TTY 'Config overrides terminal' '
+	cat >expect <<-\EOF &&
+	Author: CTO <cto@coompany.xx>
+	Author: claus <me@company.xx>
+	Author: santa <me@company.xx>
+	Author: nick2 <nick2@company.xx>
+	Author: nick2 <bugs@company.xx>
+	Author: nick1 <bugs@company.xx>
+	Author: A U Thor <author@example.com>
+	EOF
+	test_terminal git -c log.mailmap=False log | grep Author >actual &&
+	test_cmp expect actual
+'
+
 cat >expect <<\EOF
 Author: Santa Claus <santa.claus@northpole.xx>
 Author: Santa Claus <santa.claus@northpole.xx>
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 7976fa7bcc..00e09a375c 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -7,8 +7,6 @@ test_description='Test automatic use of a pager.'
 . "$TEST_DIRECTORY"/lib-terminal.sh
 
 test_expect_success 'setup' '
-	: squelch advice messages during the transition &&
-	git config --global log.mailmap false &&
 	sane_unset GIT_PAGER GIT_PAGER_IN_USE &&
 	test_unconfig core.pager &&
 
-- 
2.22.0.770.g0f2c4a37fd


  reply	other threads:[~2019-08-01  0:21 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-29 21:49 Junio C Hamano
2019-07-31 12:43 ` Git for Windows v2.23.0-rc0, was " Johannes Schindelin
2019-07-31 23:18   ` Jeff King
2019-08-01  0:21     ` Jonathan Nieder [this message]
2019-08-01  0:37       ` Ariadne Conill
2019-08-01  1:00       ` Jeff King
2019-08-01  1:14         ` Jonathan Nieder
2019-08-01  1:38         ` Ariadne Conill
2019-08-01  2:53           ` Jeff King
2019-08-01  3:21         ` Junio C Hamano
2019-08-01  4:54           ` Ariadne Conill
2019-08-01 21:39             ` Johannes Schindelin
2019-08-01 15:45       ` Junio C Hamano
2019-08-01 16:12         ` Ariadne Conill
2019-08-01 21:36         ` Jeff King
2019-08-01 21:46           ` Junio C Hamano
2019-08-01 21:54             ` Junio C Hamano
2019-08-01 22:18               ` Todd Zullinger
2019-08-02  2:27         ` Jonathan Nieder
2019-08-02 16:53           ` Junio C Hamano
2019-08-02 18:35             ` Jeff King
2019-08-01  1:04   ` [git-for-windows] " Bryan Turner
2019-08-01 21:40     ` Johannes Schindelin
2019-08-01 14:12 ` [PATCH] RelNotes/2.23.0: fix a few typos and other minor issues Martin Ågren
2019-08-01 15:56   ` Junio C Hamano
2019-08-01 15:57   ` Junio C Hamano
2019-08-01 19:28     ` Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190801002125.GA176307@google.com \
    --to=jrnieder@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=ariadne@dereferenced.org \
    --cc=git-for-windows@googlegroups.com \
    --cc=git-packagers@googlegroups.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git