* [PATCH v1 0/7] diff: fix -s and --no-patch
@ 2023-05-12 8:03 Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 1/7] line-log: set patch format explicitly by default Felipe Contreras
` (7 more replies)
0 siblings, 8 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
The diff code assumes 0 means the default, and --no-patch means
NO_OUTPUT.
The problem with this approach is that it doesn't allow distinguishing
`git diff --no-patch`, `git diff --patch --no-patch`, and `git diff`.
By introducing a DIFF_FORMAT_DEFAULT (which is not 0) it's now possible
to properly distinguish these arguments, and get rid of
DIFF_FORMAT_NO_OUTPUT which should have never been considered a format,
but the absence of a format.
This fixes an issue Sergey Organov reported.
Up to patch #2 (diff: introduce DIFF_FORMAT_DEFAULT) there are no
functional changes, patch #3 (diff: make DIFF_FORMAT_NO_OUTPUT 0)
achieves the same as a series from Junio Hamano [1] except more
properly. Patch #4 adds a simplified version of Junio's test cases.
Then in patch #5 --no-patch is split from -s, making it work as
intended: negates --patch, but not other formats.
The rest are some niceties.
Now all these work correctly:
1. git diff --raw
2. git diff -s --raw
3. git diff --no-patch
4. git diff --no-patch --raw
5. git diff --patch --no-patch --raw
6. git diff --raw --patch --no-patch
[1] https://lore.kernel.org/git/20230505165952.335256-1-gitster@pobox.com/
Felipe Contreras (7):
line-log: set patch format explicitly by default
diff: introduce DIFF_FORMAT_DEFAULT
diff: make DIFF_FORMAT_NO_OUTPUT 0
test: add various tests for diff formats with -s
diff: split --no-patch from -s
diff: add --silent as alias of -s
diff: remove DIFF_FORMAT_NO_OUTPUT
Documentation/diff-options.txt | 6 ++--
blame.c | 6 ++--
builtin/diff-files.c | 2 +-
builtin/diff-index.c | 2 +-
builtin/diff-tree.c | 2 +-
builtin/diff.c | 2 +-
builtin/log.c | 16 +++++++---
builtin/stash.c | 4 +--
builtin/submodule--helper.c | 2 +-
combine-diff.c | 10 +++---
diff-merges.c | 2 +-
diff-no-index.c | 2 +-
diff.c | 56 ++++++++++++++++++----------------
diff.h | 6 +---
line-log.c | 2 +-
log-tree.c | 4 +--
merge-ort.c | 4 +--
merge-recursive.c | 4 +--
notes-merge.c | 4 +--
range-diff.c | 4 +--
revision.c | 6 ++--
t/t4000-diff-format.sh | 26 ++++++++++++++--
tree-diff.c | 2 +-
23 files changed, 99 insertions(+), 75 deletions(-)
--
2.40.0+fc1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1 1/7] line-log: set patch format explicitly by default
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT Felipe Contreras
` (6 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
Will help further changes.
No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/log.c | 5 +++++
line-log.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 676de107d6..712bfbf5c2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -890,6 +890,11 @@ int cmd_log(int argc, const char **argv, const char *prefix)
opt.revarg_opt = REVARG_COMMITTISH;
opt.tweak = log_setup_revisions_tweak;
cmd_log_init(argc, argv, prefix, &rev, &opt);
+
+ if (!rev.diffopt.output_format)
+ if (rev.line_level_traverse)
+ rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+
return cmd_log_deinit(cmd_log_walk(&rev), &rev);
}
diff --git a/line-log.c b/line-log.c
index 6a7ac312a4..7466366860 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1141,7 +1141,7 @@ int line_log_print(struct rev_info *rev, struct commit *commit)
{
show_log(rev);
- if (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT)) {
+ if (rev->diffopt.output_format & DIFF_FORMAT_PATCH) {
struct line_log_data *range = lookup_line_range(rev, commit);
dump_diff_hacky(rev, range);
}
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 1/7] line-log: set patch format explicitly by default Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0 Felipe Contreras
` (5 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
As the name suggests this is the default format, which means no format
was specified.
This is not the same as DIFF_FORMAT_PATCH, as some commands like `git
diff-files` use a different default.
This makes it possible to distinguish `git diff` (DEFAULT)
from `git diff --no-patch` (0).
Will help further changes.
There should be no functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/diff-files.c | 2 +-
builtin/diff-index.c | 2 +-
builtin/diff-tree.c | 2 +-
builtin/diff.c | 2 +-
builtin/log.c | 13 ++++++++-----
builtin/stash.c | 2 +-
diff-merges.c | 2 +-
diff-no-index.c | 2 +-
diff.c | 39 ++++++++++++++++++++-------------------
diff.h | 1 +
range-diff.c | 2 +-
revision.c | 4 ++--
12 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index dc991f753b..b831b89236 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -52,7 +52,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
usage(diff_files_usage);
argv++; argc--;
}
- if (!rev.diffopt.output_format)
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)
rev.diffopt.output_format = DIFF_FORMAT_RAW;
rev.diffopt.rotate_to_strict = 1;
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index b9a19bb7d3..863c51c9b5 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -48,7 +48,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
else
usage(diff_cache_usage);
}
- if (!rev.diffopt.output_format)
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)
rev.diffopt.output_format = DIFF_FORMAT_RAW;
rev.diffopt.rotate_to_strict = 1;
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 0b02c62b85..7e9164187c 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -100,7 +100,7 @@ COMMON_DIFF_OPTIONS_HELP;
static void diff_tree_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)
{
- if (!rev->diffopt.output_format) {
+ if (rev->diffopt.output_format == DIFF_FORMAT_DEFAULT) {
if (rev->dense_combined_merges)
rev->diffopt.output_format = DIFF_FORMAT_PATCH;
else
diff --git a/builtin/diff.c b/builtin/diff.c
index 7b64659fe7..2decf5e531 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -505,7 +505,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
if (nongit)
die(_("Not a git repository"));
argc = setup_revisions(argc, argv, &rev, NULL);
- if (!rev.diffopt.output_format) {
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
diff_setup_done(&rev.diffopt);
}
diff --git a/builtin/log.c b/builtin/log.c
index 712bfbf5c2..d2a81f36c2 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -277,7 +277,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
PARSE_OPT_KEEP_DASHDASH);
if (quiet)
- rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
+ rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
argc = setup_revisions(argc, argv, rev, opt);
/* Any arguments at this point are not recognized */
@@ -633,7 +633,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
opt.def = "HEAD";
opt.revarg_opt = REVARG_COMMITTISH;
cmd_log_init(argc, argv, prefix, &rev, &opt);
- if (!rev.diffopt.output_format)
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)
rev.diffopt.output_format = DIFF_FORMAT_RAW;
return cmd_log_deinit(cmd_log_walk(&rev), &rev);
}
@@ -725,7 +725,7 @@ static void show_setup_revisions_tweak(struct rev_info *rev,
diff_merges_default_to_first_parent(rev);
else
diff_merges_default_to_dense_combined(rev);
- if (!rev->diffopt.output_format)
+ if (rev->diffopt.output_format == DIFF_FORMAT_DEFAULT)
rev->diffopt.output_format = DIFF_FORMAT_PATCH;
}
@@ -891,9 +891,12 @@ int cmd_log(int argc, const char **argv, const char *prefix)
opt.tweak = log_setup_revisions_tweak;
cmd_log_init(argc, argv, prefix, &rev, &opt);
- if (!rev.diffopt.output_format)
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {
if (rev.line_level_traverse)
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+ else
+ rev.diffopt.output_format = 0;
+ }
return cmd_log_deinit(cmd_log_walk(&rev), &rev);
}
@@ -2126,7 +2129,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
die(_("--remerge-diff does not make sense"));
if (!use_patch_format &&
- (!rev.diffopt.output_format ||
+ (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT ||
rev.diffopt.output_format == DIFF_FORMAT_PATCH))
rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;
if (!rev.diffopt.stat_width)
diff --git a/builtin/stash.c b/builtin/stash.c
index a7e17ffe38..398e3c9f61 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -944,7 +944,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)
argc = setup_revisions(revision_args.nr, revision_args.v, &rev, NULL);
if (argc > 1)
goto usage;
- if (!rev.diffopt.output_format) {
+ if (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {
rev.diffopt.output_format = DIFF_FORMAT_PATCH;
diff_setup_done(&rev.diffopt);
}
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1..9960d7cc36 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -183,7 +183,7 @@ void diff_merges_setup_revs(struct rev_info *revs)
if (revs->merges_imply_patch)
revs->diff = 1;
if (revs->merges_imply_patch || revs->merges_need_diff) {
- if (!revs->diffopt.output_format)
+ if (revs->diffopt.output_format == DIFF_FORMAT_DEFAULT)
revs->diffopt.output_format = DIFF_FORMAT_PATCH;
}
}
diff --git a/diff-no-index.c b/diff-no-index.c
index 4296940f90..45596cb1be 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -282,7 +282,7 @@ int diff_no_index(struct rev_info *revs,
fixup_paths(paths, &replacement);
revs->diffopt.skip_stat_unmatch = 1;
- if (!revs->diffopt.output_format)
+ if (revs->diffopt.output_format == DIFF_FORMAT_DEFAULT)
revs->diffopt.output_format = DIFF_FORMAT_PATCH;
revs->diffopt.flags.no_index = 1;
diff --git a/diff.c b/diff.c
index 71513d92e8..387944f289 100644
--- a/diff.c
+++ b/diff.c
@@ -4669,6 +4669,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
options->file = stdout;
options->repo = r;
+ options->output_format = DIFF_FORMAT_DEFAULT;
options->output_indicators[OUTPUT_INDICATOR_NEW] = '+';
options->output_indicators[OUTPUT_INDICATOR_OLD] = '-';
options->output_indicators[OUTPUT_INDICATOR_CONTEXT] = ' ';
@@ -4987,7 +4988,7 @@ static int diff_opt_diff_filter(const struct option *option,
static void enable_patch_output(int *fmt)
{
- *fmt &= ~DIFF_FORMAT_NO_OUTPUT;
+ *fmt &= ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT);
*fmt |= DIFF_FORMAT_PATCH;
}
@@ -5492,13 +5493,13 @@ struct option *add_diff_options(const struct option *opts,
OPT_GROUP(N_("Diff output format options")),
OPT_BITOP('p', "patch", &options->output_format,
N_("generate patch"),
- DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
- OPT_BIT_F('s', "no-patch", &options->output_format,
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ OPT_BITOP('s', "no-patch", &options->output_format,
N_("suppress diff output"),
- DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),
+ DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH),
OPT_BITOP('u', NULL, &options->output_format,
N_("generate patch"),
- DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
OPT_CALLBACK_F('U', "unified", options, N_("<n>"),
N_("generate diffs with <n> lines context"),
PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified),
@@ -5510,17 +5511,17 @@ struct option *add_diff_options(const struct option *opts,
OPT_BITOP(0, "patch-with-raw", &options->output_format,
N_("synonym for '-p --raw'"),
DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW,
- DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
OPT_BITOP(0, "patch-with-stat", &options->output_format,
N_("synonym for '-p --stat'"),
DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT,
- DIFF_FORMAT_NO_OUTPUT),
- OPT_BIT_F(0, "numstat", &options->output_format,
+ DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ OPT_BITOP(0, "numstat", &options->output_format,
N_("machine friendly --stat"),
- DIFF_FORMAT_NUMSTAT, PARSE_OPT_NONEG),
- OPT_BIT_F(0, "shortstat", &options->output_format,
+ DIFF_FORMAT_NUMSTAT, DIFF_FORMAT_DEFAULT),
+ OPT_BITOP(0, "shortstat", &options->output_format,
N_("output only the last line of --stat"),
- DIFF_FORMAT_SHORTSTAT, PARSE_OPT_NONEG),
+ DIFF_FORMAT_SHORTSTAT, DIFF_FORMAT_DEFAULT),
OPT_CALLBACK_F('X', "dirstat", options, N_("<param1,param2>..."),
N_("output the distribution of relative amount of changes for each sub-directory"),
PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
@@ -5533,18 +5534,18 @@ struct option *add_diff_options(const struct option *opts,
N_("synonym for --dirstat=files,param1,param2..."),
PARSE_OPT_NONEG | PARSE_OPT_OPTARG,
diff_opt_dirstat),
- OPT_BIT_F(0, "check", &options->output_format,
+ OPT_BITOP(0, "check", &options->output_format,
N_("warn if changes introduce conflict markers or whitespace errors"),
- DIFF_FORMAT_CHECKDIFF, PARSE_OPT_NONEG),
- OPT_BIT_F(0, "summary", &options->output_format,
+ DIFF_FORMAT_CHECKDIFF, DIFF_FORMAT_DEFAULT),
+ OPT_BITOP(0, "summary", &options->output_format,
N_("condensed summary such as creations, renames and mode changes"),
- DIFF_FORMAT_SUMMARY, PARSE_OPT_NONEG),
- OPT_BIT_F(0, "name-only", &options->output_format,
+ DIFF_FORMAT_SUMMARY, DIFF_FORMAT_DEFAULT),
+ OPT_BITOP(0, "name-only", &options->output_format,
N_("show only names of changed files"),
- DIFF_FORMAT_NAME, PARSE_OPT_NONEG),
- OPT_BIT_F(0, "name-status", &options->output_format,
+ DIFF_FORMAT_NAME, DIFF_FORMAT_DEFAULT),
+ OPT_BITOP(0, "name-status", &options->output_format,
N_("show only names and status of changed files"),
- DIFF_FORMAT_NAME_STATUS, PARSE_OPT_NONEG),
+ DIFF_FORMAT_NAME_STATUS, DIFF_FORMAT_DEFAULT),
OPT_CALLBACK_F(0, "stat", options, N_("<width>[,<name-width>[,<count>]]"),
N_("generate diffstat"),
PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_stat),
diff --git a/diff.h b/diff.h
index 3a7a9e8b88..15a7bf2c9f 100644
--- a/diff.h
+++ b/diff.h
@@ -101,6 +101,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_FORMAT_PATCH 0x0010
#define DIFF_FORMAT_SHORTSTAT 0x0020
#define DIFF_FORMAT_DIRSTAT 0x0040
+#define DIFF_FORMAT_DEFAULT 0x0080
/* These override all above */
#define DIFF_FORMAT_NAME 0x0100
diff --git a/range-diff.c b/range-diff.c
index 6a704e6f47..6c1ae9dd34 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -492,7 +492,7 @@ static void output(struct string_list *a, struct string_list *b,
repo_diff_setup(the_repository, &opts);
opts.no_free = 1;
- if (!opts.output_format)
+ if (opts.output_format == DIFF_FORMAT_DEFAULT)
opts.output_format = DIFF_FORMAT_PATCH;
opts.flags.suppress_diff_headers = 1;
opts.flags.dual_color_diffed_diffs =
diff --git a/revision.c b/revision.c
index b33cc1d106..cf68b533fd 100644
--- a/revision.c
+++ b/revision.c
@@ -2966,7 +2966,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
/* Did the user ask for any diff output? Run the diff! */
- if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)
+ if (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT))
revs->diff = 1;
/* Pickaxe, diff-filter and rename following need diffs */
@@ -3030,7 +3030,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
if (revs->line_level_traverse &&
- (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
+ (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
die(_("-L does not yet support diff formats besides -p and -s"));
if (revs->expand_tabs_in_log < 0)
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 1/7] line-log: set patch format explicitly by default Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 4/7] test: add various tests for diff formats with -s Felipe Contreras
` (4 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
`-s` is considered a format, but if we change the meaning to absence of
a format it now becomes possible to distinguish `git diff` from
`git diff --no-patch`, although that isn't done in this commit.
This also fixes a bug in which specifying an output format did not clear
the NO_OUTPUT flag.
For example this now works correctly:
git show -s --raw
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
diff.c | 13 ++++++-------
diff.h | 6 +-----
range-diff.c | 2 +-
revision.c | 4 ++--
4 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/diff.c b/diff.c
index 387944f289..4f4b1d7e13 100644
--- a/diff.c
+++ b/diff.c
@@ -4750,8 +4750,7 @@ void diff_setup_done(struct diff_options *options)
{
unsigned check_mask = DIFF_FORMAT_NAME |
DIFF_FORMAT_NAME_STATUS |
- DIFF_FORMAT_CHECKDIFF |
- DIFF_FORMAT_NO_OUTPUT;
+ DIFF_FORMAT_CHECKDIFF;
/*
* This must be signed because we're comparing against a potentially
* negative value.
@@ -4762,8 +4761,8 @@ void diff_setup_done(struct diff_options *options)
options->set_default(options);
if (HAS_MULTI_BITS(options->output_format & check_mask))
- die(_("options '%s', '%s', '%s', and '%s' cannot be used together"),
- "--name-only", "--name-status", "--check", "-s");
+ die(_("options '%s', '%s', and '%s' cannot be used together"),
+ "--name-only", "--name-status", "--check");
if (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))
die(_("options '%s', '%s', and '%s' cannot be used together"),
@@ -5494,9 +5493,9 @@ struct option *add_diff_options(const struct option *opts,
OPT_BITOP('p', "patch", &options->output_format,
N_("generate patch"),
DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
- OPT_BITOP('s', "no-patch", &options->output_format,
+ OPT_SET_INT_F('s', "no-patch", &options->output_format,
N_("suppress diff output"),
- DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH),
+ DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),
OPT_BITOP('u', NULL, &options->output_format,
N_("generate patch"),
DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
@@ -6646,7 +6645,7 @@ void diff_flush(struct diff_options *options)
separator++;
}
- if (output_format & DIFF_FORMAT_NO_OUTPUT &&
+ if (output_format == DIFF_FORMAT_NO_OUTPUT &&
options->flags.exit_with_status &&
options->flags.diff_from_contents) {
/*
diff --git a/diff.h b/diff.h
index 15a7bf2c9f..44da1a4ca7 100644
--- a/diff.h
+++ b/diff.h
@@ -94,6 +94,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);
+#define DIFF_FORMAT_NO_OUTPUT 0x0000
#define DIFF_FORMAT_RAW 0x0001
#define DIFF_FORMAT_DIFFSTAT 0x0002
#define DIFF_FORMAT_NUMSTAT 0x0004
@@ -108,11 +109,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_FORMAT_NAME_STATUS 0x0200
#define DIFF_FORMAT_CHECKDIFF 0x0400
-/* Same as output_format = 0 but we know that -s flag was given
- * and we should not give default value to output_format.
- */
-#define DIFF_FORMAT_NO_OUTPUT 0x0800
-
#define DIFF_FORMAT_CALLBACK 0x1000
#define DIFF_FLAGS_INIT { 0 }
diff --git a/range-diff.c b/range-diff.c
index 6c1ae9dd34..00ff5dc160 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -542,7 +542,7 @@ static void output(struct string_list *a, struct string_list *b,
a_util = a->items[b_util->matching].util;
output_pair_header(&opts, patch_no_width,
&buf, &dashes, a_util, b_util);
- if (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))
+ if (opts.output_format)
patch_diff(a->items[b_util->matching].string,
b->items[j].string, &opts);
a_util->shown = 1;
diff --git a/revision.c b/revision.c
index cf68b533fd..07d653c197 100644
--- a/revision.c
+++ b/revision.c
@@ -3030,8 +3030,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
if (revs->line_level_traverse &&
- (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
- die(_("-L does not yet support diff formats besides -p and -s"));
+ (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH)))
+ die(_("-L does not yet support diff formats besides -p"));
if (revs->expand_tabs_in_log < 0)
revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 4/7] test: add various tests for diff formats with -s
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
` (2 preceding siblings ...)
2023-05-12 8:03 ` [PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0 Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 5/7] diff: split --no-patch from -s Felipe Contreras
` (3 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
There used to be a bug when -s was used with different formats, for
example `-s --raw`.
Originally-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/t4000-diff-format.sh | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index bfcaae390f..7829cc810d 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -91,4 +91,23 @@ test_expect_success 'git diff-files --patch --no-patch does not show the patch'
test_must_be_empty err
'
+
+echo 'reset' >path1
+
+for format in stat raw numstat shortstat summary dirstat cumulative \
+ dirstat-by-file patch-with-raw patch-with-stat compact-summary
+do
+ test_expect_success "-s before --$format' is a no-op" '
+ git diff-files -s "--$format" >actual &&
+ git diff-files "--$format" >expect &&
+ test_cmp expect actual
+ '
+
+ test_expect_success "-s clears --$format" '
+ git diff-files --$format -s --patch >actual &&
+ git diff-files --patch >expect &&
+ test_cmp expect actual
+ '
+done
+
test_done
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 5/7] diff: split --no-patch from -s
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
` (3 preceding siblings ...)
2023-05-12 8:03 ` [PATCH v1 4/7] test: add various tests for diff formats with -s Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 6/7] diff: add --silent as alias of -s Felipe Contreras
` (2 subsequent siblings)
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
It should silence only the --patch output, not all output.
When --no-patch was introduced in d09cd15d19 (diff: allow --no-patch as
synonym for -s, 2013-07-16), the idea was to have a more accessible
shortcut to silence the output of `git show`.
However, the interaction with other options was not considered, for
example `--raw --no-patch`.
The original intention remains, as `git show --no-patch` still produces
the same: no output.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/diff-options.txt | 5 ++---
diff.c | 5 ++++-
t/t4000-diff-format.sh | 7 ++++---
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 08ab86189a..ba04b8292a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -21,7 +21,7 @@ endif::git-format-patch[]
ifndef::git-format-patch[]
-p::
-u::
---patch::
+--[no-]patch::
Generate patch (see section titled
ifdef::git-log[]
<<generate_patch_text_with_p, "Generating patch text with -p">>).
@@ -34,9 +34,8 @@ ifdef::git-diff[]
endif::git-diff[]
-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`.
+ show output by default.
endif::git-format-patch[]
ifdef::git-log[]
diff --git a/diff.c b/diff.c
index 4f4b1d7e13..45c860496f 100644
--- a/diff.c
+++ b/diff.c
@@ -5493,9 +5493,12 @@ struct option *add_diff_options(const struct option *opts,
OPT_BITOP('p', "patch", &options->output_format,
N_("generate patch"),
DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
- OPT_SET_INT_F('s', "no-patch", &options->output_format,
+ OPT_SET_INT_F('s', NULL, &options->output_format,
N_("suppress diff output"),
DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),
+ OPT_BITOP(0, "no-patch", &options->output_format,
+ N_("negate --patch"),
+ DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),
OPT_BITOP('u', NULL, &options->output_format,
N_("generate patch"),
DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
diff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
index 7829cc810d..d7b9a2dab8 100755
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -67,9 +67,10 @@ test_expect_success 'git diff-files -s after editing work tree' '
test_must_be_empty err
'
-test_expect_success 'git diff-files --no-patch as synonym for -s' '
- git diff-files --no-patch >actual 2>err &&
- test_must_be_empty actual &&
+test_expect_success 'git diff-files --no-patch negates --patch' '
+ git diff-files >expected_raw &&
+ git diff-files --raw --patch --no-patch >actual 2>err &&
+ test_cmp expected_raw actual &&
test_must_be_empty err
'
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 6/7] diff: add --silent as alias of -s
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
` (4 preceding siblings ...)
2023-05-12 8:03 ` [PATCH v1 5/7] diff: split --no-patch from -s Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT Felipe Contreras
2023-05-12 8:20 ` [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
Ever since 2e5e98a40 ([PATCH] Silent flag for show-diff, 2005-04-13) -s
meant silent.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/diff-options.txt | 1 +
diff.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ba04b8292a..a402b6f9b6 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -34,6 +34,7 @@ ifdef::git-diff[]
endif::git-diff[]
-s::
+--silent::
Suppress diff output. Useful for commands like `git show` that
show output by default.
endif::git-format-patch[]
diff --git a/diff.c b/diff.c
index 45c860496f..d171f155b1 100644
--- a/diff.c
+++ b/diff.c
@@ -5493,7 +5493,7 @@ struct option *add_diff_options(const struct option *opts,
OPT_BITOP('p', "patch", &options->output_format,
N_("generate patch"),
DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
- OPT_SET_INT_F('s', NULL, &options->output_format,
+ OPT_SET_INT_F('s', "silent", &options->output_format,
N_("suppress diff output"),
DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),
OPT_BITOP(0, "no-patch", &options->output_format,
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
` (5 preceding siblings ...)
2023-05-12 8:03 ` [PATCH v1 6/7] diff: add --silent as alias of -s Felipe Contreras
@ 2023-05-12 8:03 ` Felipe Contreras
2023-05-12 8:20 ` [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
7 siblings, 0 replies; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:03 UTC (permalink / raw)
To: git; +Cc: Sergey Organov, Junio C Hamano, Felipe Contreras
Instead use an empty output_format (0) as NO_OUTPUT.
No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
blame.c | 6 +++---
builtin/log.c | 2 +-
builtin/stash.c | 2 +-
builtin/submodule--helper.c | 2 +-
combine-diff.c | 10 ++++------
diff.c | 21 ++++++++++-----------
diff.h | 1 -
log-tree.c | 4 ++--
merge-ort.c | 4 ++--
merge-recursive.c | 4 ++--
notes-merge.c | 4 ++--
revision.c | 2 +-
tree-diff.c | 2 +-
13 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/blame.c b/blame.c
index b830654062..d382af6798 100644
--- a/blame.c
+++ b/blame.c
@@ -1337,7 +1337,7 @@ static struct blame_origin *find_origin(struct repository *r,
repo_diff_setup(r, &diff_opts);
diff_opts.flags.recursive = 1;
diff_opts.detect_rename = 0;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
paths[0] = origin->path;
paths[1] = NULL;
@@ -1420,7 +1420,7 @@ static struct blame_origin *find_rename(struct repository *r,
repo_diff_setup(r, &diff_opts);
diff_opts.flags.recursive = 1;
diff_opts.detect_rename = DIFF_DETECT_RENAME;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_opts.single_follow = origin->path;
diff_setup_done(&diff_opts);
@@ -2242,7 +2242,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
repo_diff_setup(sb->repo, &diff_opts);
diff_opts.flags.recursive = 1;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_setup_done(&diff_opts);
diff --git a/builtin/log.c b/builtin/log.c
index d2a81f36c2..4fbdbd349d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -277,7 +277,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
PARSE_OPT_KEEP_DASHDASH);
if (quiet)
- rev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+ rev->diffopt.output_format = 0;
argc = setup_revisions(argc, argv, rev, opt);
/* Any arguments at this point are not recognized */
diff --git a/builtin/stash.c b/builtin/stash.c
index 398e3c9f61..d7a4ade8d4 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -437,7 +437,7 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
repo_diff_setup(the_repository, &diff_opts);
diff_opts.flags.recursive = 1;
diff_opts.detect_rename = 0;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_setup_done(&diff_opts);
do_diff_cache(orig_tree, &diff_opts);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6bf8d666ce..885cd57e90 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1120,7 +1120,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
rev.abbrev = 0;
precompose_argv_prefix(diff_args.nr, diff_args.v, NULL);
setup_revisions(diff_args.nr, diff_args.v, &rev, &opt);
- rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;
+ rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = submodule_summary_callback;
rev.diffopt.format_callback_data = &list;
diff --git a/combine-diff.c b/combine-diff.c
index 1e3cd7fb17..b054177b20 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1394,7 +1394,7 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
int output_format = opt->output_format;
const char *orderfile = opt->orderfile;
- opt->output_format = DIFF_FORMAT_NO_OUTPUT;
+ opt->output_format = 0;
/* tell diff_tree to emit paths in sorted (=tree) order */
opt->orderfile = NULL;
@@ -1408,17 +1408,15 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
if (i == 0 && stat_opt)
opt->output_format = stat_opt;
else
- opt->output_format = DIFF_FORMAT_NO_OUTPUT;
+ opt->output_format = 0;
diff_tree_oid(&parents->oid[i], oid, "", opt);
diffcore_std(opt);
paths = intersect_paths(paths, i, num_parent,
combined_all_paths);
/* if showing diff, show it in requested order */
- if (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&
- orderfile) {
+ if (opt->output_format && orderfile)
diffcore_order(orderfile);
- }
diff_flush(opt);
}
@@ -1521,7 +1519,7 @@ void diff_tree_combined(const struct object_id *oid,
show_log(rev);
if (rev->verbose_header && opt->output_format &&
- opt->output_format != DIFF_FORMAT_NO_OUTPUT &&
+ opt->output_format &&
!commit_format_is_empty(rev->commit_format))
printf("%s%c", diff_line_prefix(opt),
opt->line_termination);
diff --git a/diff.c b/diff.c
index d171f155b1..1f87127a93 100644
--- a/diff.c
+++ b/diff.c
@@ -4801,8 +4801,7 @@ void diff_setup_done(struct diff_options *options)
if (options->output_format & (DIFF_FORMAT_NAME |
DIFF_FORMAT_NAME_STATUS |
- DIFF_FORMAT_CHECKDIFF |
- DIFF_FORMAT_NO_OUTPUT))
+ DIFF_FORMAT_CHECKDIFF))
options->output_format &= ~(DIFF_FORMAT_RAW |
DIFF_FORMAT_NUMSTAT |
DIFF_FORMAT_DIFFSTAT |
@@ -4846,7 +4845,7 @@ void diff_setup_done(struct diff_options *options)
* exit code in such a case either.
*/
if (options->flags.quick) {
- options->output_format = DIFF_FORMAT_NO_OUTPUT;
+ options->output_format = 0;
options->flags.exit_with_status = 1;
}
@@ -4987,7 +4986,7 @@ static int diff_opt_diff_filter(const struct option *option,
static void enable_patch_output(int *fmt)
{
- *fmt &= ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT);
+ *fmt &= ~DIFF_FORMAT_DEFAULT;
*fmt |= DIFF_FORMAT_PATCH;
}
@@ -5492,16 +5491,16 @@ struct option *add_diff_options(const struct option *opts,
OPT_GROUP(N_("Diff output format options")),
OPT_BITOP('p', "patch", &options->output_format,
N_("generate patch"),
- DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT),
OPT_SET_INT_F('s', "silent", &options->output_format,
N_("suppress diff output"),
- DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),
+ 0, PARSE_OPT_NONEG),
OPT_BITOP(0, "no-patch", &options->output_format,
N_("negate --patch"),
- DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),
+ 0, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),
OPT_BITOP('u', NULL, &options->output_format,
N_("generate patch"),
- DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT),
OPT_CALLBACK_F('U', "unified", options, N_("<n>"),
N_("generate diffs with <n> lines context"),
PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified),
@@ -5513,11 +5512,11 @@ struct option *add_diff_options(const struct option *opts,
OPT_BITOP(0, "patch-with-raw", &options->output_format,
N_("synonym for '-p --raw'"),
DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW,
- DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_DEFAULT),
OPT_BITOP(0, "patch-with-stat", &options->output_format,
N_("synonym for '-p --stat'"),
DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT,
- DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),
+ DIFF_FORMAT_DEFAULT),
OPT_BITOP(0, "numstat", &options->output_format,
N_("machine friendly --stat"),
DIFF_FORMAT_NUMSTAT, DIFF_FORMAT_DEFAULT),
@@ -6648,7 +6647,7 @@ void diff_flush(struct diff_options *options)
separator++;
}
- if (output_format == DIFF_FORMAT_NO_OUTPUT &&
+ if (!output_format &&
options->flags.exit_with_status &&
options->flags.diff_from_contents) {
/*
diff --git a/diff.h b/diff.h
index 44da1a4ca7..48e8ff962e 100644
--- a/diff.h
+++ b/diff.h
@@ -94,7 +94,6 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);
-#define DIFF_FORMAT_NO_OUTPUT 0x0000
#define DIFF_FORMAT_RAW 0x0001
#define DIFF_FORMAT_DIFFSTAT 0x0002
#define DIFF_FORMAT_NUMSTAT 0x0004
diff --git a/log-tree.c b/log-tree.c
index f4b22a60cc..b073e1dea4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -874,7 +874,7 @@ int log_tree_diff_flush(struct rev_info *opt)
if (diff_queue_is_empty(&opt->diffopt)) {
int saved_fmt = opt->diffopt.output_format;
- opt->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;
+ opt->diffopt.output_format = 0;
diff_flush(&opt->diffopt);
opt->diffopt.output_format = saved_fmt;
return 0;
@@ -882,7 +882,7 @@ int log_tree_diff_flush(struct rev_info *opt)
if (opt->loginfo && !opt->no_commit_id) {
show_log(opt);
- if ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&
+ if (opt->diffopt.output_format &&
opt->verbose_header &&
opt->commit_format != CMIT_FMT_ONELINE &&
!commit_format_is_empty(opt->commit_format)) {
diff --git a/merge-ort.c b/merge-ort.c
index a50b095c47..392549a24b 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -3248,7 +3248,7 @@ static int detect_regular_renames(struct merge_options *opt,
diff_opts.rename_limit = 7000;
diff_opts.rename_score = opt->rename_score;
diff_opts.show_rename_progress = opt->show_rename_progress;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_setup_done(&diff_opts);
diff_queued_diff = renames->pairs[side_index];
@@ -3269,7 +3269,7 @@ static int detect_regular_renames(struct merge_options *opt,
renames->pairs[side_index] = diff_queued_diff;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_queued_diff.nr = 0;
diff_queued_diff.queue = NULL;
diff_flush(&diff_opts);
diff --git a/merge-recursive.c b/merge-recursive.c
index 8e87b6386d..d5678424f3 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1904,7 +1904,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,
opts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 7000;
opts.rename_score = opt->rename_score;
opts.show_rename_progress = opt->show_rename_progress;
- opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ opts.output_format = 0;
diff_setup_done(&opts);
diff_tree_oid(&o_tree->object.oid, &tree->object.oid, "", &opts);
diffcore_std(&opts);
@@ -1914,7 +1914,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,
ret = xmalloc(sizeof(*ret));
*ret = diff_queued_diff;
- opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ opts.output_format = 0;
diff_queued_diff.nr = 0;
diff_queued_diff.queue = NULL;
diff_flush(&opts);
diff --git a/notes-merge.c b/notes-merge.c
index 233e49e319..9a8bac2579 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -139,7 +139,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
repo_diff_setup(o->repo, &opt);
opt.flags.recursive = 1;
- opt.output_format = DIFF_FORMAT_NO_OUTPUT;
+ opt.output_format = 0;
diff_setup_done(&opt);
diff_tree_oid(base, remote, "", &opt);
diffcore_std(&opt);
@@ -201,7 +201,7 @@ static void diff_tree_local(struct notes_merge_options *o,
repo_diff_setup(o->repo, &opt);
opt.flags.recursive = 1;
- opt.output_format = DIFF_FORMAT_NO_OUTPUT;
+ opt.output_format = 0;
diff_setup_done(&opt);
diff_tree_oid(base, local, "", &opt);
diffcore_std(&opt);
diff --git a/revision.c b/revision.c
index 07d653c197..52c2f415c7 100644
--- a/revision.c
+++ b/revision.c
@@ -2966,7 +2966,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
/* Did the user ask for any diff output? Run the diff! */
- if (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT))
+ if (revs->diffopt.output_format & ~DIFF_FORMAT_DEFAULT)
revs->diff = 1;
/* Pickaxe, diff-filter and rename following need diffs */
diff --git a/tree-diff.c b/tree-diff.c
index 20bb15f38d..757a271348 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -627,7 +627,7 @@ static void try_to_follow_renames(const struct object_id *old_oid,
repo_diff_setup(opt->repo, &diff_opts);
diff_opts.flags.recursive = 1;
diff_opts.flags.find_copies_harder = 1;
- diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+ diff_opts.output_format = 0;
diff_opts.single_follow = opt->pathspec.items[0].match;
diff_opts.break_opt = opt->break_opt;
diff_opts.rename_score = opt->rename_score;
--
2.40.0+fc1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1 0/7] diff: fix -s and --no-patch
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
` (6 preceding siblings ...)
2023-05-12 8:03 ` [PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT Felipe Contreras
@ 2023-05-12 8:20 ` Felipe Contreras
2023-05-12 9:32 ` Sergey Organov
7 siblings, 1 reply; 10+ messages in thread
From: Felipe Contreras @ 2023-05-12 8:20 UTC (permalink / raw)
To: Sergey Organov; +Cc: git
Felipe Contreras wrote:
> This fixes an issue Sergey Organov reported.
Sergey, as you can see this series fixes the issue you reported.
First, I think these should remain working the same, simply for convenience:
* git diff # default output
* git diff --patch # patch output
* git diff --raw # raw output
* git diff --stat # stat output
I don't think there's a way I can be convinced otherwise.
But there's many changes:
1. git diff -s --raw # before: nil, after: raw
2. git diff --no-patch --raw # before: nil, after: raw
3. git diff --patch --no-patch --raw # before: nil, after: raw
4. git diff --raw --patch --no-patch # before: nil, after: raw
I don't think there's any way you can say my 174 changes make the code work
"exactly the same".
And this is better than Junio's solution, because #4 outputs a raw format,
while in Junio's solution it doesn't output anything.
Even if you don't agree with everything, this solution is better than the
status quo, and it's better than Junio's solution as it fixes --no-patch
immediately.
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1 0/7] diff: fix -s and --no-patch
2023-05-12 8:20 ` [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
@ 2023-05-12 9:32 ` Sergey Organov
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Organov @ 2023-05-12 9:32 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
Felipe Contreras <felipe.contreras@gmail.com> writes:
> Felipe Contreras wrote:
>> This fixes an issue Sergey Organov reported.
>
> Sergey, as you can see this series fixes the issue you reported.
>
> First, I think these should remain working the same, simply for convenience:
>
> * git diff # default output
> * git diff --patch # patch output
> * git diff --raw # raw output
> * git diff --stat # stat output
>
> I don't think there's a way I can be convinced otherwise.
Fine with me.
>
> But there's many changes:
>
> 1. git diff -s --raw # before: nil, after: raw
> 2. git diff --no-patch --raw # before: nil, after: raw
> 3. git diff --patch --no-patch --raw # before: nil, after: raw
> 4. git diff --raw --patch --no-patch # before: nil, after: raw
Fine as well.
>
> I don't think there's any way you can say my 174 changes make the code work
> "exactly the same".
I said that in the context where we discussed entirely separate issue
"handling of defaults by Git commands". Irrelevant to these series as
they don't touch this aspect as visible from outside, even though you
do change the implementation for better.
>
> And this is better than Junio's solution, because #4 outputs a raw format,
> while in Junio's solution it doesn't output anything.
Yes, and that's where I agreed from the very beginning.
> Even if you don't agree with everything, this solution is better than the
> status quo, and it's better than Junio's solution as it fixes --no-patch
> immediately.
Yep, it fixes "--no-patch" semantics indeed, and as I already said, I do
vote in favor of this change, for what it's worth.
Thanks,
-- Sergey Organov
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-05-12 9:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-12 8:03 [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 1/7] line-log: set patch format explicitly by default Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0 Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 4/7] test: add various tests for diff formats with -s Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 5/7] diff: split --no-patch from -s Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 6/7] diff: add --silent as alias of -s Felipe Contreras
2023-05-12 8:03 ` [PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT Felipe Contreras
2023-05-12 8:20 ` [PATCH v1 0/7] diff: fix -s and --no-patch Felipe Contreras
2023-05-12 9:32 ` Sergey Organov
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).