{"thread":{"id":"59734","subject":"[PATCH v1 1/7] line-log: set patch format explicitly by default","startedAt":"2023-05-12T08:03:47Z","lastAt":"2023-05-12T09:32:39Z","messageCount":10,"participants":["Felipe Contreras","Sergey Organov"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"477163","messageId":"20230512080339.2186324-2-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 1/7] line-log: set patch format explicitly by default","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:33Z","receivedAt":"2023-05-12T08:03:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Will help further changes.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/log.c | 5 +++++\n line-log.c    | 2 +-\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 676de107d6..712bfbf5c2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -890,6 +890,11 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \topt.revarg_opt = REVARG_COMMITTISH;\n \topt.tweak = log_setup_revisions_tweak;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n+\n+\tif (!rev.diffopt.output_format)\n+\t\tif (rev.line_level_traverse)\n+\t\t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n+\n \treturn cmd_log_deinit(cmd_log_walk(&rev), &rev);\n }\n \ndiff --git a/line-log.c b/line-log.c\nindex 6a7ac312a4..7466366860 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -1141,7 +1141,7 @@ int line_log_print(struct rev_info *rev, struct commit *commit)\n {\n \n \tshow_log(rev);\n-\tif (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT)) {\n+\tif (rev->diffopt.output_format & DIFF_FORMAT_PATCH) {\n \t\tstruct line_log_data *range = lookup_line_range(rev, commit);\n \t\tdump_diff_hacky(rev, range);\n \t}\n-- \n2.40.0+fc1\n\n"},{"id":"477164","messageId":"20230512080339.2186324-1-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":null,"subject":"[PATCH v1 0/7] diff: fix -s and --no-patch","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:32Z","receivedAt":"2023-05-12T08:03:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The diff code assumes 0 means the default, and --no-patch means\nNO_OUTPUT.\n\nThe problem with this approach is that it doesn't allow distinguishing\n`git diff --no-patch`, `git diff --patch --no-patch`, and `git diff`.\n\nBy introducing a DIFF_FORMAT_DEFAULT (which is not 0) it's now possible\nto properly distinguish these arguments, and get rid of\nDIFF_FORMAT_NO_OUTPUT which should have never been considered a format,\nbut the absence of a format.\n\nThis fixes an issue Sergey Organov reported.\n\nUp to patch #2 (diff: introduce DIFF_FORMAT_DEFAULT) there are no\nfunctional changes, patch #3 (diff: make DIFF_FORMAT_NO_OUTPUT 0)\nachieves the same as a series from Junio Hamano [1] except more\nproperly. Patch #4 adds a simplified version of Junio's test cases.\n\nThen in patch #5 --no-patch is split from -s, making it work as\nintended: negates --patch, but not other formats.\n\nThe rest are some niceties.\n\nNow all these work correctly:\n\n 1. git diff --raw\n 2. git diff -s --raw\n 3. git diff --no-patch\n 4. git diff --no-patch --raw\n 5. git diff --patch --no-patch --raw\n 6. git diff --raw --patch --no-patch\n\n[1] https://lore.kernel.org/git/20230505165952.335256-1-gitster@pobox.com/\n\nFelipe Contreras (7):\n  line-log: set patch format explicitly by default\n  diff: introduce DIFF_FORMAT_DEFAULT\n  diff: make DIFF_FORMAT_NO_OUTPUT 0\n  test: add various tests for diff formats with -s\n  diff: split --no-patch from -s\n  diff: add --silent as alias of -s\n  diff: remove DIFF_FORMAT_NO_OUTPUT\n\n Documentation/diff-options.txt |  6 ++--\n blame.c                        |  6 ++--\n builtin/diff-files.c           |  2 +-\n builtin/diff-index.c           |  2 +-\n builtin/diff-tree.c            |  2 +-\n builtin/diff.c                 |  2 +-\n builtin/log.c                  | 16 +++++++---\n builtin/stash.c                |  4 +--\n builtin/submodule--helper.c    |  2 +-\n combine-diff.c                 | 10 +++---\n diff-merges.c                  |  2 +-\n diff-no-index.c                |  2 +-\n diff.c                         | 56 ++++++++++++++++++----------------\n diff.h                         |  6 +---\n line-log.c                     |  2 +-\n log-tree.c                     |  4 +--\n merge-ort.c                    |  4 +--\n merge-recursive.c              |  4 +--\n notes-merge.c                  |  4 +--\n range-diff.c                   |  4 +--\n revision.c                     |  6 ++--\n t/t4000-diff-format.sh         | 26 ++++++++++++++--\n tree-diff.c                    |  2 +-\n 23 files changed, 99 insertions(+), 75 deletions(-)\n\n-- \n2.40.0+fc1\n\n"},{"id":"477165","messageId":"20230512080339.2186324-3-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 2/7] diff: introduce DIFF_FORMAT_DEFAULT","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:34Z","receivedAt":"2023-05-12T08:03:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"As the name suggests this is the default format, which means no format\nwas specified.\n\nThis is not the same as DIFF_FORMAT_PATCH, as some commands like `git\ndiff-files` use a different default.\n\nThis makes it possible to distinguish `git diff` (DEFAULT)\nfrom  `git diff --no-patch` (0).\n\nWill help further changes.\n\nThere should be no functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/diff-files.c |  2 +-\n builtin/diff-index.c |  2 +-\n builtin/diff-tree.c  |  2 +-\n builtin/diff.c       |  2 +-\n builtin/log.c        | 13 ++++++++-----\n builtin/stash.c      |  2 +-\n diff-merges.c        |  2 +-\n diff-no-index.c      |  2 +-\n diff.c               | 39 ++++++++++++++++++++-------------------\n diff.h               |  1 +\n range-diff.c         |  2 +-\n revision.c           |  4 ++--\n 12 files changed, 39 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex dc991f753b..b831b89236 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -52,7 +52,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \t\t\tusage(diff_files_usage);\n \t\targv++; argc--;\n \t}\n-\tif (!rev.diffopt.output_format)\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \trev.diffopt.rotate_to_strict = 1;\n \ndiff --git a/builtin/diff-index.c b/builtin/diff-index.c\nindex b9a19bb7d3..863c51c9b5 100644\n--- a/builtin/diff-index.c\n+++ b/builtin/diff-index.c\n@@ -48,7 +48,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\tusage(diff_cache_usage);\n \t}\n-\tif (!rev.diffopt.output_format)\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \n \trev.diffopt.rotate_to_strict = 1;\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 0b02c62b85..7e9164187c 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -100,7 +100,7 @@ COMMON_DIFF_OPTIONS_HELP;\n \n static void diff_tree_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)\n {\n-\tif (!rev->diffopt.output_format) {\n+\tif (rev->diffopt.output_format == DIFF_FORMAT_DEFAULT) {\n \t\tif (rev->dense_combined_merges)\n \t\t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\telse\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 7b64659fe7..2decf5e531 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -505,7 +505,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tif (nongit)\n \t\tdie(_(\"Not a git repository\"));\n \targc = setup_revisions(argc, argv, &rev, NULL);\n-\tif (!rev.diffopt.output_format) {\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\tdiff_setup_done(&rev.diffopt);\n \t}\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 712bfbf5c2..d2a81f36c2 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -277,7 +277,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n \tif (quiet)\n-\t\trev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;\n+\t\trev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n \targc = setup_revisions(argc, argv, rev, opt);\n \n \t/* Any arguments at this point are not recognized */\n@@ -633,7 +633,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \topt.def = \"HEAD\";\n \topt.revarg_opt = REVARG_COMMITTISH;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n-\tif (!rev.diffopt.output_format)\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \treturn cmd_log_deinit(cmd_log_walk(&rev), &rev);\n }\n@@ -725,7 +725,7 @@ static void show_setup_revisions_tweak(struct rev_info *rev,\n \t\tdiff_merges_default_to_first_parent(rev);\n \telse\n \t\tdiff_merges_default_to_dense_combined(rev);\n-\tif (!rev->diffopt.output_format)\n+\tif (rev->diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\trev->diffopt.output_format = DIFF_FORMAT_PATCH;\n }\n \n@@ -891,9 +891,12 @@ int cmd_log(int argc, const char **argv, const char *prefix)\n \topt.tweak = log_setup_revisions_tweak;\n \tcmd_log_init(argc, argv, prefix, &rev, &opt);\n \n-\tif (!rev.diffopt.output_format)\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {\n \t\tif (rev.line_level_traverse)\n \t\t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n+\t\telse\n+\t\t\trev.diffopt.output_format = 0;\n+\t}\n \n \treturn cmd_log_deinit(cmd_log_walk(&rev), &rev);\n }\n@@ -2126,7 +2129,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--remerge-diff does not make sense\"));\n \n \tif (!use_patch_format &&\n-\t\t(!rev.diffopt.output_format ||\n+\t\t(rev.diffopt.output_format == DIFF_FORMAT_DEFAULT ||\n \t\t rev.diffopt.output_format == DIFF_FORMAT_PATCH))\n \t\trev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY;\n \tif (!rev.diffopt.stat_width)\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex a7e17ffe38..398e3c9f61 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -944,7 +944,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)\n \targc = setup_revisions(revision_args.nr, revision_args.v, &rev, NULL);\n \tif (argc > 1)\n \t\tgoto usage;\n-\tif (!rev.diffopt.output_format) {\n+\tif (rev.diffopt.output_format == DIFF_FORMAT_DEFAULT) {\n \t\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n \t\tdiff_setup_done(&rev.diffopt);\n \t}\ndiff --git a/diff-merges.c b/diff-merges.c\nindex ec97616db1..9960d7cc36 100644\n--- a/diff-merges.c\n+++ b/diff-merges.c\n@@ -183,7 +183,7 @@ void diff_merges_setup_revs(struct rev_info *revs)\n \tif (revs->merges_imply_patch)\n \t\trevs->diff = 1;\n \tif (revs->merges_imply_patch || revs->merges_need_diff) {\n-\t\tif (!revs->diffopt.output_format)\n+\t\tif (revs->diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n \t}\n }\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 4296940f90..45596cb1be 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -282,7 +282,7 @@ int diff_no_index(struct rev_info *revs,\n \tfixup_paths(paths, &replacement);\n \n \trevs->diffopt.skip_stat_unmatch = 1;\n-\tif (!revs->diffopt.output_format)\n+\tif (revs->diffopt.output_format == DIFF_FORMAT_DEFAULT)\n \t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n \n \trevs->diffopt.flags.no_index = 1;\ndiff --git a/diff.c b/diff.c\nindex 71513d92e8..387944f289 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4669,6 +4669,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)\n \toptions->file = stdout;\n \toptions->repo = r;\n \n+\toptions->output_format = DIFF_FORMAT_DEFAULT;\n \toptions->output_indicators[OUTPUT_INDICATOR_NEW] = '+';\n \toptions->output_indicators[OUTPUT_INDICATOR_OLD] = '-';\n \toptions->output_indicators[OUTPUT_INDICATOR_CONTEXT] = ' ';\n@@ -4987,7 +4988,7 @@ static int diff_opt_diff_filter(const struct option *option,\n \n static void enable_patch_output(int *fmt)\n {\n-\t*fmt &= ~DIFF_FORMAT_NO_OUTPUT;\n+\t*fmt &= ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT);\n \t*fmt |= DIFF_FORMAT_PATCH;\n }\n \n@@ -5492,13 +5493,13 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_GROUP(N_(\"Diff output format options\")),\n \t\tOPT_BITOP('p', \"patch\", &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n-\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),\n-\t\tOPT_BIT_F('s', \"no-patch\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\tOPT_BITOP('s', \"no-patch\", &options->output_format,\n \t\t\t  N_(\"suppress diff output\"),\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),\n+\t\t\t  DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH),\n \t\tOPT_BITOP('u', NULL, &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n-\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n \t\tOPT_CALLBACK_F('U', \"unified\", options, N_(\"<n>\"),\n \t\t\t       N_(\"generate diffs with <n> lines context\"),\n \t\t\t       PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified),\n@@ -5510,17 +5511,17 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_BITOP(0, \"patch-with-raw\", &options->output_format,\n \t\t\t  N_(\"synonym for '-p --raw'\"),\n \t\t\t  DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW,\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n \t\tOPT_BITOP(0, \"patch-with-stat\", &options->output_format,\n \t\t\t  N_(\"synonym for '-p --stat'\"),\n \t\t\t  DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT,\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT),\n-\t\tOPT_BIT_F(0, \"numstat\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\tOPT_BITOP(0, \"numstat\", &options->output_format,\n \t\t\t  N_(\"machine friendly --stat\"),\n-\t\t\t  DIFF_FORMAT_NUMSTAT, PARSE_OPT_NONEG),\n-\t\tOPT_BIT_F(0, \"shortstat\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_NUMSTAT, DIFF_FORMAT_DEFAULT),\n+\t\tOPT_BITOP(0, \"shortstat\", &options->output_format,\n \t\t\t  N_(\"output only the last line of --stat\"),\n-\t\t\t  DIFF_FORMAT_SHORTSTAT, PARSE_OPT_NONEG),\n+\t\t\t  DIFF_FORMAT_SHORTSTAT, DIFF_FORMAT_DEFAULT),\n \t\tOPT_CALLBACK_F('X', \"dirstat\", options, N_(\"<param1,param2>...\"),\n \t\t\t       N_(\"output the distribution of relative amount of changes for each sub-directory\"),\n \t\t\t       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,\n@@ -5533,18 +5534,18 @@ struct option *add_diff_options(const struct option *opts,\n \t\t\t       N_(\"synonym for --dirstat=files,param1,param2...\"),\n \t\t\t       PARSE_OPT_NONEG | PARSE_OPT_OPTARG,\n \t\t\t       diff_opt_dirstat),\n-\t\tOPT_BIT_F(0, \"check\", &options->output_format,\n+\t\tOPT_BITOP(0, \"check\", &options->output_format,\n \t\t\t  N_(\"warn if changes introduce conflict markers or whitespace errors\"),\n-\t\t\t  DIFF_FORMAT_CHECKDIFF, PARSE_OPT_NONEG),\n-\t\tOPT_BIT_F(0, \"summary\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_CHECKDIFF, DIFF_FORMAT_DEFAULT),\n+\t\tOPT_BITOP(0, \"summary\", &options->output_format,\n \t\t\t  N_(\"condensed summary such as creations, renames and mode changes\"),\n-\t\t\t  DIFF_FORMAT_SUMMARY, PARSE_OPT_NONEG),\n-\t\tOPT_BIT_F(0, \"name-only\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_SUMMARY, DIFF_FORMAT_DEFAULT),\n+\t\tOPT_BITOP(0, \"name-only\", &options->output_format,\n \t\t\t  N_(\"show only names of changed files\"),\n-\t\t\t  DIFF_FORMAT_NAME, PARSE_OPT_NONEG),\n-\t\tOPT_BIT_F(0, \"name-status\", &options->output_format,\n+\t\t\t  DIFF_FORMAT_NAME, DIFF_FORMAT_DEFAULT),\n+\t\tOPT_BITOP(0, \"name-status\", &options->output_format,\n \t\t\t  N_(\"show only names and status of changed files\"),\n-\t\t\t  DIFF_FORMAT_NAME_STATUS, PARSE_OPT_NONEG),\n+\t\t\t  DIFF_FORMAT_NAME_STATUS, DIFF_FORMAT_DEFAULT),\n \t\tOPT_CALLBACK_F(0, \"stat\", options, N_(\"<width>[,<name-width>[,<count>]]\"),\n \t\t\t       N_(\"generate diffstat\"),\n \t\t\t       PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_stat),\ndiff --git a/diff.h b/diff.h\nindex 3a7a9e8b88..15a7bf2c9f 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -101,6 +101,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_FORMAT_PATCH\t0x0010\n #define DIFF_FORMAT_SHORTSTAT\t0x0020\n #define DIFF_FORMAT_DIRSTAT\t0x0040\n+#define DIFF_FORMAT_DEFAULT\t0x0080\n \n /* These override all above */\n #define DIFF_FORMAT_NAME\t0x0100\ndiff --git a/range-diff.c b/range-diff.c\nindex 6a704e6f47..6c1ae9dd34 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -492,7 +492,7 @@ static void output(struct string_list *a, struct string_list *b,\n \t\trepo_diff_setup(the_repository, &opts);\n \n \topts.no_free = 1;\n-\tif (!opts.output_format)\n+\tif (opts.output_format == DIFF_FORMAT_DEFAULT)\n \t\topts.output_format = DIFF_FORMAT_PATCH;\n \topts.flags.suppress_diff_headers = 1;\n \topts.flags.dual_color_diffed_diffs =\ndiff --git a/revision.c b/revision.c\nindex b33cc1d106..cf68b533fd 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2966,7 +2966,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \n \t/* Did the user ask for any diff output? Run the diff! */\n-\tif (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT)\n+\tif (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT))\n \t\trevs->diff = 1;\n \n \t/* Pickaxe, diff-filter and rename following need diffs */\n@@ -3030,7 +3030,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"--grep-reflog\", \"--walk-reflogs\");\n \n \tif (revs->line_level_traverse &&\n-\t    (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n+\t    (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n \t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\n \n \tif (revs->expand_tabs_in_log < 0)\n-- \n2.40.0+fc1\n\n"},{"id":"477166","messageId":"20230512080339.2186324-4-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 3/7] diff: make DIFF_FORMAT_NO_OUTPUT 0","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:35Z","receivedAt":"2023-05-12T08:03:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"`-s` is considered a format, but if we change the meaning to absence of\na format it now becomes possible to distinguish `git diff` from\n`git diff --no-patch`, although that isn't done in this commit.\n\nThis also fixes a bug in which specifying an output format did not clear\nthe NO_OUTPUT flag.\n\nFor example this now works correctly:\n\n  git show -s --raw\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n diff.c       | 13 ++++++-------\n diff.h       |  6 +-----\n range-diff.c |  2 +-\n revision.c   |  4 ++--\n 4 files changed, 10 insertions(+), 15 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 387944f289..4f4b1d7e13 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4750,8 +4750,7 @@ void diff_setup_done(struct diff_options *options)\n {\n \tunsigned check_mask = DIFF_FORMAT_NAME |\n \t\t\t      DIFF_FORMAT_NAME_STATUS |\n-\t\t\t      DIFF_FORMAT_CHECKDIFF |\n-\t\t\t      DIFF_FORMAT_NO_OUTPUT;\n+\t\t\t      DIFF_FORMAT_CHECKDIFF;\n \t/*\n \t * This must be signed because we're comparing against a potentially\n \t * negative value.\n@@ -4762,8 +4761,8 @@ void diff_setup_done(struct diff_options *options)\n \t\toptions->set_default(options);\n \n \tif (HAS_MULTI_BITS(options->output_format & check_mask))\n-\t\tdie(_(\"options '%s', '%s', '%s', and '%s' cannot be used together\"),\n-\t\t\t\"--name-only\", \"--name-status\", \"--check\", \"-s\");\n+\t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n+\t\t\t\"--name-only\", \"--name-status\", \"--check\");\n \n \tif (HAS_MULTI_BITS(options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK))\n \t\tdie(_(\"options '%s', '%s', and '%s' cannot be used together\"),\n@@ -5494,9 +5493,9 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_BITOP('p', \"patch\", &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n-\t\tOPT_BITOP('s', \"no-patch\", &options->output_format,\n+\t\tOPT_SET_INT_F('s', \"no-patch\", &options->output_format,\n \t\t\t  N_(\"suppress diff output\"),\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH),\n+\t\t\t  DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),\n \t\tOPT_BITOP('u', NULL, &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n@@ -6646,7 +6645,7 @@ void diff_flush(struct diff_options *options)\n \t\tseparator++;\n \t}\n \n-\tif (output_format & DIFF_FORMAT_NO_OUTPUT &&\n+\tif (output_format == DIFF_FORMAT_NO_OUTPUT &&\n \t    options->flags.exit_with_status &&\n \t    options->flags.diff_from_contents) {\n \t\t/*\ndiff --git a/diff.h b/diff.h\nindex 15a7bf2c9f..44da1a4ca7 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -94,6 +94,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n \n typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n \n+#define DIFF_FORMAT_NO_OUTPUT\t0x0000\n #define DIFF_FORMAT_RAW\t\t0x0001\n #define DIFF_FORMAT_DIFFSTAT\t0x0002\n #define DIFF_FORMAT_NUMSTAT\t0x0004\n@@ -108,11 +109,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_FORMAT_NAME_STATUS\t0x0200\n #define DIFF_FORMAT_CHECKDIFF\t0x0400\n \n-/* Same as output_format = 0 but we know that -s flag was given\n- * and we should not give default value to output_format.\n- */\n-#define DIFF_FORMAT_NO_OUTPUT\t0x0800\n-\n #define DIFF_FORMAT_CALLBACK\t0x1000\n \n #define DIFF_FLAGS_INIT { 0 }\ndiff --git a/range-diff.c b/range-diff.c\nindex 6c1ae9dd34..00ff5dc160 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -542,7 +542,7 @@ static void output(struct string_list *a, struct string_list *b,\n \t\t\ta_util = a->items[b_util->matching].util;\n \t\t\toutput_pair_header(&opts, patch_no_width,\n \t\t\t\t\t   &buf, &dashes, a_util, b_util);\n-\t\t\tif (!(opts.output_format & DIFF_FORMAT_NO_OUTPUT))\n+\t\t\tif (opts.output_format)\n \t\t\t\tpatch_diff(a->items[b_util->matching].string,\n \t\t\t\t\t   b->items[j].string, &opts);\n \t\t\ta_util->shown = 1;\ndiff --git a/revision.c b/revision.c\nindex cf68b533fd..07d653c197 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -3030,8 +3030,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"--grep-reflog\", \"--walk-reflogs\");\n \n \tif (revs->line_level_traverse &&\n-\t    (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))\n-\t\tdie(_(\"-L does not yet support diff formats besides -p and -s\"));\n+\t    (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_PATCH)))\n+\t\tdie(_(\"-L does not yet support diff formats besides -p\"));\n \n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n-- \n2.40.0+fc1\n\n"},{"id":"477167","messageId":"20230512080339.2186324-5-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 4/7] test: add various tests for diff formats with -s","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:36Z","receivedAt":"2023-05-12T08:03:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There used to be a bug when -s was used with different formats, for\nexample `-s --raw`.\n\nOriginally-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/t4000-diff-format.sh | 19 +++++++++++++++++++\n 1 file changed, 19 insertions(+)\n\ndiff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh\nindex bfcaae390f..7829cc810d 100755\n--- a/t/t4000-diff-format.sh\n+++ b/t/t4000-diff-format.sh\n@@ -91,4 +91,23 @@ test_expect_success 'git diff-files --patch --no-patch does not show the patch'\n \ttest_must_be_empty err\n '\n \n+\n+echo 'reset' >path1\n+\n+for format in stat raw numstat shortstat summary dirstat cumulative \\\n+\tdirstat-by-file patch-with-raw patch-with-stat compact-summary\n+do\n+\ttest_expect_success \"-s before --$format' is a no-op\" '\n+\t\tgit diff-files -s \"--$format\" >actual &&\n+\t\tgit diff-files \"--$format\" >expect &&\n+\t\ttest_cmp expect actual\n+\t'\n+\n+\ttest_expect_success \"-s clears --$format\" '\n+\t\tgit diff-files --$format -s --patch >actual &&\n+\t\tgit diff-files --patch >expect &&\n+\t\ttest_cmp expect actual\n+\t'\n+done\n+\n test_done\n-- \n2.40.0+fc1\n\n"},{"id":"477168","messageId":"20230512080339.2186324-6-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 5/7] diff: split --no-patch from -s","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:37Z","receivedAt":"2023-05-12T08:03:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"It should silence only the --patch output, not all output.\n\nWhen --no-patch was introduced in d09cd15d19 (diff: allow --no-patch as\nsynonym for -s, 2013-07-16), the idea was to have a more accessible\nshortcut to silence the output of `git show`.\n\nHowever, the interaction with other options was not considered, for\nexample `--raw --no-patch`.\n\nThe original intention remains, as `git show --no-patch` still produces\nthe same: no output.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/diff-options.txt | 5 ++---\n diff.c                         | 5 ++++-\n t/t4000-diff-format.sh         | 7 ++++---\n 3 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 08ab86189a..ba04b8292a 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -21,7 +21,7 @@ endif::git-format-patch[]\n ifndef::git-format-patch[]\n -p::\n -u::\n---patch::\n+--[no-]patch::\n \tGenerate patch (see section titled\n ifdef::git-log[]\n <<generate_patch_text_with_p, \"Generating patch text with -p\">>).\n@@ -34,9 +34,8 @@ ifdef::git-diff[]\n endif::git-diff[]\n \n -s::\n---no-patch::\n \tSuppress diff output. Useful for commands like `git show` that\n-\tshow the patch by default, or to cancel the effect of `--patch`.\n+\tshow output by default.\n endif::git-format-patch[]\n \n ifdef::git-log[]\ndiff --git a/diff.c b/diff.c\nindex 4f4b1d7e13..45c860496f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5493,9 +5493,12 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_BITOP('p', \"patch\", &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n-\t\tOPT_SET_INT_F('s', \"no-patch\", &options->output_format,\n+\t\tOPT_SET_INT_F('s', NULL, &options->output_format,\n \t\t\t  N_(\"suppress diff output\"),\n \t\t\t  DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),\n+\t\tOPT_BITOP(0, \"no-patch\", &options->output_format,\n+\t\t\t  N_(\"negate --patch\"),\n+\t\t\t  DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),\n \t\tOPT_BITOP('u', NULL, &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\ndiff --git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh\nindex 7829cc810d..d7b9a2dab8 100755\n--- a/t/t4000-diff-format.sh\n+++ b/t/t4000-diff-format.sh\n@@ -67,9 +67,10 @@ test_expect_success 'git diff-files -s after editing work tree' '\n \ttest_must_be_empty err\n '\n \n-test_expect_success 'git diff-files --no-patch as synonym for -s' '\n-\tgit diff-files --no-patch >actual 2>err &&\n-\ttest_must_be_empty actual &&\n+test_expect_success 'git diff-files --no-patch negates --patch' '\n+\tgit diff-files >expected_raw &&\n+\tgit diff-files --raw --patch --no-patch >actual 2>err &&\n+\ttest_cmp expected_raw actual &&\n \ttest_must_be_empty err\n '\n \n-- \n2.40.0+fc1\n\n"},{"id":"477169","messageId":"20230512080339.2186324-7-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 6/7] diff: add --silent as alias of -s","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:38Z","receivedAt":"2023-05-12T08:04:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ever since 2e5e98a40 ([PATCH] Silent flag for show-diff, 2005-04-13) -s\nmeant silent.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/diff-options.txt | 1 +\n diff.c                         | 2 +-\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex ba04b8292a..a402b6f9b6 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -34,6 +34,7 @@ ifdef::git-diff[]\n endif::git-diff[]\n \n -s::\n+--silent::\n \tSuppress diff output. Useful for commands like `git show` that\n \tshow output by default.\n endif::git-format-patch[]\ndiff --git a/diff.c b/diff.c\nindex 45c860496f..d171f155b1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5493,7 +5493,7 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_BITOP('p', \"patch\", &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n-\t\tOPT_SET_INT_F('s', NULL, &options->output_format,\n+\t\tOPT_SET_INT_F('s', \"silent\", &options->output_format,\n \t\t\t  N_(\"suppress diff output\"),\n \t\t\t  DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),\n \t\tOPT_BITOP(0, \"no-patch\", &options->output_format,\n-- \n2.40.0+fc1\n\n"},{"id":"477170","messageId":"20230512080339.2186324-8-felipe.contreras@gmail.com","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"[PATCH v1 7/7] diff: remove DIFF_FORMAT_NO_OUTPUT","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:03:39Z","receivedAt":"2023-05-12T08:04:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Instead use an empty output_format (0) as NO_OUTPUT.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n blame.c                     |  6 +++---\n builtin/log.c               |  2 +-\n builtin/stash.c             |  2 +-\n builtin/submodule--helper.c |  2 +-\n combine-diff.c              | 10 ++++------\n diff.c                      | 21 ++++++++++-----------\n diff.h                      |  1 -\n log-tree.c                  |  4 ++--\n merge-ort.c                 |  4 ++--\n merge-recursive.c           |  4 ++--\n notes-merge.c               |  4 ++--\n revision.c                  |  2 +-\n tree-diff.c                 |  2 +-\n 13 files changed, 30 insertions(+), 34 deletions(-)\n\ndiff --git a/blame.c b/blame.c\nindex b830654062..d382af6798 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -1337,7 +1337,7 @@ static struct blame_origin *find_origin(struct repository *r,\n \trepo_diff_setup(r, &diff_opts);\n \tdiff_opts.flags.recursive = 1;\n \tdiff_opts.detect_rename = 0;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tpaths[0] = origin->path;\n \tpaths[1] = NULL;\n \n@@ -1420,7 +1420,7 @@ static struct blame_origin *find_rename(struct repository *r,\n \trepo_diff_setup(r, &diff_opts);\n \tdiff_opts.flags.recursive = 1;\n \tdiff_opts.detect_rename = DIFF_DETECT_RENAME;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tdiff_opts.single_follow = origin->path;\n \tdiff_setup_done(&diff_opts);\n \n@@ -2242,7 +2242,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,\n \n \trepo_diff_setup(sb->repo, &diff_opts);\n \tdiff_opts.flags.recursive = 1;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \n \tdiff_setup_done(&diff_opts);\n \ndiff --git a/builtin/log.c b/builtin/log.c\nindex d2a81f36c2..4fbdbd349d 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -277,7 +277,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n \tif (quiet)\n-\t\trev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\trev->diffopt.output_format = 0;\n \targc = setup_revisions(argc, argv, rev, opt);\n \n \t/* Any arguments at this point are not recognized */\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 398e3c9f61..d7a4ade8d4 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -437,7 +437,7 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)\n \trepo_diff_setup(the_repository, &diff_opts);\n \tdiff_opts.flags.recursive = 1;\n \tdiff_opts.detect_rename = 0;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tdiff_setup_done(&diff_opts);\n \n \tdo_diff_cache(orig_tree, &diff_opts);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6bf8d666ce..885cd57e90 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1120,7 +1120,7 @@ static int compute_summary_module_list(struct object_id *head_oid,\n \trev.abbrev = 0;\n \tprecompose_argv_prefix(diff_args.nr, diff_args.v, NULL);\n \tsetup_revisions(diff_args.nr, diff_args.v, &rev, &opt);\n-\trev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;\n+\trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = submodule_summary_callback;\n \trev.diffopt.format_callback_data = &list;\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex 1e3cd7fb17..b054177b20 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -1394,7 +1394,7 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \tint output_format = opt->output_format;\n \tconst char *orderfile = opt->orderfile;\n \n-\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n+\topt->output_format = 0;\n \t/* tell diff_tree to emit paths in sorted (=tree) order */\n \topt->orderfile = NULL;\n \n@@ -1408,17 +1408,15 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,\n \t\tif (i == 0 && stat_opt)\n \t\t\topt->output_format = stat_opt;\n \t\telse\n-\t\t\topt->output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\t\topt->output_format = 0;\n \t\tdiff_tree_oid(&parents->oid[i], oid, \"\", opt);\n \t\tdiffcore_std(opt);\n \t\tpaths = intersect_paths(paths, i, num_parent,\n \t\t\t\t\tcombined_all_paths);\n \n \t\t/* if showing diff, show it in requested order */\n-\t\tif (opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n-\t\t    orderfile) {\n+\t\tif (opt->output_format && orderfile)\n \t\t\tdiffcore_order(orderfile);\n-\t\t}\n \n \t\tdiff_flush(opt);\n \t}\n@@ -1521,7 +1519,7 @@ void diff_tree_combined(const struct object_id *oid,\n \t\tshow_log(rev);\n \n \t\tif (rev->verbose_header && opt->output_format &&\n-\t\t    opt->output_format != DIFF_FORMAT_NO_OUTPUT &&\n+\t\t    opt->output_format &&\n \t\t    !commit_format_is_empty(rev->commit_format))\n \t\t\tprintf(\"%s%c\", diff_line_prefix(opt),\n \t\t\t       opt->line_termination);\ndiff --git a/diff.c b/diff.c\nindex d171f155b1..1f87127a93 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4801,8 +4801,7 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (options->output_format & (DIFF_FORMAT_NAME |\n \t\t\t\t      DIFF_FORMAT_NAME_STATUS |\n-\t\t\t\t      DIFF_FORMAT_CHECKDIFF |\n-\t\t\t\t      DIFF_FORMAT_NO_OUTPUT))\n+\t\t\t\t      DIFF_FORMAT_CHECKDIFF))\n \t\toptions->output_format &= ~(DIFF_FORMAT_RAW |\n \t\t\t\t\t    DIFF_FORMAT_NUMSTAT |\n \t\t\t\t\t    DIFF_FORMAT_DIFFSTAT |\n@@ -4846,7 +4845,7 @@ void diff_setup_done(struct diff_options *options)\n \t * exit code in such a case either.\n \t */\n \tif (options->flags.quick) {\n-\t\toptions->output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\toptions->output_format = 0;\n \t\toptions->flags.exit_with_status = 1;\n \t}\n \n@@ -4987,7 +4986,7 @@ static int diff_opt_diff_filter(const struct option *option,\n \n static void enable_patch_output(int *fmt)\n {\n-\t*fmt &= ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT);\n+\t*fmt &= ~DIFF_FORMAT_DEFAULT;\n \t*fmt |= DIFF_FORMAT_PATCH;\n }\n \n@@ -5492,16 +5491,16 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_GROUP(N_(\"Diff output format options\")),\n \t\tOPT_BITOP('p', \"patch\", &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n-\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT),\n \t\tOPT_SET_INT_F('s', \"silent\", &options->output_format,\n \t\t\t  N_(\"suppress diff output\"),\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT, PARSE_OPT_NONEG),\n+\t\t\t  0, PARSE_OPT_NONEG),\n \t\tOPT_BITOP(0, \"no-patch\", &options->output_format,\n \t\t\t  N_(\"negate --patch\"),\n-\t\t\t  DIFF_FORMAT_NO_OUTPUT, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),\n+\t\t\t  0, DIFF_FORMAT_PATCH | DIFF_FORMAT_DEFAULT),\n \t\tOPT_BITOP('u', NULL, &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n-\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_DEFAULT),\n \t\tOPT_CALLBACK_F('U', \"unified\", options, N_(\"<n>\"),\n \t\t\t       N_(\"generate diffs with <n> lines context\"),\n \t\t\t       PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified),\n@@ -5513,11 +5512,11 @@ struct option *add_diff_options(const struct option *opts,\n \t\tOPT_BITOP(0, \"patch-with-raw\", &options->output_format,\n \t\t\t  N_(\"synonym for '-p --raw'\"),\n \t\t\t  DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW,\n-\t\t\t  DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_DEFAULT),\n \t\tOPT_BITOP(0, \"patch-with-stat\", &options->output_format,\n \t\t\t  N_(\"synonym for '-p --stat'\"),\n \t\t\t  DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT,\n-\t\t\t  DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT),\n+\t\t\t  DIFF_FORMAT_DEFAULT),\n \t\tOPT_BITOP(0, \"numstat\", &options->output_format,\n \t\t\t  N_(\"machine friendly --stat\"),\n \t\t\t  DIFF_FORMAT_NUMSTAT, DIFF_FORMAT_DEFAULT),\n@@ -6648,7 +6647,7 @@ void diff_flush(struct diff_options *options)\n \t\tseparator++;\n \t}\n \n-\tif (output_format == DIFF_FORMAT_NO_OUTPUT &&\n+\tif (!output_format &&\n \t    options->flags.exit_with_status &&\n \t    options->flags.diff_from_contents) {\n \t\t/*\ndiff --git a/diff.h b/diff.h\nindex 44da1a4ca7..48e8ff962e 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -94,7 +94,6 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n \n typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n \n-#define DIFF_FORMAT_NO_OUTPUT\t0x0000\n #define DIFF_FORMAT_RAW\t\t0x0001\n #define DIFF_FORMAT_DIFFSTAT\t0x0002\n #define DIFF_FORMAT_NUMSTAT\t0x0004\ndiff --git a/log-tree.c b/log-tree.c\nindex f4b22a60cc..b073e1dea4 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -874,7 +874,7 @@ int log_tree_diff_flush(struct rev_info *opt)\n \n \tif (diff_queue_is_empty(&opt->diffopt)) {\n \t\tint saved_fmt = opt->diffopt.output_format;\n-\t\topt->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\topt->diffopt.output_format = 0;\n \t\tdiff_flush(&opt->diffopt);\n \t\topt->diffopt.output_format = saved_fmt;\n \t\treturn 0;\n@@ -882,7 +882,7 @@ int log_tree_diff_flush(struct rev_info *opt)\n \n \tif (opt->loginfo && !opt->no_commit_id) {\n \t\tshow_log(opt);\n-\t\tif ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&\n+\t\tif (opt->diffopt.output_format &&\n \t\t    opt->verbose_header &&\n \t\t    opt->commit_format != CMIT_FMT_ONELINE &&\n \t\t    !commit_format_is_empty(opt->commit_format)) {\ndiff --git a/merge-ort.c b/merge-ort.c\nindex a50b095c47..392549a24b 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3248,7 +3248,7 @@ static int detect_regular_renames(struct merge_options *opt,\n \t\tdiff_opts.rename_limit = 7000;\n \tdiff_opts.rename_score = opt->rename_score;\n \tdiff_opts.show_rename_progress = opt->show_rename_progress;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tdiff_setup_done(&diff_opts);\n \n \tdiff_queued_diff = renames->pairs[side_index];\n@@ -3269,7 +3269,7 @@ static int detect_regular_renames(struct merge_options *opt,\n \n \trenames->pairs[side_index] = diff_queued_diff;\n \n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tdiff_queued_diff.nr = 0;\n \tdiff_queued_diff.queue = NULL;\n \tdiff_flush(&diff_opts);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 8e87b6386d..d5678424f3 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1904,7 +1904,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,\n \topts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 7000;\n \topts.rename_score = opt->rename_score;\n \topts.show_rename_progress = opt->show_rename_progress;\n-\topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\topts.output_format = 0;\n \tdiff_setup_done(&opts);\n \tdiff_tree_oid(&o_tree->object.oid, &tree->object.oid, \"\", &opts);\n \tdiffcore_std(&opts);\n@@ -1914,7 +1914,7 @@ static struct diff_queue_struct *get_diffpairs(struct merge_options *opt,\n \tret = xmalloc(sizeof(*ret));\n \t*ret = diff_queued_diff;\n \n-\topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\topts.output_format = 0;\n \tdiff_queued_diff.nr = 0;\n \tdiff_queued_diff.queue = NULL;\n \tdiff_flush(&opts);\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 233e49e319..9a8bac2579 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -139,7 +139,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,\n \n \trepo_diff_setup(o->repo, &opt);\n \topt.flags.recursive = 1;\n-\topt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\topt.output_format = 0;\n \tdiff_setup_done(&opt);\n \tdiff_tree_oid(base, remote, \"\", &opt);\n \tdiffcore_std(&opt);\n@@ -201,7 +201,7 @@ static void diff_tree_local(struct notes_merge_options *o,\n \n \trepo_diff_setup(o->repo, &opt);\n \topt.flags.recursive = 1;\n-\topt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\topt.output_format = 0;\n \tdiff_setup_done(&opt);\n \tdiff_tree_oid(base, local, \"\", &opt);\n \tdiffcore_std(&opt);\ndiff --git a/revision.c b/revision.c\nindex 07d653c197..52c2f415c7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2966,7 +2966,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \n \t/* Did the user ask for any diff output? Run the diff! */\n-\tif (revs->diffopt.output_format & ~(DIFF_FORMAT_DEFAULT | DIFF_FORMAT_NO_OUTPUT))\n+\tif (revs->diffopt.output_format & ~DIFF_FORMAT_DEFAULT)\n \t\trevs->diff = 1;\n \n \t/* Pickaxe, diff-filter and rename following need diffs */\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 20bb15f38d..757a271348 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -627,7 +627,7 @@ static void try_to_follow_renames(const struct object_id *old_oid,\n \trepo_diff_setup(opt->repo, &diff_opts);\n \tdiff_opts.flags.recursive = 1;\n \tdiff_opts.flags.find_copies_harder = 1;\n-\tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\tdiff_opts.output_format = 0;\n \tdiff_opts.single_follow = opt->pathspec.items[0].match;\n \tdiff_opts.break_opt = opt->break_opt;\n \tdiff_opts.rename_score = opt->rename_score;\n-- \n2.40.0+fc1\n\n"},{"id":"477171","messageId":"645df6e614f00_215cec29462@chronos.notmuch","threadId":"59734","inReplyTo":"20230512080339.2186324-1-felipe.contreras@gmail.com","subject":"Re: [PATCH v1 0/7] diff: fix -s and --no-patch","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-12T08:20:54Z","receivedAt":"2023-05-12T08:20:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras wrote:\n> This fixes an issue Sergey Organov reported.\n\nSergey, as you can see this series fixes the issue you reported.\n\nFirst, I think these should remain working the same, simply for convenience:\n\n * git diff         # default output\n * git diff --patch # patch output\n * git diff --raw   # raw output\n * git diff --stat  # stat output\n\nI don't think there's a way I can be convinced otherwise.\n\nBut there's many changes:\n\n 1. git diff -s --raw                 # before: nil, after: raw\n 2. git diff --no-patch --raw         # before: nil, after: raw\n 3. git diff --patch --no-patch --raw # before: nil, after: raw\n 4. git diff --raw --patch --no-patch # before: nil, after: raw\n\nI don't think there's any way you can say my 174 changes make the code work\n\"exactly the same\".\n\nAnd this is better than Junio's solution, because #4 outputs a raw format,\nwhile in Junio's solution it doesn't output anything.\n\nEven if you don't agree with everything, this solution is better than the\nstatus quo, and it's better than Junio's solution as it fixes --no-patch\nimmediately.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"477174","messageId":"87r0rlj3od.fsf@osv.gnss.ru","threadId":"59734","inReplyTo":"645df6e614f00_215cec29462@chronos.notmuch","subject":"Re: [PATCH v1 0/7] diff: fix -s and --no-patch","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2023-05-12T09:32:18Z","receivedAt":"2023-05-12T09:32:39Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Felipe Contreras wrote:\n>> This fixes an issue Sergey Organov reported.\n>\n> Sergey, as you can see this series fixes the issue you reported.\n>\n> First, I think these should remain working the same, simply for convenience:\n>\n>  * git diff         # default output\n>  * git diff --patch # patch output\n>  * git diff --raw   # raw output\n>  * git diff --stat  # stat output\n>\n> I don't think there's a way I can be convinced otherwise.\n\nFine with me.\n\n>\n> But there's many changes:\n>\n>  1. git diff -s --raw                 # before: nil, after: raw\n>  2. git diff --no-patch --raw         # before: nil, after: raw\n>  3. git diff --patch --no-patch --raw # before: nil, after: raw\n>  4. git diff --raw --patch --no-patch # before: nil, after: raw\n\nFine as well.\n\n>\n> I don't think there's any way you can say my 174 changes make the code work\n> \"exactly the same\".\n\nI said that in the context where we discussed entirely separate issue\n\"handling of defaults by Git commands\". Irrelevant to these series as\nthey don't touch this aspect as visible from outside, even though you\ndo change the implementation for better.\n\n>\n> And this is better than Junio's solution, because #4 outputs a raw format,\n> while in Junio's solution it doesn't output anything.\n\nYes, and that's where I agreed from the very beginning.\n\n> Even if you don't agree with everything, this solution is better than the\n> status quo, and it's better than Junio's solution as it fixes --no-patch\n> immediately.\n\nYep, it fixes \"--no-patch\" semantics indeed, and as I already said, I do\nvote in favor of this change, for what it's worth.\n\nThanks,\n-- Sergey Organov\n"}]}