threads / patch / 21775

v2, 3 partsformat-patch: make full-diff enabled by default

Subject: [PATCH v2 2/3] format-patch: make full-diff enabled by default

## tl;dr

6 messages between Nov 28, 2009 and Jan 5, 2010. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Felipe Contreras· Nov 28, 2009, 11:40 UTC · lore

[PATCH v2 0/3] format-patch: improve pathspec usage

Even though it's not documented, users still can make use of pathspecs in 'git format-patch'. This patch series improves the way it works.

First, parse_options must not eat the '--', then, full-diff is the only way pathspecs make sense.

This is v2, after comments from Junio. Still pending are the improvements in documentation to explain how pathspecs make sense.

Felipe Contreras (3):
  format-patch: fix dashdash usage
  format-patch: make full-diff enabled by default
  format-patch: add test for dashdash
 builtin-log.c           |    4 +++-
 t/t4014-format-patch.sh |    4 ++++
 2 files changed, 7 insertions(+), 1 deletions(-)
Felipe Contreras· Nov 28, 2009, 11:40 UTC · re: Felipe Contreras · lore

[PATCH v2 1/3] format-patch: fix dashdash usage

$ git format-patch <committish> -- <path-not-in-working-dir>

Doesn't work otherwise. The current code would complain that the path is not in the working tree and that "--" must be specified, even if the user _did_ specify it. This happens because "--" is removed from the arguments, so we need to pass PARSE_OPT_KEEP_DASHDASH to parse_options to avoid that.

Comments by Junio C Hamano.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin-log.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to builtin-log.c +2 −1
diff --git a/builtin-log.c b/builtin-log.c
index 33fa6ea..1766349 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -976,7 +976,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	 */
 	argc = parse_options(argc, argv, prefix, builtin_format_patch_options,
 			     builtin_format_patch_usage,
-			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
+			     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
+			     PARSE_OPT_KEEP_DASHDASH);
 
 	if (do_signoff) {
 		const char *committer;

-- 
1.6.6.rc0.59.g5117f7.dirty
Felipe Contreras· Nov 28, 2009, 11:40 UTC · re: Felipe Contreras · lore

It doesn't make much sense to generate partial patches (with some paths omitted).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin-log.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to builtin-log.c +1 −0
diff --git a/builtin-log.c b/builtin-log.c
index 1766349..1e06859 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -960,6 +960,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	rev.diff = 1;
 	rev.combine_merges = 0;
 	rev.ignore_merges = 1;
+	rev.full_diff = 1;
 	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
 
 	rev.subject_prefix = fmt_patch_subject_prefix;

-- 
1.6.6.rc0.59.g5117f7.dirty
Felipe Contreras· Nov 28, 2009, 11:40 UTC · re: Felipe Contreras · lore

[PATCH v2 3/3] format-patch: add test for dashdash

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t4014-format-patch.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to t/t4014-format-patch.sh +4 −0
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7f267f9..d5b002d 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -536,6 +536,10 @@ test_expect_success 'format-patch --signoff' '
 	grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 '
 
+test_expect_success 'format-patch -- <path>' '
+	git format-patch master..side -- file
+'
+
 echo "fatal: --name-only does not make sense" > expect.name-only
 echo "fatal: --name-status does not make sense" > expect.name-status
 echo "fatal: --check does not make sense" > expect.check

-- 
1.6.6.rc0.59.g5117f7.dirty
Junio C Hamano· Nov 28, 2009, 19:47 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/3] format-patch: improve pathspec usage

Thanks; I think I have queued the equivalent already and pushed out last night.

I think making the full-diff default is a regression without an ability to turn it off and is a bit premature without a solid discussion in the documentation.

← back to recent threads