From: Junio C Hamano <hidden> Date: 2016-06-15 22:58:10
Matthieu Moy [off-list ref] writes:
Junio C Hamano [off-list ref] writes:
quoted
I am wondering if the difference after this patch between "-p" and
"-U8" is deliberate, or just an accident coming from the way the
original was written in ee1e5412 (git diff: support "-U" and
"--unified" options properly, 2006-05-13).
No, it isn't. I just didn't notice the -U case.
quoted
If the original were written in this way:
if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch") ||
opt_arg(arg, 'U', "unified", &options->context))
options->output_format |= DIFF_FORMAT_PATCH;
Yes, this seems to be a better way.
There are other cases like --patch-with-raw, I'll send a reroll.
This fixes the issue found by Junio where "git log --no-patch -u" was
showing the patch, but not "git log --no-patch -U8". Other patches are
unmodified.
Matthieu Moy (5):
diff: allow --no-patch as synonym for -s
diff: allow --patch & cie to override -s/--no-patch
Documentation/git-show.txt: include common diff options, like
git-log.txt
Documentation: move description of -s, --no-patch to diff-options.txt
Documentation/git-log.txt: capitalize section names
Documentation/diff-options.txt | 5 +++++
Documentation/git-log.txt | 8 ++++----
Documentation/git-show.txt | 9 +++++++++
Documentation/rev-list-options.txt | 3 ---
diff.c | 30 ++++++++++++++++++------------
5 files changed, 36 insertions(+), 19 deletions(-)
--
1.8.3.1.495.g13f33cf.dirty
This is the convention in other files and even at the beginning of git-log.txt
Signed-off-by: Matthieu Moy <redacted>
---
Documentation/git-log.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
@@ -161,12 +161,12 @@ git log -L '/int main/',/^}/:main.c:: `git log -3`:: Limits the number of commits to show to 3.-Discussion+DISCUSSION ---------- include::i18n.txt[]-Configuration+CONFIGURATION ------------- See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
Technically, "-s, --no-patch" is implemented in diff.c ("git diff
--no-patch" is essentially useless, but valid). From the user point of
view, this allows the documentation to show up in "git show --help",
which is one of the most useful use of the option.
While we're there, add a sentence explaining why the option can be
useful.
Signed-off-by: Matthieu Moy <redacted>
---
Documentation/diff-options.txt | 5 +++++
Documentation/rev-list-options.txt | 4 ----
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -26,6 +26,11 @@ ifndef::git-format-patch[] {git-diff? This is the default.} endif::git-format-patch[]+-s::+--no-patch::+ Suppress diff output. Useful for commands like `git show` that+ show the patch by default, or to cancel the effect of `--patch`.+ -U<n>:: --unified=<n>:: Generate diffs with <n> lines of context instead of
@@ -849,8 +849,4 @@ options may be given. See linkgit:git-diff-files[1] for more options. -t:: Show the tree objects in the diff output. This implies '-r'.---s::---no-patch::- Suppress diff output. endif::git-rev-list[]
@@ -851,5 +851,6 @@ options may be given. See linkgit:git-diff-files[1] for more options. Show the tree objects in the diff output. This implies '-r'. -s::+--no-patch:: Suppress diff output. endif::git-rev-list[]
@@ -59,4 +59,18 @@ test_expect_success \'validate git diff-files -p output.'\'compare_diff_patch current expected'+test_expect_success\+'git diff-files -s after editing work tree'\+'>empty&&+gitdiff-files-s>diff-s-output2>err&&+test_cmpemptydiff-s-output&&+test_cmpemptyerr'++test_expect_success\+'git diff-files --no-patch as synonym for -s'\+'>empty&&+gitdiff-files--no-patch>diff-np-output2>err&&+test_cmpemptydiff-np-output&&+test_cmpemptyerr'+ test_done
Compared to v2, I just added tests. Strongly inspired from Jonathan's,
but there's one more, and I chose the "modern" indentation style
(hence a clean-up patch before, to avoid mixed-style in the same file).
Matthieu Moy (6):
t4000-diff-format.sh: modernize style
diff: allow --no-patch as synonym for -s
diff: allow --patch & cie to override -s/--no-patch
Documentation/git-show.txt: include common diff options, like
git-log.txt
Documentation: move description of -s, --no-patch to diff-options.txt
Documentation/git-log.txt: capitalize section names
Documentation/diff-options.txt | 5 +++++
Documentation/git-log.txt | 8 ++++----
Documentation/git-show.txt | 9 +++++++++
Documentation/rev-list-options.txt | 3 ---
diff.c | 30 ++++++++++++++++++------------
t/t4000-diff-format.sh | 35 ++++++++++++++++++++++++++---------
6 files changed, 62 insertions(+), 28 deletions(-)
--
1.8.3.1.495.g13f33cf.dirty
@@ -15,17 +15,17 @@ line 3' catpath0>path1 chmod+xpath1-test_expect_success\-'update-index --add two files with and without +x.'\-'git update-index --add path0 path1'+test_expect_success'update-index --add two files with and without +x.''+gitupdate-index--addpath0path1+' mvpath0path0- sed-e's/line/Line/'<path0->path0 chmod+xpath0 rm-fpath1-test_expect_success\-'git diff-files -p after editing work tree.'\-'git diff-files -p >current'+test_expect_success'git diff-files -p after editing work tree.''+gitdiff-files-p>actual+'# that's as far as it comesif["$(gitconfig--getcore.filemode)"=false]
This test script can use more cleanup, but as preparation for later
patches in this series the above is enough. :) If I forget to do more
cleanup as a followup, feel free to kick me.
Reviewed-by: Jonathan Nieder <redacted>
All options that trigger a patch output now override --no-patch.
The case of --binary is particular as the name may suggest that it turns
a normal patch into a binary patch, but it actually already enables patch
output when normally disabled (e.g. "git log --binary" displays a patch),
hence it makes sense that "git show --no-patch --binary" display the
binary patch.
Signed-off-by: Matthieu Moy <redacted>
---
diff.c | 28 +++++++++++++++++-----------
t/t4000-diff-format.sh | 5 +++++
2 files changed, 22 insertions(+), 11 deletions(-)
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:58:10
Matthieu Moy wrote:
All options that trigger a patch output now override --no-patch.
The case of --binary is particular as the name may suggest that it turns
Usage nit: this should say "is unusual" or "In the case of --binary in
particular, the name may suggest ...".
a normal patch into a binary patch, but it actually already enables patch
output when normally disabled (e.g. "git log --binary" displays a patch),
hence it makes sense that "git show --no-patch --binary" display the
binary patch.
@@ -71,4 +71,9 @@ test_expect_success 'git diff-files --no-patch as synonym for -s' 'test_must_be_emptyerr'+test_expect_success'git diff-files --no-patch --patch shows the patch''+gitdiff-files--no-patch--patch>diff-np-output2>err&&+compare_diff_patchexpectedactual
Shouldn't that be "compare_diff_patch expected diff-np-output"?
A couple of other test ideas:
- "git diff-files --patch --no-patch"
- "git diff-files -s --patch-with-stat"
Aside from that, the patch looks good.
Thanks,
Jonathan
This is the convention in other files and even at the beginning of git-log.txt
Signed-off-by: Matthieu Moy <redacted>
---
Documentation/git-log.txt | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
@@ -161,12 +161,12 @@ git log -L '/int main/',/^}/:main.c:: `git log -3`:: Limits the number of commits to show to 3.-Discussion+DISCUSSION ---------- include::i18n.txt[]-Configuration+CONFIGURATION ------------- See linkgit:git-config[1] for core variables and linkgit:git-diff[1]
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:58:10
Matthieu Moy wrote:
This is the convention in other files and even at the beginning of git-log.txt
The docs aren't so consistent on this, but I agree that it makes sense
to at least be consistent within the generated git-log.html. :)
Generally the series looks very good. Thanks for taking this on.
@@ -851,5 +851,6 @@ options may be given. See linkgit:git-diff-files[1] for more options. Show the tree objects in the diff output. This implies '-r'. -s::+--no-patch:: Suppress diff output. endif::git-rev-list[]
Technically, "-s, --no-patch" is implemented in diff.c ("git diff
--no-patch" is essentially useless, but valid). From the user point of
view, this allows the documentation to show up in "git show --help",
which is one of the most useful use of the option.
While we're there, add a sentence explaining why the option can be
useful.
Signed-off-by: Matthieu Moy <redacted>
---
Documentation/diff-options.txt | 5 +++++
Documentation/rev-list-options.txt | 4 ----
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -26,6 +26,11 @@ ifndef::git-format-patch[] {git-diff? This is the default.} endif::git-format-patch[]+-s::+--no-patch::+ Suppress diff output. Useful for commands like `git show` that+ show the patch by default, or to cancel the effect of `--patch`.+ -U<n>:: --unified=<n>:: Generate diffs with <n> lines of context instead of
@@ -849,8 +849,4 @@ options may be given. See linkgit:git-diff-files[1] for more options. -t:: Show the tree objects in the diff output. This implies '-r'.---s::---no-patch::- Suppress diff output. endif::git-rev-list[]
All options that trigger a patch output now override --no-patch.
The case of --binary is particular as the name may suggest that it turns
a normal patch into a binary patch, but it actually already enables patch
output when normally disabled (e.g. "git log --binary" displays a patch),
hence it makes sense that "git show --no-patch --binary" display the
binary patch.
Signed-off-by: Matthieu Moy <redacted>
---
This is the one which changed.
diff.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)