From: Junio C Hamano <gitster@pobox.com>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: David Turner <dturner@twopensource.com>, git@vger.kernel.org
Subject: Re: [PATCH v4] log: add log.follow config option
Date: Thu, 09 Jul 2015 10:23:24 -0700 [thread overview]
Message-ID: <xmqqegkhw8hf.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <vpqfv4xafpp.fsf@anie.imag.fr> (Matthieu Moy's message of "Thu, 09 Jul 2015 10:38:10 +0200")
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> David Turner <dturner@twopensource.com> writes:
>
>> This version uses tweak, and also includes Matthieu Moy's suggested
>> whitespace fix.
>
> This comment should come below the --- after the commit message (right
> before the diffstat). Otherwise, Junio will get this as the commit
> message when applying, and your actual commit message will be ignored.
>
>> ---
>> Many users prefer to always use --follow with logs. Rather than
>> aliasing the command, an option might be more convenient for some.
>>
>> Junio C Hamano <gitster@pobox.com> suggested using the tweak
>> functionality for this, which is much nicer than what I had before.
>
> I would avoid using "what I had before" in the commit message: readers
> of "git log" do not know what you had before.
That, together with "This version uses...", should go below the
three-dash line after the real log message. "compared to what I had
before" is a perfectly good thing to say to help reviewers there.
> OTOH, crediting Junio for the idea is good.
I do not think I deserve anything more than a Helped-by on this
particular one, if any.
More importantly, the "real log message" part somewhat lacking, I
think. The commit author is in no position to declare "Many users
prefer X" as if it were a fact. But the author is in a very good
position to explain:
- why users might want to do X under what condition;
- how the new feature helps them do so; and
- how the implementation carefully avoids not doing unwanted things
when inappropriate.
all of which would give good justifications for the change.
If I were David and sending this v4 patch, it would have looked like
this.
-- >8 --
From: David Turner <dturner@twopensource.com>
Date: Tue, 7 Jul 2015 21:29:34 -0400
Subject: [PATCH v4] log: add "log.follow" configuration variable
People who work on projects with mostly linear history with frequent
whole file renames may want to always use "git log --follow" when
inspecting the life of the content that live in a single path.
Teach the command to behave as if "--follow" was given from the
command line when log.follow configuration variable is set *and*
there is one (and only one) path on the command line.
Signed-off-by: David Turner <dturner@twopensource.com>
---
* Changes from v3:
- fixed whitespace breakage in test, pointed out by Matthieu
- revert all changes to revision.c; instead use the existing
opt->tweak() mechanism, suggested by Junio
Documentation/git-log.txt | 6 ++++++
builtin/log.c | 16 ++++++++++++++++
diff.c | 5 +++--
diff.h | 1 +
t/t4202-log.sh | 23 +++++++++++++++++++++++
5 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 5692945..79bf4d4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -184,6 +184,12 @@ log.date::
`--date` option.) Defaults to "default", which means to write
dates like `Sat May 8 19:35:34 2010 -0500`.
+log.follow::
+ If a single <path> is given to git log, it will act as
+ if the `--follow` option was also used. This has the same
+ limitations as `--follow`, i.e. it cannot be used to follow
+ multiple files and does not work well on non-linear history.
+
log.showRoot::
If `false`, `git log` and related commands will not treat the
initial commit as a big creation event. Any root commits in
diff --git a/builtin/log.c b/builtin/log.c
index 8781049..d06248a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -31,6 +31,7 @@ static const char *default_date_mode = NULL;
static int default_abbrev_commit;
static int default_show_root = 1;
+static int default_follow;
static int decoration_style;
static int decoration_given;
static int use_mailmap_config;
@@ -102,6 +103,8 @@ static void cmd_log_init_defaults(struct rev_info *rev)
{
if (fmt_pretty)
get_commit_format(fmt_pretty, rev);
+ if (default_follow)
+ DIFF_OPT_SET(&rev->diffopt, DEFAULT_FOLLOW_RENAMES);
rev->verbose_header = 1;
DIFF_OPT_SET(&rev->diffopt, RECURSIVE);
rev->diffopt.stat_width = -1; /* use full terminal width */
@@ -390,6 +393,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
default_show_root = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "log.follow")) {
+ default_follow = git_config_bool(var, value);
+ return 0;
+ }
if (skip_prefix(var, "color.decorate.", &slot_name))
return parse_decorate_color_config(var, slot_name, value);
if (!strcmp(var, "log.mailmap")) {
@@ -618,6 +625,14 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
return cmd_log_walk(&rev);
}
+static void default_follow_tweak(struct rev_info *rev,
+ struct setup_revision_opt *opt)
+{
+ if (DIFF_OPT_TST(&rev->diffopt, DEFAULT_FOLLOW_RENAMES) &&
+ rev->prune_data.nr == 1)
+ DIFF_OPT_SET(&rev->diffopt, FOLLOW_RENAMES);
+}
+
int cmd_log(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
@@ -631,6 +646,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
memset(&opt, 0, sizeof(opt));
opt.def = "HEAD";
opt.revarg_opt = REVARG_COMMITTISH;
+ opt.tweak = default_follow_tweak;
cmd_log_init(argc, argv, prefix, &rev, &opt);
return cmd_log_walk(&rev);
}
diff --git a/diff.c b/diff.c
index 87b16d5..135b222 100644
--- a/diff.c
+++ b/diff.c
@@ -3815,9 +3815,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
DIFF_OPT_SET(options, FIND_COPIES_HARDER);
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
- else if (!strcmp(arg, "--no-follow"))
+ else if (!strcmp(arg, "--no-follow")) {
DIFF_OPT_CLR(options, FOLLOW_RENAMES);
- else if (!strcmp(arg, "--color"))
+ DIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);
+ } else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (skip_prefix(arg, "--color=", &arg)) {
int value = git_config_colorbool(NULL, arg);
diff --git a/diff.h b/diff.h
index c7ad42a..f7208ad 100644
--- a/diff.h
+++ b/diff.h
@@ -91,6 +91,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_OPT_DIRSTAT_BY_LINE (1 << 28)
#define DIFF_OPT_FUNCCONTEXT (1 << 29)
#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)
+#define DIFF_OPT_DEFAULT_FOLLOW_RENAMES (1 << 31)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_TOUCHED(opts, flag) ((opts)->touched_flags & DIFF_OPT_##flag)
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 1b2e981..35d2d7c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -146,7 +146,30 @@ test_expect_success 'git log --follow' '
actual=$(git log --follow --pretty="format:%s" ichi) &&
expect=$(echo third ; echo second ; echo initial) &&
verbose test "$actual" = "$expect"
+'
+
+test_expect_success 'git config log.follow works like --follow' '
+ test_config log.follow true &&
+ actual=$(git log --pretty="format:%s" ichi) &&
+ expect=$(echo third ; echo second ; echo initial) &&
+ verbose test "$actual" = "$expect"
+'
+test_expect_success 'git config log.follow does not die with multiple paths' '
+ test_config log.follow true &&
+ git log --pretty="format:%s" ichi ein
+'
+
+test_expect_success 'git config log.follow does not die with no paths' '
+ test_config log.follow true &&
+ git log --
+'
+
+test_expect_success 'git config log.follow is overridden by --no-follow' '
+ test_config log.follow true &&
+ actual=$(git log --no-follow --pretty="format:%s" ichi) &&
+ expect="third" &&
+ verbose test "$actual" = "$expect"
'
cat > expect << EOF
--
2.5.0-rc1-326-g3cdaa82
next prev parent reply other threads:[~2015-07-09 17:23 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-08 17:42 [PATCH v4] log: add log.follow config option David Turner
2015-07-09 8:38 ` Matthieu Moy
2015-07-09 17:23 ` Junio C Hamano [this message]
2015-07-09 17:38 ` David Turner
2015-07-09 17:58 ` Junio C Hamano
2015-07-10 17:53 ` David Turner
2015-07-10 21:03 ` Junio C Hamano
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=xmqqegkhw8hf.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=dturner@twopensource.com \
--cc=git@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).