{"thread":{"id":"4643","subject":"[PATCH 1/5] git-merge: Don't use -p when outputting summary","startedAt":"2006-06-23T21:45:40Z","lastAt":"2006-06-24T11:29:43Z","messageCount":8,"participants":["Timo Hirvonen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"22376","messageId":"20060624004540.3a6705e1.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624003315.804a1796.tihirvon@gmail.com","subject":"[PATCH 1/5] git-merge: Don't use -p when outputting summary","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T21:45:40Z","receivedAt":"2006-06-23T21:45:40Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"-p is not needed and we only want diffstat and summary.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n git-merge.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-merge.sh b/git-merge.sh\nindex af1f25b..da5657e 100755\n--- a/git-merge.sh\n+++ b/git-merge.sh\n@@ -55,7 +55,7 @@ finish () {\n \n \tcase \"$no_summary\" in\n \t'')\n-\t\tgit-diff-tree -p --stat --summary -M \"$head\" \"$1\"\n+\t\tgit-diff-tree --stat --summary -M \"$head\" \"$1\"\n \t\t;;\n \tesac\n }\n-- \n1.4.1.rc1.gf603-dirty\n"},{"id":"22380","messageId":"20060624005252.c694e421.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624003315.804a1796.tihirvon@gmail.com","subject":"[PATCH 2/5] Rework diff options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T21:52:52Z","receivedAt":"2006-06-23T21:52:52Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Make raw, diffstat, summary and patch diff output format options\nindependent of each other.\n\nname-only, name-status and checkdiff options override other options.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff-files.c |    6 +-\n builtin-diff.c       |   22 +++---\n builtin-log.c        |    8 +-\n combine-diff.c       |   45 ++++-------\n diff.c               |  199 +++++++++++++++++++++++++-------------------------\n diff.h               |   29 ++++---\n log-tree.c           |   15 +++-\n revision.c           |    4 +\n 8 files changed, 160 insertions(+), 168 deletions(-)\n\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex 5afc1d7..42ca07d 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -47,9 +47,9 @@ int cmd_diff_files(int argc, const char \n \t/*\n \t * Backward compatibility wart - \"diff-files -s\" used to\n \t * defeat the common diff option \"-s\" which asked for\n-\t * DIFF_FORMAT_NO_OUTPUT.\n+\t * OUTPUT_FMT_NONE\n \t */\n-\tif (rev.diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)\n-\t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n+\tif (rev.diffopt.output_fmt & OUTPUT_FMT_NONE)\n+\t\trev.diffopt.output_fmt = OUTPUT_FMT_RAW;\n \treturn run_diff_files(&rev, silent);\n }\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 99a2f76..372894a 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -40,7 +40,7 @@ static int builtin_diff_files(struct rev\n \t\telse if (!strcmp(arg, \"-q\"))\n \t\t\tsilent = 1;\n \t\telse if (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -56,15 +56,15 @@ static int builtin_diff_files(struct rev\n \t    3 < revs->max_count)\n \t\tusage(builtin_diff_usage);\n \tif (revs->max_count < 0 &&\n-\t    (revs->diffopt.output_format == DIFF_FORMAT_PATCH))\n+\t    (revs->diffopt.output_fmt & OUTPUT_FMT_PATCH))\n \t\trevs->combine_merges = revs->dense_combined_merges = 1;\n \t/*\n \t * Backward compatibility wart - \"diff-files -s\" used to\n \t * defeat the common diff option \"-s\" which asked for\n-\t * DIFF_FORMAT_NO_OUTPUT.\n+\t * OUTPUT_FMT_NONE\n \t */\n-\tif (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)\n-\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\tif (revs->diffopt.output_fmt & OUTPUT_FMT_NONE)\n+\t\trevs->diffopt.output_fmt = OUTPUT_FMT_RAW;\n \treturn run_diff_files(revs, silent);\n }\n \n@@ -110,7 +110,7 @@ static int builtin_diff_b_f(struct rev_i\n \twhile (1 < argc) {\n \t\tconst char *arg = argv[1];\n \t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -140,7 +140,7 @@ static int builtin_diff_blobs(struct rev\n \twhile (1 < argc) {\n \t\tconst char *arg = argv[1];\n \t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -163,7 +163,7 @@ static int builtin_diff_index(struct rev\n \t\tif (!strcmp(arg, \"--cached\"))\n \t\t\tcached = 1;\n \t\telse if (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -188,7 +188,7 @@ static int builtin_diff_tree(struct rev_\n \twhile (1 < argc) {\n \t\tconst char *arg = argv[1];\n \t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -217,7 +217,7 @@ static int builtin_diff_combined(struct \n \twhile (1 < argc) {\n \t\tconst char *arg = argv[1];\n \t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -276,7 +276,7 @@ int cmd_diff(int argc, const char **argv\n \n \tgit_config(git_diff_config);\n \tinit_revisions(&rev);\n-\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n+\trev.diffopt.output_fmt = OUTPUT_FMT_PATCH;\n \n \targc = setup_revisions(argc, argv, &rev, NULL);\n \t/* Do we have --cached and not have a pending object, then\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5a8a50b..e4a6385 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -26,8 +26,8 @@ static int cmd_log_wc(int argc, const ch\n \tif (rev->always_show_header) {\n \t\tif (rev->diffopt.pickaxe || rev->diffopt.filter) {\n \t\t\trev->always_show_header = 0;\n-\t\t\tif (rev->diffopt.output_format == DIFF_FORMAT_RAW)\n-\t\t\t\trev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\t\tif (rev->diffopt.output_fmt & OUTPUT_FMT_RAW)\n+\t\t\t\trev->diffopt.output_fmt |= OUTPUT_FMT_NONE;\n \t\t}\n \t}\n \n@@ -176,11 +176,9 @@ int cmd_format_patch(int argc, const cha\n \trev.commit_format = CMIT_FMT_EMAIL;\n \trev.verbose_header = 1;\n \trev.diff = 1;\n-\trev.diffopt.with_raw = 0;\n-\trev.diffopt.with_stat = 1;\n \trev.combine_merges = 0;\n \trev.ignore_merges = 1;\n-\trev.diffopt.output_format = DIFF_FORMAT_PATCH;\n+\trev.diffopt.output_fmt = OUTPUT_FMT_DIFFSTAT | OUTPUT_FMT_PATCH;\n \n \tgit_config(git_format_config);\n \trev.extra_headers = extra_headers;\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 64b20cc..d0d8d01 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -771,7 +771,7 @@ static void show_raw_diff(struct combine\n \tif (rev->loginfo)\n \t\tshow_log(rev, rev->loginfo, \"\\n\");\n \n-\tif (opt->output_format == DIFF_FORMAT_RAW) {\n+\tif (opt->output_fmt & OUTPUT_FMT_RAW) {\n \t\toffset = strlen(COLONS) - num_parent;\n \t\tif (offset < 0)\n \t\t\toffset = 0;\n@@ -791,8 +791,7 @@ static void show_raw_diff(struct combine\n \t\tprintf(\" %s \", diff_unique_abbrev(p->sha1, opt->abbrev));\n \t}\n \n-\tif (opt->output_format == DIFF_FORMAT_RAW ||\n-\t    opt->output_format == DIFF_FORMAT_NAME_STATUS) {\n+\tif (opt->output_fmt & (OUTPUT_FMT_RAW | OUTPUT_FMT_NAME_STATUS)) {\n \t\tfor (i = 0; i < num_parent; i++)\n \t\t\tputchar(p->parent[i].status);\n \t\tputchar(inter_name_termination);\n@@ -818,17 +817,12 @@ void show_combined_diff(struct combine_d\n \tstruct diff_options *opt = &rev->diffopt;\n \tif (!p->len)\n \t\treturn;\n-\tswitch (opt->output_format) {\n-\tcase DIFF_FORMAT_RAW:\n-\tcase DIFF_FORMAT_NAME_STATUS:\n-\tcase DIFF_FORMAT_NAME:\n+\tif (opt->output_fmt & (OUTPUT_FMT_RAW |\n+\t\t\t       OUTPUT_FMT_NAME |\n+\t\t\t       OUTPUT_FMT_NAME_STATUS)) {\n \t\tshow_raw_diff(p, num_parent, rev);\n-\t\treturn;\n-\tcase DIFF_FORMAT_PATCH:\n+\t} else if (opt->output_fmt & OUTPUT_FMT_PATCH) {\n \t\tshow_patch_diff(p, num_parent, dense, rev);\n-\t\treturn;\n-\tdefault:\n-\t\treturn;\n \t}\n }\n \n@@ -842,13 +836,9 @@ void diff_tree_combined(const unsigned c\n \tstruct diff_options diffopts;\n \tstruct combine_diff_path *p, *paths = NULL;\n \tint i, num_paths;\n-\tint do_diffstat;\n \n-\tdo_diffstat = (opt->output_format == DIFF_FORMAT_DIFFSTAT ||\n-\t\t       opt->with_stat);\n \tdiffopts = *opt;\n-\tdiffopts.with_raw = 0;\n-\tdiffopts.with_stat = 0;\n+\tdiffopts.output_fmt &= ~(OUTPUT_FMT_RAW | OUTPUT_FMT_DIFFSTAT);\n \tdiffopts.recursive = 1;\n \n \t/* find set of paths that everybody touches */\n@@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c\n \t\t/* show stat against the first parent even\n \t\t * when doing combined diff.\n \t\t */\n-\t\tif (i == 0 && do_diffstat)\n-\t\t\tdiffopts.output_format = DIFF_FORMAT_DIFFSTAT;\n+\t\tif (i == 0 && opt->output_fmt & OUTPUT_FMT_DIFFSTAT)\n+\t\t\tdiffopts.output_fmt |= OUTPUT_FMT_DIFFSTAT;\n \t\telse\n-\t\t\tdiffopts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\t\tdiffopts.output_fmt |= OUTPUT_FMT_NONE;\n \t\tdiff_tree_sha1(parent[i], sha1, \"\", &diffopts);\n \t\tdiffcore_std(&diffopts);\n \t\tpaths = intersect_paths(paths, i, num_parent);\n \n-\t\tif (do_diffstat && rev->loginfo)\n-\t\t\tshow_log(rev, rev->loginfo,\n-\t\t\t\t opt->with_stat ? \"---\\n\" : \"\\n\");\n+\t\tif (opt->output_fmt & OUTPUT_FMT_DIFFSTAT && rev->loginfo)\n+\t\t\tshow_log(rev, rev->loginfo, \"---\\n\");\n \t\tdiff_flush(&diffopts);\n-\t\tif (opt->with_stat)\n+\t\tif (opt->output_fmt & OUTPUT_FMT_DIFFSTAT)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c\n \t\t\tnum_paths++;\n \t}\n \tif (num_paths) {\n-\t\tif (opt->with_raw) {\n-\t\t\tint saved_format = opt->output_format;\n-\t\t\topt->output_format = DIFF_FORMAT_RAW;\n+\t\tif (opt->output_fmt & OUTPUT_FMT_RAW) {\n+\t\t\tint saved_fmt = opt->output_fmt;\n+\t\t\topt->output_fmt |= OUTPUT_FMT_RAW;\n \t\t\tfor (p = paths; p; p = p->next) {\n \t\t\t\tshow_combined_diff(p, num_parent, dense, rev);\n \t\t\t}\n-\t\t\topt->output_format = saved_format;\n+\t\t\topt->output_fmt = saved_fmt;\n \t\t\tputchar(opt->line_termination);\n \t\t}\n \t\tfor (p = paths; p; p = p->next) {\ndiff --git a/diff.c b/diff.c\nindex 1db0285..6eb7db0 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -203,7 +203,7 @@ static void emit_rewrite_diff(const char\n static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n {\n \tif (!DIFF_FILE_VALID(one)) {\n-\t\tmf->ptr = \"\"; /* does not matter */\n+\t\tmf->ptr = (char *)\"\"; /* does not matter */\n \t\tmf->size = 0;\n \t\treturn 0;\n \t}\n@@ -395,7 +395,7 @@ static void show_stats(struct diffstat_t\n \t}\n \n \tfor (i = 0; i < data->nr; i++) {\n-\t\tchar *prefix = \"\";\n+\t\tconst char *prefix = \"\";\n \t\tchar *name = data->files[i]->name;\n \t\tint added = data->files[i]->added;\n \t\tint deleted = data->files[i]->deleted;\n@@ -917,7 +917,7 @@ int diff_populate_filespec(struct diff_f\n \t\t\terr_empty:\n \t\t\t\terr = -1;\n \t\t\tempty:\n-\t\t\t\ts->data = \"\";\n+\t\t\t\ts->data = (char *)\"\";\n \t\t\t\ts->size = 0;\n \t\t\t\treturn err;\n \t\t\t}\n@@ -1354,7 +1354,7 @@ static void run_checkdiff(struct diff_fi\n void diff_setup(struct diff_options *options)\n {\n \tmemset(options, 0, sizeof(*options));\n-\toptions->output_format = DIFF_FORMAT_RAW;\n+\toptions->output_fmt = OUTPUT_FMT_RAW;\n \toptions->line_termination = '\\n';\n \toptions->break_opt = -1;\n \toptions->rename_limit = -1;\n@@ -1371,23 +1371,26 @@ int diff_setup_done(struct diff_options \n \t    (0 <= options->rename_limit && !options->detect_rename))\n \t\treturn -1;\n \n+\tif (options->output_fmt & OUTPUT_FMT_NONE)\n+\t\toptions->output_fmt = 0;\n+\n+\tif (options->output_fmt & (OUTPUT_FMT_NAME |\n+\t\t\t\t   OUTPUT_FMT_CHECKDIFF |\n+\t\t\t\t   OUTPUT_FMT_NONE))\n+\t\toptions->output_fmt &= ~(OUTPUT_FMT_RAW |\n+\t\t\t\t\t OUTPUT_FMT_DIFFSTAT |\n+\t\t\t\t\t OUTPUT_FMT_SUMMARY |\n+\t\t\t\t\t OUTPUT_FMT_PATCH);\n+\n \t/*\n \t * These cases always need recursive; we do not drop caller-supplied\n \t * recursive bits for other formats here.\n \t */\n-\tif ((options->output_format == DIFF_FORMAT_PATCH) ||\n-\t    (options->output_format == DIFF_FORMAT_DIFFSTAT) ||\n-\t    (options->output_format == DIFF_FORMAT_CHECKDIFF))\n+\tif (options->output_fmt & (OUTPUT_FMT_PATCH |\n+\t\t\t\t   OUTPUT_FMT_DIFFSTAT |\n+\t\t\t\t   OUTPUT_FMT_CHECKDIFF))\n \t\toptions->recursive = 1;\n \n-\t/*\n-\t * These combinations do not make sense.\n-\t */\n-\tif (options->output_format == DIFF_FORMAT_RAW)\n-\t\toptions->with_raw = 0;\n-\tif (options->output_format == DIFF_FORMAT_DIFFSTAT)\n-\t\toptions->with_stat  = 0;\n-\n \tif (options->detect_rename && options->rename_limit < 0)\n \t\toptions->rename_limit = diff_rename_limit_default;\n \tif (options->setup & DIFF_SETUP_USE_CACHE) {\n@@ -1408,7 +1411,7 @@ int diff_setup_done(struct diff_options \n \treturn 0;\n }\n \n-int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n+static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n {\n \tchar c, *eq;\n \tint len;\n@@ -1459,22 +1462,20 @@ int diff_opt_parse(struct diff_options *\n {\n \tconst char *arg = av[0];\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\"))\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n+\t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n \telse if (opt_arg(arg, 'U', \"unified\", &options->context))\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n+\t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n \telse if (!strcmp(arg, \"--patch-with-raw\")) {\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n-\t\toptions->with_raw = 1;\n+\t\toptions->output_fmt |= OUTPUT_FMT_PATCH | OUTPUT_FMT_RAW;\n \t}\n \telse if (!strcmp(arg, \"--stat\"))\n-\t\toptions->output_format = DIFF_FORMAT_DIFFSTAT;\n+\t\toptions->output_fmt |= OUTPUT_FMT_DIFFSTAT;\n \telse if (!strcmp(arg, \"--check\"))\n-\t\toptions->output_format = DIFF_FORMAT_CHECKDIFF;\n+\t\toptions->output_fmt |= OUTPUT_FMT_CHECKDIFF;\n \telse if (!strcmp(arg, \"--summary\"))\n-\t\toptions->summary = 1;\n+\t\toptions->output_fmt |= OUTPUT_FMT_SUMMARY;\n \telse if (!strcmp(arg, \"--patch-with-stat\")) {\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n-\t\toptions->with_stat = 1;\n+\t\toptions->output_fmt |= OUTPUT_FMT_PATCH | OUTPUT_FMT_DIFFSTAT;\n \t}\n \telse if (!strcmp(arg, \"-z\"))\n \t\toptions->line_termination = 0;\n@@ -1483,19 +1484,20 @@ int diff_opt_parse(struct diff_options *\n \telse if (!strcmp(arg, \"--full-index\"))\n \t\toptions->full_index = 1;\n \telse if (!strcmp(arg, \"--binary\")) {\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n+\t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n \t\toptions->full_index = options->binary = 1;\n \t}\n \telse if (!strcmp(arg, \"--name-only\"))\n-\t\toptions->output_format = DIFF_FORMAT_NAME;\n+\t\toptions->output_fmt |= OUTPUT_FMT_NAME;\n \telse if (!strcmp(arg, \"--name-status\"))\n-\t\toptions->output_format = DIFF_FORMAT_NAME_STATUS;\n+\t\toptions->output_fmt |= OUTPUT_FMT_NAME_STATUS;\n \telse if (!strcmp(arg, \"-R\"))\n \t\toptions->reverse_diff = 1;\n \telse if (!strncmp(arg, \"-S\", 2))\n \t\toptions->pickaxe = arg + 2;\n-\telse if (!strcmp(arg, \"-s\"))\n-\t\toptions->output_format = DIFF_FORMAT_NO_OUTPUT;\n+\telse if (!strcmp(arg, \"-s\")) {\n+\t\toptions->output_fmt |= OUTPUT_FMT_NONE;\n+\t}\n \telse if (!strncmp(arg, \"-O\", 2))\n \t\toptions->orderfile = arg + 2;\n \telse if (!strncmp(arg, \"--diff-filter=\", 14))\n@@ -1669,7 +1671,7 @@ static void diff_flush_raw(struct diff_f\n \t\t\t   int line_termination,\n \t\t\t   int inter_name_termination,\n \t\t\t   struct diff_options *options,\n-\t\t\t   int output_format)\n+\t\t\t   int raw)\n {\n \tint two_paths;\n \tchar status[10];\n@@ -1703,7 +1705,7 @@ static void diff_flush_raw(struct diff_f\n \t\ttwo_paths = 0;\n \t\tbreak;\n \t}\n-\tif (output_format != DIFF_FORMAT_NAME_STATUS) {\n+\tif (raw) {\n \t\tprintf(\":%06o %06o %s \",\n \t\t       p->one->mode, p->two->mode,\n \t\t       diff_unique_abbrev(p->one->sha1, abbrev));\n@@ -1720,16 +1722,12 @@ static void diff_flush_raw(struct diff_f\n \t\tfree((void*)path_two);\n }\n \n-static void diff_flush_name(struct diff_filepair *p,\n-\t\t\t    int inter_name_termination,\n-\t\t\t    int line_termination)\n+static void diff_flush_name(struct diff_filepair *p, int line_termination)\n {\n \tchar *path = p->two->path;\n \n \tif (line_termination)\n \t\tpath = quote_one(p->two->path);\n-\telse\n-\t\tpath = p->two->path;\n \tprintf(\"%s%c\", path, line_termination);\n \tif (p->two->path != path)\n \t\tfree(path);\n@@ -1916,50 +1914,44 @@ static void diff_resolve_rename_copy(voi\n \tdiff_debug_queue(\"resolve-rename-copy done\", q);\n }\n \n-static void flush_one_pair(struct diff_filepair *p,\n-\t\t\t   int diff_output_format,\n-\t\t\t   struct diff_options *options,\n-\t\t\t   struct diffstat_t *diffstat)\n+static int check_pair_status(struct diff_filepair *p)\n {\n-\tint inter_name_termination = '\\t';\n-\tint line_termination = options->line_termination;\n-\tif (!line_termination)\n-\t\tinter_name_termination = 0;\n-\n \tswitch (p->status) {\n \tcase DIFF_STATUS_UNKNOWN:\n-\t\tbreak;\n+\t\treturn 0;\n \tcase 0:\n \t\tdie(\"internal error in diff-resolve-rename-copy\");\n-\t\tbreak;\n \tdefault:\n-\t\tswitch (diff_output_format) {\n-\t\tcase DIFF_FORMAT_DIFFSTAT:\n-\t\t\tdiff_flush_stat(p, options, diffstat);\n-\t\t\tbreak;\n-\t\tcase DIFF_FORMAT_CHECKDIFF:\n-\t\t\tdiff_flush_checkdiff(p, options);\n-\t\t\tbreak;\n-\t\tcase DIFF_FORMAT_PATCH:\n-\t\t\tdiff_flush_patch(p, options);\n-\t\t\tbreak;\n-\t\tcase DIFF_FORMAT_RAW:\n-\t\tcase DIFF_FORMAT_NAME_STATUS:\n-\t\t\tdiff_flush_raw(p, line_termination,\n-\t\t\t\t       inter_name_termination,\n-\t\t\t\t       options, diff_output_format);\n-\t\t\tbreak;\n-\t\tcase DIFF_FORMAT_NAME:\n-\t\t\tdiff_flush_name(p,\n-\t\t\t\t\tinter_name_termination,\n-\t\t\t\t\tline_termination);\n-\t\t\tbreak;\n-\t\tcase DIFF_FORMAT_NO_OUTPUT:\n-\t\t\tbreak;\n-\t\t}\n+\t\treturn 1;\n \t}\n }\n \n+static void flush_one_pair_raw(struct diff_filepair *p,\n+\t\t\t   struct diff_options *options, int raw)\n+{\n+\tint inter_name_termination = '\\t';\n+\tint line_termination = options->line_termination;\n+\n+\tif (!line_termination)\n+\t\tinter_name_termination = 0;\n+\tdiff_flush_raw(p, line_termination, inter_name_termination,\n+\t\t\toptions, raw);\n+}\n+\n+static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n+{\n+\tint fmt = opt->output_fmt;\n+\n+\tif (fmt & OUTPUT_FMT_CHECKDIFF)\n+\t\tdiff_flush_checkdiff(p, opt);\n+\telse if (fmt & OUTPUT_FMT_RAW)\n+\t\tflush_one_pair_raw(p, opt, 1);\n+\telse if (fmt & OUTPUT_FMT_NAME_STATUS)\n+\t\tflush_one_pair_raw(p, opt, 0);\n+\telse if (fmt & OUTPUT_FMT_NAME)\n+\t\tdiff_flush_name(p, opt->line_termination);\n+}\n+\n static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)\n {\n \tif (fs->mode)\n@@ -2042,55 +2034,60 @@ static void diff_summary(struct diff_fil\n void diff_flush(struct diff_options *options)\n {\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n-\tint i;\n-\tint diff_output_format = options->output_format;\n-\tstruct diffstat_t *diffstat = NULL;\n+\tint i, output_fmt = options->output_fmt;\n \n-\tif (diff_output_format == DIFF_FORMAT_DIFFSTAT || options->with_stat) {\n-\t\tdiffstat = xcalloc(sizeof (struct diffstat_t), 1);\n-\t\tdiffstat->xm.consume = diffstat_consume;\n-\t}\n+\t/*\n+\t * Order: raw, stat, summary, patch\n+\t * or:    name/name-status/checkdiff (other bits clear)\n+\t */\n \n-\tif (options->with_raw) {\n+\tif (output_fmt & (OUTPUT_FMT_RAW |\n+\t\t\t  OUTPUT_FMT_NAME |\n+\t\t\t  OUTPUT_FMT_NAME_STATUS |\n+\t\t\t  OUTPUT_FMT_CHECKDIFF)) {\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tflush_one_pair(p, DIFF_FORMAT_RAW, options, NULL);\n+\t\t\tif (check_pair_status(p))\n+\t\t\t\tflush_one_pair(p, options);\n \t\t}\n-\t\tputchar(options->line_termination);\n \t}\n-\tif (options->with_stat) {\n+\n+\tif (output_fmt & OUTPUT_FMT_DIFFSTAT) {\n+\t\tstruct diffstat_t *diffstat;\n+\n+\t\tdiffstat = xcalloc(sizeof (struct diffstat_t), 1);\n+\t\tdiffstat->xm.consume = diffstat_consume;\n \t\tfor (i = 0; i < q->nr; i++) {\n \t\t\tstruct diff_filepair *p = q->queue[i];\n-\t\t\tflush_one_pair(p, DIFF_FORMAT_DIFFSTAT, options,\n-\t\t\t\t       diffstat);\n+\t\t\tif (check_pair_status(p))\n+\t\t\t\tdiff_flush_stat(p, options, diffstat);\n \t\t}\n \t\tshow_stats(diffstat);\n \t\tfree(diffstat);\n-\t\tdiffstat = NULL;\n-\t\tif (options->summary)\n-\t\t\tfor (i = 0; i < q->nr; i++)\n-\t\t\t\tdiff_summary(q->queue[i]);\n+\t}\n+\n+\tif (output_fmt & OUTPUT_FMT_SUMMARY) {\n+\t\tfor (i = 0; i < q->nr; i++)\n+\t\t\tdiff_summary(q->queue[i]);\n+\t}\n+\n+\tif (output_fmt & (OUTPUT_FMT_DIFFSTAT | OUTPUT_FMT_SUMMARY)) {\n \t\tif (options->stat_sep)\n \t\t\tfputs(options->stat_sep, stdout);\n \t\telse\n \t\t\tputchar(options->line_termination);\n \t}\n-\tfor (i = 0; i < q->nr; i++) {\n-\t\tstruct diff_filepair *p = q->queue[i];\n-\t\tflush_one_pair(p, diff_output_format, options, diffstat);\n-\t}\n \n-\tif (diffstat) {\n-\t\tshow_stats(diffstat);\n-\t\tfree(diffstat);\n+\tif (output_fmt & OUTPUT_FMT_PATCH) {\n+\t\tfor (i = 0; i < q->nr; i++) {\n+\t\t\tstruct diff_filepair *p = q->queue[i];\n+\t\t\tif (check_pair_status(p))\n+\t\t\t\tdiff_flush_patch(p, options);\n+\t\t}\n \t}\n \n-\tfor (i = 0; i < q->nr; i++) {\n-\t\tif (diffstat && options->summary)\n-\t\t\tdiff_summary(q->queue[i]);\n+\tfor (i = 0; i < q->nr; i++)\n \t\tdiff_free_filepair(q->queue[i]);\n-\t}\n-\n \tfree(q->queue);\n \tq->queue = NULL;\n \tq->nr = q->alloc = 0;\ndiff --git a/diff.h b/diff.h\nindex de9de57..a16bc5a 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -20,25 +20,37 @@ typedef void (*add_remove_fn_t)(struct d\n \t\t    const unsigned char *sha1,\n \t\t    const char *base, const char *path);\n \n+#define OUTPUT_FMT_RAW\t\t0x0001\n+#define OUTPUT_FMT_DIFFSTAT\t0x0002\n+#define OUTPUT_FMT_SUMMARY\t0x0004\n+#define OUTPUT_FMT_PATCH\t0x0008\n+\n+/* These override all above */\n+#define OUTPUT_FMT_NAME\t\t0x0010\n+#define OUTPUT_FMT_NAME_STATUS\t0x0020\n+#define OUTPUT_FMT_CHECKDIFF\t0x0040\n+\n+/* Same as output_fmt = 0 but we know that -s flag was given\n+ * and we should not give default value to output_fmt.\n+ */\n+#define OUTPUT_FMT_NONE\t\t0x0080\n+\n struct diff_options {\n \tconst char *filter;\n \tconst char *orderfile;\n \tconst char *pickaxe;\n \tunsigned recursive:1,\n-\t\t with_raw:1,\n-\t\t with_stat:1,\n \t\t tree_in_recursive:1,\n \t\t binary:1,\n \t\t full_index:1,\n \t\t silent_on_remove:1,\n \t\t find_copies_harder:1,\n-\t\t summary:1,\n \t\t color_diff:1;\n \tint context;\n \tint break_opt;\n \tint detect_rename;\n \tint line_termination;\n-\tint output_format;\n+\tint output_fmt;\n \tint pickaxe_opts;\n \tint rename_score;\n \tint reverse_diff;\n@@ -150,15 +162,6 @@ #define COMMON_DIFF_OPTIONS_HELP \\\n \"                show all files diff when -S is used and hit is found.\\n\"\n \n extern int diff_queue_is_empty(void);\n-\n-#define DIFF_FORMAT_RAW\t\t1\n-#define DIFF_FORMAT_PATCH\t2\n-#define DIFF_FORMAT_NO_OUTPUT\t3\n-#define DIFF_FORMAT_NAME\t4\n-#define DIFF_FORMAT_NAME_STATUS\t5\n-#define DIFF_FORMAT_DIFFSTAT\t6\n-#define DIFF_FORMAT_CHECKDIFF\t7\n-\n extern void diff_flush(struct diff_options*);\n \n /* diff-raw status letters */\ndiff --git a/log-tree.c b/log-tree.c\nindex ebb49f2..2cdb0ab 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -156,15 +156,20 @@ int log_tree_diff_flush(struct rev_info \n \tdiffcore_std(&opt->diffopt);\n \n \tif (diff_queue_is_empty()) {\n-\t\tint saved_fmt = opt->diffopt.output_format;\n-\t\topt->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\tint saved_fmt = opt->diffopt.output_fmt;\n+\t\topt->diffopt.output_fmt = OUTPUT_FMT_NONE;\n \t\tdiff_flush(&opt->diffopt);\n-\t\topt->diffopt.output_format = saved_fmt;\n+\t\topt->diffopt.output_fmt = saved_fmt;\n \t\treturn 0;\n \t}\n \n-\tif (opt->loginfo && !opt->no_commit_id)\n-\t\tshow_log(opt, opt->loginfo, opt->diffopt.with_stat ? \"---\\n\" : \"\\n\");\n+\tif (opt->loginfo && !opt->no_commit_id) {\n+\t\tif (opt->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT) {\n+\t\t\tshow_log(opt, opt->loginfo,  \"---\\n\");\n+\t\t} else {\n+\t\t\tshow_log(opt, opt->loginfo,  \"\\n\");\n+\t\t}\n+\t}\n \tdiff_flush(&opt->diffopt);\n \treturn 1;\n }\ndiff --git a/revision.c b/revision.c\nindex b963f2a..4ad2272 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -852,8 +852,8 @@ int setup_revisions(int argc, const char\n \tif (revs->combine_merges) {\n \t\trevs->ignore_merges = 0;\n \t\tif (revs->dense_combined_merges &&\n-\t\t    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))\n-\t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n+\t\t   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))\n+\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;\n \t}\n \trevs->diffopt.abbrev = revs->abbrev;\n \tdiff_setup_done(&revs->diffopt);\n-- \n1.4.1.rc1.gf603-dirty\n"},{"id":"22379","messageId":"20060624005825.92b46022.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624003315.804a1796.tihirvon@gmail.com","subject":"[PATCH 3/5] Set default diff output format after parsing command line","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T21:58:25Z","receivedAt":"2006-06-23T21:58:25Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Move code that sets default output format values after command line\nargument parsing.  Only set defaults if output_fmt was not touched by\ncommand line options.\n\nThis makes \"git diff --raw\" output only in raw format instead of -p --raw.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff-files.c  |    4 ++++\n builtin-diff-index.c  |    4 ++++\n builtin-diff-stages.c |    3 +++\n builtin-diff-tree.c   |    3 +++\n builtin-diff.c        |    4 +++-\n builtin-log.c         |    4 +++-\n diff.c                |    1 -\n 7 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex 42ca07d..4cf2e2f 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -36,6 +36,10 @@ int cmd_diff_files(int argc, const char \n \t\t\tusage(diff_files_usage);\n \t\targv++; argc--;\n \t}\n+\n+\tif (!rev.diffopt.output_fmt)\n+\t\trev.diffopt.output_fmt = OUTPUT_FMT_RAW;\n+\n \t/*\n \t * Make sure there are NO revision (i.e. pending object) parameter,\n \t * rev.max_count is reasonable (0 <= n <= 3),\ndiff --git a/builtin-diff-index.c b/builtin-diff-index.c\nindex c42ef9a..8e58308 100644\n--- a/builtin-diff-index.c\n+++ b/builtin-diff-index.c\n@@ -28,6 +28,10 @@ int cmd_diff_index(int argc, const char \n \t\telse\n \t\t\tusage(diff_cache_usage);\n \t}\n+\n+\tif (!rev.diffopt.output_fmt)\n+\t\trev.diffopt.output_fmt = OUTPUT_FMT_RAW;\n+\n \t/*\n \t * Make sure there is one revision (i.e. pending object),\n \t * and there is no revision filtering parameters.\ndiff --git a/builtin-diff-stages.c b/builtin-diff-stages.c\nindex 7c157ca..c26a589 100644\n--- a/builtin-diff-stages.c\n+++ b/builtin-diff-stages.c\n@@ -85,6 +85,9 @@ int cmd_diff_stages(int ac, const char *\n \t\tac--; av++;\n \t}\n \n+\tif (!diff_options.output_fmt)\n+\t\tdiff_options.output_fmt = OUTPUT_FMT_RAW;\n+\n \tif (ac < 3 ||\n \t    sscanf(av[1], \"%d\", &stage1) != 1 ||\n \t    ! (0 <= stage1 && stage1 <= 3) ||\ndiff --git a/builtin-diff-tree.c b/builtin-diff-tree.c\nindex 3409a39..29b3fe1 100644\n--- a/builtin-diff-tree.c\n+++ b/builtin-diff-tree.c\n@@ -84,6 +84,9 @@ int cmd_diff_tree(int argc, const char *\n \t\tusage(diff_tree_usage);\n \t}\n \n+\tif (!opt->diffopt.output_fmt)\n+\t\topt->diffopt.output_fmt = OUTPUT_FMT_RAW;\n+\n \t/*\n \t * NOTE! We expect \"a ^b\" to be equal to \"a..b\", so we\n \t * reverse the order of the objects if the second one\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 372894a..b6f7727 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -276,9 +276,11 @@ int cmd_diff(int argc, const char **argv\n \n \tgit_config(git_diff_config);\n \tinit_revisions(&rev);\n-\trev.diffopt.output_fmt = OUTPUT_FMT_PATCH;\n \n \targc = setup_revisions(argc, argv, &rev, NULL);\n+\tif (!rev.diffopt.output_fmt)\n+\t\trev.diffopt.output_fmt = OUTPUT_FMT_PATCH;\n+\n \t/* Do we have --cached and not have a pending object, then\n \t * default to HEAD by hand.  Eek.\n \t */\ndiff --git a/builtin-log.c b/builtin-log.c\nindex e4a6385..e72d7fe 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -178,7 +178,6 @@ int cmd_format_patch(int argc, const cha\n \trev.diff = 1;\n \trev.combine_merges = 0;\n \trev.ignore_merges = 1;\n-\trev.diffopt.output_fmt = OUTPUT_FMT_DIFFSTAT | OUTPUT_FMT_PATCH;\n \n \tgit_config(git_format_config);\n \trev.extra_headers = extra_headers;\n@@ -247,6 +246,9 @@ int cmd_format_patch(int argc, const cha\n \tif (argc > 1)\n \t\tdie (\"unrecognized argument: %s\", argv[1]);\n \n+\tif (!rev.diffopt.output_fmt)\n+\t\trev.diffopt.output_fmt = OUTPUT_FMT_DIFFSTAT | OUTPUT_FMT_PATCH;\n+\n \tif (output_directory) {\n \t\tif (use_stdout)\n \t\t\tdie(\"standard output, or directory, which one?\");\ndiff --git a/diff.c b/diff.c\nindex 6eb7db0..45c93c9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1354,7 +1354,6 @@ static void run_checkdiff(struct diff_fi\n void diff_setup(struct diff_options *options)\n {\n \tmemset(options, 0, sizeof(*options));\n-\toptions->output_fmt = OUTPUT_FMT_RAW;\n \toptions->line_termination = '\\n';\n \toptions->break_opt = -1;\n \toptions->rename_limit = -1;\n-- \n1.4.1.rc1.gf603-dirty\n"},{"id":"22378","messageId":"20060624010034.a7355905.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624003315.804a1796.tihirvon@gmail.com","subject":"[PATCH 4/5] Make --raw option available for all diff commands","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T22:00:34Z","receivedAt":"2006-06-23T22:00:34Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Makes --raw a global diff output format option.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff.c |   48 ++++++++++++------------------------------------\n diff.c         |    2 ++\n 2 files changed, 14 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex b6f7727..8fd91b8 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -39,8 +39,6 @@ static int builtin_diff_files(struct rev\n \t\t\trevs->max_count = 3;\n \t\telse if (!strcmp(arg, \"-q\"))\n \t\t\tsilent = 1;\n-\t\telse if (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -107,14 +105,9 @@ static int builtin_diff_b_f(struct rev_i\n \t/* Blob vs file in the working tree*/\n \tstruct stat st;\n \n-\twhile (1 < argc) {\n-\t\tconst char *arg = argv[1];\n-\t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n-\t\telse\n-\t\t\tusage(builtin_diff_usage);\n-\t\targv++; argc--;\n-\t}\n+\tif (argc > 1)\n+\t\tusage(builtin_diff_usage);\n+\n \tif (lstat(path, &st))\n \t\tdie(\"'%s': %s\", path, strerror(errno));\n \tif (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))\n@@ -137,14 +130,9 @@ static int builtin_diff_blobs(struct rev\n \t */\n \tunsigned mode = canon_mode(S_IFREG | 0644);\n \n-\twhile (1 < argc) {\n-\t\tconst char *arg = argv[1];\n-\t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n-\t\telse\n-\t\t\tusage(builtin_diff_usage);\n-\t\targv++; argc--;\n-\t}\n+\tif (argc > 1)\n+\t\tusage(builtin_diff_usage);\n+\n \tstuff_change(&revs->diffopt,\n \t\t     mode, mode,\n \t\t     blob[1].sha1, blob[0].sha1,\n@@ -162,8 +150,6 @@ static int builtin_diff_index(struct rev\n \t\tconst char *arg = argv[1];\n \t\tif (!strcmp(arg, \"--cached\"))\n \t\t\tcached = 1;\n-\t\telse if (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n \t\telse\n \t\t\tusage(builtin_diff_usage);\n \t\targv++; argc--;\n@@ -185,14 +171,9 @@ static int builtin_diff_tree(struct rev_\n {\n \tconst unsigned char *(sha1[2]);\n \tint swap = 0;\n-\twhile (1 < argc) {\n-\t\tconst char *arg = argv[1];\n-\t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n-\t\telse\n-\t\t\tusage(builtin_diff_usage);\n-\t\targv++; argc--;\n-\t}\n+\n+\tif (argc > 1)\n+\t\tusage(builtin_diff_usage);\n \n \t/* We saw two trees, ent[0] and ent[1].\n \t * if ent[1] is unintesting, they are swapped\n@@ -214,14 +195,9 @@ static int builtin_diff_combined(struct \n \tconst unsigned char (*parent)[20];\n \tint i;\n \n-\twhile (1 < argc) {\n-\t\tconst char *arg = argv[1];\n-\t\tif (!strcmp(arg, \"--raw\"))\n-\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n-\t\telse\n-\t\t\tusage(builtin_diff_usage);\n-\t\targv++; argc--;\n-\t}\n+\tif (argc > 1)\n+\t\tusage(builtin_diff_usage);\n+\n \tif (!revs->dense_combined_merges && !revs->combine_merges)\n \t\trevs->dense_combined_merges = revs->combine_merges = 1;\n \tparent = xmalloc(ents * sizeof(*parent));\ndiff --git a/diff.c b/diff.c\nindex 45c93c9..ab8aed7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1464,6 +1464,8 @@ int diff_opt_parse(struct diff_options *\n \t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n \telse if (opt_arg(arg, 'U', \"unified\", &options->context))\n \t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n+\telse if (!strcmp(arg, \"--raw\"))\n+\t\toptions->output_fmt |= OUTPUT_FMT_RAW;\n \telse if (!strcmp(arg, \"--patch-with-raw\")) {\n \t\toptions->output_fmt |= OUTPUT_FMT_PATCH | OUTPUT_FMT_RAW;\n \t}\n-- \n1.4.1.rc1.gf603-dirty\n"},{"id":"22381","messageId":"20060624010156.d357164d.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624003315.804a1796.tihirvon@gmail.com","subject":"[PATCH 5/5] Add --patch option for diff-*","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T22:01:56Z","receivedAt":"2006-06-23T22:01:56Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n diff.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ab8aed7..596a877 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1460,7 +1460,7 @@ static int opt_arg(const char *arg, int \n int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n {\n \tconst char *arg = av[0];\n-\tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\"))\n+\tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\"))\n \t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n \telse if (opt_arg(arg, 'U', \"unified\", &options->context))\n \t\toptions->output_fmt |= OUTPUT_FMT_PATCH;\n-- \n1.4.1.rc1.gf603-dirty\n"},{"id":"22383","messageId":"20060624014021.ebce199e.tihirvon@gmail.com","threadId":"4643","inReplyTo":"20060624005252.c694e421.tihirvon@gmail.com","subject":"Re: [PATCH 2/5] Rework diff options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-23T22:40:21Z","receivedAt":"2006-06-23T22:40:21Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Timo Hirvonen <tihirvon@gmail.com> wrote:\n\n> @@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c\n>  \t\t\tnum_paths++;\n>  \t}\n>  \tif (num_paths) {\n> -\t\tif (opt->with_raw) {\n> -\t\t\tint saved_format = opt->output_format;\n> -\t\t\topt->output_format = DIFF_FORMAT_RAW;\n> +\t\tif (opt->output_fmt & OUTPUT_FMT_RAW) {\n> +\t\t\tint saved_fmt = opt->output_fmt;\n> +\t\t\topt->output_fmt |= OUTPUT_FMT_RAW;\n\nI have no idea if this is more right than this:\n\n                        opt->output_fmt = OUTPUT_FMT_RAW;\n\n> @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char\n>  \tif (revs->combine_merges) {\n>  \t\trevs->ignore_merges = 0;\n>  \t\tif (revs->dense_combined_merges &&\n> -\t\t    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))\n> -\t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n> +\t\t   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))\n> +\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;\n\nI'm not sure about this. Didn't really understand the code :)\n\n-- \nhttp://onion.dynserv.net/~timo/\n"},{"id":"22421","messageId":"7vodwj11qa.fsf@assigned-by-dhcp.cox.net","threadId":"4643","inReplyTo":"20060624005252.c694e421.tihirvon@gmail.com","subject":"Re: [PATCH 2/5] Rework diff options","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-24T06:55:57Z","receivedAt":"2006-06-24T06:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Timo Hirvonen <tihirvon@gmail.com> writes:\n\n> diff --git a/builtin-diff.c b/builtin-diff.c\n> index 99a2f76..372894a 100644\n> --- a/builtin-diff.c\n> +++ b/builtin-diff.c\n> @@ -56,15 +56,15 @@ static int builtin_diff_files(struct rev\n>  \t    3 < revs->max_count)\n>  \t\tusage(builtin_diff_usage);\n>  \tif (revs->max_count < 0 &&\n> -\t    (revs->diffopt.output_format == DIFF_FORMAT_PATCH))\n> +\t    (revs->diffopt.output_fmt & OUTPUT_FMT_PATCH))\n>  \t\trevs->combine_merges = revs->dense_combined_merges = 1;\n>  \t/*\n>  \t * Backward compatibility wart - \"diff-files -s\" used to\n>  \t * defeat the common diff option \"-s\" which asked for\n> -\t * DIFF_FORMAT_NO_OUTPUT.\n> +\t * OUTPUT_FMT_NONE\n>  \t */\n> -\tif (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)\n> -\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n> +\tif (revs->diffopt.output_fmt & OUTPUT_FMT_NONE)\n> +\t\trevs->diffopt.output_fmt = OUTPUT_FMT_RAW;\n>  \treturn run_diff_files(revs, silent);\n>  }\n\nWe do not have to remove this now, but I think we can remove\nthis \"backward compatibility wart\" sometime in the next round;\nCogito has been fixed not to use this.\n\n> @@ -110,7 +110,7 @@ static int builtin_diff_b_f(struct rev_i\n>  \twhile (1 < argc) {\n>  \t\tconst char *arg = argv[1];\n>  \t\tif (!strcmp(arg, \"--raw\"))\n> -\t\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n> +\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_RAW;\n>  \t\telse\n>  \t\t\tusage(builtin_diff_usage);\n>  \t\targv++; argc--;\n\nI see later in the series you taught low-level diff_opt_parse\nabout --raw switch, which is good.\n\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 5a8a50b..e4a6385 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -26,8 +26,8 @@ static int cmd_log_wc(int argc, const ch\n>  \tif (rev->always_show_header) {\n>  \t\tif (rev->diffopt.pickaxe || rev->diffopt.filter) {\n>  \t\t\trev->always_show_header = 0;\n> -\t\t\tif (rev->diffopt.output_format == DIFF_FORMAT_RAW)\n> -\t\t\t\trev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n> +\t\t\tif (rev->diffopt.output_fmt & OUTPUT_FMT_RAW)\n> +\t\t\t\trev->diffopt.output_fmt |= OUTPUT_FMT_NONE;\n>  \t\t}\n>  \t}\n\nThe original code is saying \"For git-log command (i.e. when\nalways-show-header is on), if the command line did not override\nbut ended up asking for diff only because it wanted to do -S or\n--diff-filter, do not show any diff\" which is quite an opaque\nlogic.\n\n> diff --git a/combine-diff.c b/combine-diff.c\n> index 64b20cc..d0d8d01 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> @@ -878,13 +867,13 @@ void diff_tree_combined(const unsigned c\n>  \t\t\tnum_paths++;\n>  \t}\n>  \tif (num_paths) {\n> -\t\tif (opt->with_raw) {\n> -\t\t\tint saved_format = opt->output_format;\n> -\t\t\topt->output_format = DIFF_FORMAT_RAW;\n> +\t\tif (opt->output_fmt & OUTPUT_FMT_RAW) {\n> +\t\t\tint saved_fmt = opt->output_fmt;\n> +\t\t\topt->output_fmt |= OUTPUT_FMT_RAW;\n>  \t\t\tfor (p = paths; p; p = p->next) {\n>  \t\t\t\tshow_combined_diff(p, num_parent, dense, rev);\n>  \t\t\t}\n> -\t\t\topt->output_format = saved_format;\n> +\t\t\topt->output_fmt = saved_fmt;\n>  \t\t\tputchar(opt->line_termination);\n>  \t\t}\n>  \t\tfor (p = paths; p; p = p->next) {\n\nThe code needs to run show_combined_diff twice, once for raw\noutput and another for normal output, since the processing\nneeded to be done in show_combined_diff are quite different\nbetween the case where you emit raw diff (in which case you do\nnot combine at the hunk level) and normal diff (in which case\nyou do).  Since your modified show_combined_diff checks FMT_RAW\nfirst, I suspect it would not make any difference if it is ORing\nOUTPUT_FMT_RAW into output_fmt or assignment.  In fact, since\nthe bit is already on when the if () statement is taken, I think\nyou can lose the whole saved_fmt business, and just say\nsomething like this here:\n\n\t\tif (opt->output_fmt & OUTPUT_FMT_RAW) {\n\t\t\tfor (p = paths; p; p = p->next) {\n\t\t\t\tshow_combined_diff(p, num_parent, dense, rev);\n\t\t\t}\n\t\t\tputchar(opt->line_termination);\n\t\t}\n\nBut I think the other side needs to drop OUTPUT_FMT_RAW bit,\nsince around ll. 817 in combine-diff.c you show_raw_diff() if\nOUTPUT_FMT_RAW bit is on regardless of the OUTPUT_FMT_PATCH bit.\n\n> diff --git a/diff.c b/diff.c\n> index 1db0285..6eb7db0 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -203,7 +203,7 @@ static void emit_rewrite_diff(const char\n>  static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)\n>  {\n>  \tif (!DIFF_FILE_VALID(one)) {\n> -\t\tmf->ptr = \"\"; /* does not matter */\n> +\t\tmf->ptr = (char *)\"\"; /* does not matter */\n>  \t\tmf->size = 0;\n>  \t\treturn 0;\n>  \t}\n> @@ -395,7 +395,7 @@ static void show_stats(struct diffstat_t\n>  \t}\n>  \n>  \tfor (i = 0; i < data->nr; i++) {\n> -\t\tchar *prefix = \"\";\n> +\t\tconst char *prefix = \"\";\n>  \t\tchar *name = data->files[i]->name;\n>  \t\tint added = data->files[i]->added;\n>  \t\tint deleted = data->files[i]->deleted;\n> @@ -917,7 +917,7 @@ int diff_populate_filespec(struct diff_f\n>  \t\t\terr_empty:\n>  \t\t\t\terr = -1;\n>  \t\t\tempty:\n> -\t\t\t\ts->data = \"\";\n> +\t\t\t\ts->data = (char *)\"\";\n>  \t\t\t\ts->size = 0;\n>  \t\t\t\treturn err;\n>  \t\t\t}\n> @@ -1408,7 +1411,7 @@ int diff_setup_done(struct diff_options \n>  \treturn 0;\n>  }\n>  \n> -int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n> +static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n>  {\n>  \tchar c, *eq;\n>  \tint len;\n> @@ -1720,16 +1722,12 @@ static void diff_flush_raw(struct diff_f\n>  \t\tfree((void*)path_two);\n>  }\n>  \n> -static void diff_flush_name(struct diff_filepair *p,\n> -\t\t\t    int inter_name_termination,\n> -\t\t\t    int line_termination)\n> +static void diff_flush_name(struct diff_filepair *p, int line_termination)\n>  {\n>  \tchar *path = p->two->path;\n>  \n>  \tif (line_termination)\n>  \t\tpath = quote_one(p->two->path);\n> -\telse\n> -\t\tpath = p->two->path;\n>  \tprintf(\"%s%c\", path, line_termination);\n>  \tif (p->two->path != path)\n>  \t\tfree(path);\n\nThese are all good changes but I would have liked to see them as\na separate series.\n\n> @@ -1371,23 +1371,26 @@ int diff_setup_done(struct diff_options \n>  \t    (0 <= options->rename_limit && !options->detect_rename))\n>  \t\treturn -1;\n>  \n> +\tif (options->output_fmt & OUTPUT_FMT_NONE)\n> +\t\toptions->output_fmt = 0;\n> +\n> +\tif (options->output_fmt & (OUTPUT_FMT_NAME |\n> +\t\t\t\t   OUTPUT_FMT_CHECKDIFF |\n> +\t\t\t\t   OUTPUT_FMT_NONE))\n> +\t\toptions->output_fmt &= ~(OUTPUT_FMT_RAW |\n> +\t\t\t\t\t OUTPUT_FMT_DIFFSTAT |\n> +\t\t\t\t\t OUTPUT_FMT_SUMMARY |\n> +\t\t\t\t\t OUTPUT_FMT_PATCH);\n> +\n\nMaybe doing the same for --name-status?  I wonder if the --name,\n--name-status and --check should be mutually exclusive.  What\nhappens when you specify more than one of them?\n\n> +/* Same as output_fmt = 0 but we know that -s flag was given\n> + * and we should not give default value to output_fmt.\n> + */\n> +#define OUTPUT_FMT_NONE\t\t0x0080\n\nThis is a very good comment to tell the reader what is going on.\nI appreciate it.\n\n> diff --git a/revision.c b/revision.c\n> index b963f2a..4ad2272 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char\n>  \tif (revs->combine_merges) {\n>  \t\trevs->ignore_merges = 0;\n>  \t\tif (revs->dense_combined_merges &&\n> -\t\t    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))\n> -\t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n> +\t\t   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))\n> +\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;\n>  \t}\n>  \trevs->diffopt.abbrev = revs->abbrev;\n>  \tdiff_setup_done(&revs->diffopt);\n\nThis tells it to default to patch format unless we are asked to\ndo diffstat only, in which case we just show stat without patch.\nThe new logic seems to be fishy.\n\nWhew, now it's quite a lot of changes.  How to proceed from\nhere?  My rather selfish preference is:\n\n - \"git-merge not add -p when asking for --summary --stat\" is an\n   obvious fix that is independently applicable to \"master\", so\n   I took it already.\n\n - could I ask you to redo a patch to do only the clean-up part\n   first, so that I can accept it for either \"next\" or \"master\".\n\n - Then after I take the clean-up, could you rebase four\n   remainder patches (\"Rework diff options\" to \"Add --patch\n   option for diff-*\") on the result?  The patches this round\n   are already split quite well in that the first one does the\n   enum to bit conversion and the latter three cleans things up\n   (all of which I like a lot).  As Johannes suggested, it might\n   be easier to review if they reused the same preprocessor\n   symbols instead of renaming them.  I'd take them for \"next\".\n"},{"id":"22440","messageId":"20060624142943.2aeda5d2.tihirvon@gmail.com","threadId":"4643","inReplyTo":"7vodwj11qa.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/5] Rework diff options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T11:29:43Z","receivedAt":"2006-06-24T11:29:43Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Junio C Hamano <junkio@cox.net> wrote:\n\n> > diff --git a/builtin-log.c b/builtin-log.c\n> > index 5a8a50b..e4a6385 100644\n> > --- a/builtin-log.c\n> > +++ b/builtin-log.c\n> > @@ -26,8 +26,8 @@ static int cmd_log_wc(int argc, const ch\n> >  \tif (rev->always_show_header) {\n> >  \t\tif (rev->diffopt.pickaxe || rev->diffopt.filter) {\n> >  \t\t\trev->always_show_header = 0;\n> > -\t\t\tif (rev->diffopt.output_format == DIFF_FORMAT_RAW)\n> > -\t\t\t\trev->diffopt.output_format = DIFF_FORMAT_NO_OUTPUT;\n> > +\t\t\tif (rev->diffopt.output_fmt & OUTPUT_FMT_RAW)\n> > +\t\t\t\trev->diffopt.output_fmt |= OUTPUT_FMT_NONE;\n> >  \t\t}\n> >  \t}\n> \n> The original code is saying \"For git-log command (i.e. when\n> always-show-header is on), if the command line did not override\n> but ended up asking for diff only because it wanted to do -S or\n> --diff-filter, do not show any diff\" which is quite an opaque\n> logic.\n\nI'll just remove this change from the fixed patch 2/5.  New version of\nthe patch 3/5 should then fix this logic.\n\n> > @@ -1371,23 +1371,26 @@ int diff_setup_done(struct diff_options \n> >  \t    (0 <= options->rename_limit && !options->detect_rename))\n> >  \t\treturn -1;\n> >  \n> > +\tif (options->output_fmt & OUTPUT_FMT_NONE)\n> > +\t\toptions->output_fmt = 0;\n> > +\n> > +\tif (options->output_fmt & (OUTPUT_FMT_NAME |\n> > +\t\t\t\t   OUTPUT_FMT_CHECKDIFF |\n> > +\t\t\t\t   OUTPUT_FMT_NONE))\n> > +\t\toptions->output_fmt &= ~(OUTPUT_FMT_RAW |\n> > +\t\t\t\t\t OUTPUT_FMT_DIFFSTAT |\n> > +\t\t\t\t\t OUTPUT_FMT_SUMMARY |\n> > +\t\t\t\t\t OUTPUT_FMT_PATCH);\n> > +\n> \n> Maybe doing the same for --name-status?\n\nWill fix.  Originally I made --name-status imply --name-only but changed\nit and forgot to fix this.\n\n> I wonder if the --name,\n> --name-status and --check should be mutually exclusive.  What\n> happens when you specify more than one of them?\n\nI'll just make it die() then. If it breaks something then the code is\nreally dumb anyway.\n\n> > diff --git a/revision.c b/revision.c\n> > index b963f2a..4ad2272 100644\n> > --- a/revision.c\n> > +++ b/revision.c\n> > @@ -852,8 +852,8 @@ int setup_revisions(int argc, const char\n> >  \tif (revs->combine_merges) {\n> >  \t\trevs->ignore_merges = 0;\n> >  \t\tif (revs->dense_combined_merges &&\n> > -\t\t    (revs->diffopt.output_format != DIFF_FORMAT_DIFFSTAT))\n> > -\t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n> > +\t\t   !(revs->diffopt.output_fmt & OUTPUT_FMT_DIFFSTAT))\n> > +\t\t\trevs->diffopt.output_fmt |= OUTPUT_FMT_PATCH;\n> >  \t}\n> >  \trevs->diffopt.abbrev = revs->abbrev;\n> >  \tdiff_setup_done(&revs->diffopt);\n> \n> This tells it to default to patch format unless we are asked to\n> do diffstat only, in which case we just show stat without patch.\n> The new logic seems to be fishy.\n\nIf we first initialize it to 0 instead of DIFF_FORMAT_RAW and after\ncommand line flags have been parsed, if it still is 0, then default to\nDIFF_FORMAT_PATCH.\n\n>  - could I ask you to redo a patch to do only the clean-up part\n>    first, so that I can accept it for either \"next\" or \"master\".\n> \n>  - Then after I take the clean-up, could you rebase four\n>    remainder patches (\"Rework diff options\" to \"Add --patch\n>    option for diff-*\") on the result?  The patches this round\n>    are already split quite well in that the first one does the\n>    enum to bit conversion and the latter three cleans things up\n>    (all of which I like a lot).  As Johannes suggested, it might\n>    be easier to review if they reused the same preprocessor\n>    symbols instead of renaming them.  I'd take them for \"next\".\n\nYes, all this makes sense.\n\n-- \nhttp://onion.dynserv.net/~timo/\n"}]}