From: Thomas Rast <trast@student.ethz.ch>
To: Oded Shimon <ods15@ods15.dyndns.org>
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jan Krüger" <jk@jk.gs>,
git@vger.kernel.org
Subject: [Alt. PATCH] format-patch: do not use diff UI config
Date: Thu, 9 Sep 2010 10:36:54 +0200 [thread overview]
Message-ID: <2a6b8c51903fd6a22606b8f592b1a2e11ea68741.1284020917.git.trast@student.ethz.ch> (raw)
In-Reply-To: <1284019625-14096-1-git-send-email-ods15@ods15.dyndns.org>
format-patch read and used the diff UI config, such as diff.renames,
diff.noprefix and diff.mnemnoicprefix. These have a history of
breaking rebase and patch application in general; cf. 840b3ca (rebase:
protect against diff.renames configuration, 2008-11-10).
Instead of continually putting more options inside git-rebase to avoid
these issues, this patch takes the stance that output from
format-patch is intended primarily for git-am and only as a side
effect also for human consumption. Hence, ignore the diff UI config
entirely when coming from format-patch.
Note that all existing calls to git_log_config except for the one in
git_format_config use a NULL callback.
Reported-by: Oded Shimon <ods15@ods15.dyndns.org>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
This is a bolder approach that just outright ignores the backwards
compatibility complaints Junio had in 840b3ca. Among the variables
parsed in git_diff_ui_config, namely
color.diff (and its legacy alias diff.color)
diff.renames
diff.autorefreshindex
diff.mnemonicprefix
diff.noprefix
diff.external
diff.wordregex
diff.ignoresubmodules
arguably only diff.renames (and perhaps diff.ignoresubmodules, I don't
use them) should affect format-patch. Everything else undermines the
guarantee (by having a consistent format) that format-patch|am works.
So now I'm not so sure about diff.renames. Perhaps it needs to be
retained, but that requires a special case since we cannot move it to
git_diff_basic_config() (which affects diff-* plumbing too).
In any case I also made a test. If you decide to go for the original
patch, please feel free to "steal" it.
builtin/log.c | 20 ++++++++++++++++++--
t/t3400-rebase.sh | 7 +++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index f2d9d61..a1079fe 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -385,8 +385,19 @@ static int cmd_log_walk(struct rev_info *rev)
return diff_result_code(&rev->diffopt, 0);
}
+struct log_config_cb_data
+{
+ /*
+ * If no_diff_ui_config is set, we use diff_basic_config
+ * instead, ignoring the plumbing-specific UI settings.
+ */
+ int no_diff_ui_config;
+};
+
static int git_log_config(const char *var, const char *value, void *cb)
{
+ struct log_config_cb_data *cb_data = cb;
+
if (!strcmp(var, "format.pretty"))
return git_config_string(&fmt_pretty, var, value);
if (!strcmp(var, "format.subjectprefix"))
@@ -406,7 +417,10 @@ static int git_log_config(const char *var, const char *value, void *cb)
if (!prefixcmp(var, "color.decorate."))
return parse_decorate_color_config(var, 15, value);
- return git_diff_ui_config(var, value, cb);
+ if (!cb_data || !cb_data->no_diff_ui_config)
+ return git_diff_ui_config(var, value, cb);
+ else
+ return git_diff_basic_config(var, value, cb);
}
int cmd_whatchanged(int argc, const char **argv, const char *prefix)
@@ -1099,6 +1113,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
char *add_signoff = NULL;
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
+ struct log_config_cb_data config_cb_data;
const struct option builtin_format_patch_options[] = {
{ OPTION_CALLBACK, 'n', "numbered", &numbered, NULL,
"use [PATCH n/m] even with a single patch",
@@ -1160,7 +1175,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
extra_hdr.strdup_strings = 1;
extra_to.strdup_strings = 1;
extra_cc.strdup_strings = 1;
- git_config(git_format_config, NULL);
+ config_cb_data.no_diff_ui_config = 1;
+ git_config(git_format_config, &config_cb_data);
init_revisions(&rev, prefix);
rev.commit_format = CMIT_FMT_EMAIL;
rev.verbose_header = 1;
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 349eebd..0e2fe71 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -144,6 +144,13 @@ test_expect_success 'rebase is not broken by diff.renames' '
GIT_TRACE=1 git rebase force-3way
'
+test_expect_success 'rebase is not broken by diff.noprefix' '
+ git config diff.noprefix true &&
+ test_when_finished "git config --unset diff.noprefix" &&
+ git checkout -b noprefix side &&
+ GIT_TRACE=1 git rebase master
+'
+
test_expect_success 'setup: recover' '
test_might_fail git rebase --abort &&
git reset --hard &&
--
1.7.3.rc0.289.gcd076
next prev parent reply other threads:[~2010-09-09 8:37 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-08 13:54 [PATCH] Add explicit --src/dst-prefix to git-formt-patch in git-rebase.sh for the case of "diff.noprefix" in git-config Oded Shimon
2010-09-08 20:31 ` ods15
2010-09-08 21:07 ` Jan Krüger
2010-09-09 8:07 ` [PATCH] Add --src/dst-prefix to git-formt-patch in git-rebase.sh Oded Shimon
2010-09-09 8:36 ` Thomas Rast [this message]
2010-09-09 19:13 ` [Alt. PATCH] format-patch: do not use diff UI config Sverre Rabbelier
2010-09-09 19:43 ` Jeff King
2010-09-10 16:21 ` Junio C Hamano
2010-09-09 18:35 ` [PATCH] Add --src/dst-prefix to git-formt-patch in git-rebase.sh Junio C Hamano
2010-09-09 18:49 ` ods15
2010-09-09 18:49 ` Oded Shimon
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2a6b8c51903fd6a22606b8f592b1a2e11ea68741.1284020917.git.trast@student.ethz.ch \
--to=trast@student.ethz.ch \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jk@jk.gs \
--cc=ods15@ods15.dyndns.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.
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).