From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: combined diff, but not condensed, howto?
Date: Thu, 18 Sep 2008 00:01:39 -0700 [thread overview]
Message-ID: <7vskryym24.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <48D1F426.4040208@viscovery.net> (Johannes Sixt's message of "Thu, 18 Sep 2008 08:24:38 +0200")
Johannes Sixt <j.sixt@viscovery.net> writes:
> Junio C Hamano schrieb:
> ...
>> It all happens inside combine-diff.c::make_hunks(). If you pass dense==0,
>> you should be able to get all the uninteresting hunks, I think.
>>
>> Perhaps
>>
>> $ git diff --base -c
>
> Yes, that does it!
It may do so, but I do not necessarily agree with the logic.
It is very much Ok and useful for "git diff" to default to "--cc -p", and
it may be fine for "git diff-files -p" to default to "--cc".
But I think ideally an explicit "git diff -c" and "git diff-files -c -p"
should not turn the "dense combined" mode on.
The commonality of these two functions is striking and calls for proper
refactoring, but aside from that, I think a patch like this should be
applied. What do you think?
---
builtin-diff-files.c | 7 ++++++-
builtin-diff.c | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git i/builtin-diff-files.c w/builtin-diff-files.c
index 9bf10bb..2b578c7 100644
--- i/builtin-diff-files.c
+++ w/builtin-diff-files.c
@@ -50,7 +50,12 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
3 < rev.max_count)
usage(diff_files_usage);
- if (rev.max_count == -1 &&
+ /*
+ * "diff-files --base -p" should not combine merges because it
+ * was not asked to. "diff-files -c -p" should not densify
+ * (the user should ask with "diff-files --cc" explicitly).
+ */
+ if (rev.max_count == -1 && !rev.combine_merges &&
(rev.diffopt.output_format & DIFF_FORMAT_PATCH))
rev.combine_merges = rev.dense_combined_merges = 1;
diff --git i/builtin-diff.c w/builtin-diff.c
index 037c303..9fb30c6 100644
--- i/builtin-diff.c
+++ w/builtin-diff.c
@@ -223,7 +223,12 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
argv++; argc--;
}
- if (revs->max_count == -1 &&
+ /*
+ * "diff --base" should not combine merges because it was not
+ * asked to. "diff -c" should not densify (the user should ask
+ * with "diff --cc" explicitly.
+ */
+ if (revs->max_count == -1 && !revs->combine_merges &&
(revs->diffopt.output_format & DIFF_FORMAT_PATCH))
revs->combine_merges = revs->dense_combined_merges = 1;
next prev parent reply other threads:[~2008-09-18 7:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-17 8:00 combined diff, but not condensed, howto? Johannes Sixt
2008-09-17 16:58 ` Junio C Hamano
2008-09-18 5:57 ` Johannes Sixt
2008-09-18 6:14 ` Junio C Hamano
2008-09-18 6:24 ` Johannes Sixt
2008-09-18 7:01 ` Junio C Hamano [this message]
2008-09-18 7:30 ` Johannes Sixt
2008-09-18 9:08 ` [PATCH] diff/diff-files: do not use --cc too aggressively Junio C Hamano
2008-09-18 9:30 ` Johannes Sixt
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=7vskryym24.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.