{"thread":{"id":"4656","subject":"[PATCH 3/7] Make --raw option available for all diff commands","startedAt":"2006-06-24T17:18:43Z","lastAt":"2006-06-26T18:24:17Z","messageCount":20,"participants":["Timo Hirvonen","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"22464","messageId":"20060624201843.a5b4f7b9.tihirvon@gmail.com","threadId":"4656","inReplyTo":null,"subject":"[PATCH 0/7] Rework diff options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:18:43Z","receivedAt":"2006-06-24T17:18:43Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"This patch series cleans up diff output format options.\n\nThis makes it possible to use any combination of --raw, -p, --stat and\n--summary options and they work as you would expect.\n\nThese patches passed all test and are for the next branch. Patches 6 and\n7 are optional.\n\n b/builtin-diff-files.c  |   10 --\n b/builtin-diff-index.c  |    3 \n b/builtin-diff-stages.c |    3 \n b/builtin-diff-tree.c   |    3 \n b/builtin-diff.c        |   62 +++----------\n b/builtin-log.c         |   13 +-\n b/combine-diff.c        |   55 +++++------\n b/diff.c                |  221 +++++++++++++++++++++++-------------------------\n b/diff.h                |   27 +++--\n b/log-tree.c            |   10 +-\n b/revision.c            |    4 \n 11 files changed, 189 insertions(+), 222 deletions(-)\n\n-- \nhttp://onion.dynserv.net/~timo/\n"},{"id":"22462","messageId":"20060624202032.ec7203d8.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 1/7] Clean up diff.c","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:20:32Z","receivedAt":"2006-06-24T17:20:32Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n diff.c |   18 ++++++------------\n 1 files changed, 6 insertions(+), 12 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ad77543..f358546 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@@ -918,7 +918,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@@ -1409,7 +1409,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@@ -1725,16 +1725,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@@ -1955,9 +1951,7 @@ static void flush_one_pair(struct diff_f\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\tdiff_flush_name(p, line_termination);\n \t\t\tbreak;\n \t\tcase DIFF_FORMAT_NO_OUTPUT:\n \t\t\tbreak;\n-- \n1.4.1.rc1.g8637\n"},{"id":"22463","messageId":"20060624202153.1001a66c.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:21:53Z","receivedAt":"2006-06-24T17:21:53Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"DIFF_FORMAT_* are now bit-flags instead of enumerated values.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff.c |    2 -\n builtin-log.c  |    4 -\n combine-diff.c |   55 +++++++----------\n diff.c         |  183 +++++++++++++++++++++++++++-----------------------------\n diff.h         |   27 +++++---\n log-tree.c     |    9 ++-\n 6 files changed, 136 insertions(+), 144 deletions(-)\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 99a2f76..3b44296 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -56,7 +56,7 @@ 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_format & DIFF_FORMAT_PATCH))\n \t\trevs->combine_merges = revs->dense_combined_merges = 1;\n \t/*\n \t * Backward compatibility wart - \"diff-files -s\" used to\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5a8a50b..5c656bc 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\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_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_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..3daa8cb 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_format & DIFF_FORMAT_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_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_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_format & (DIFF_FORMAT_RAW |\n+\t\t\t\t  DIFF_FORMAT_NAME |\n+\t\t\t\t  DIFF_FORMAT_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_format & DIFF_FORMAT_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_format &= ~(DIFF_FORMAT_RAW | DIFF_FORMAT_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_format & DIFF_FORMAT_DIFFSTAT)\n+\t\t\tdiffopts.output_format |= DIFF_FORMAT_DIFFSTAT;\n \t\telse\n-\t\t\tdiffopts.output_format = DIFF_FORMAT_NO_OUTPUT;\n+\t\t\tdiffopts.output_format |= DIFF_FORMAT_NO_OUTPUT;\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_format & DIFF_FORMAT_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_format & DIFF_FORMAT_DIFFSTAT)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -878,17 +867,21 @@ 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_format & (DIFF_FORMAT_RAW |\n+\t\t\t\t\t  DIFF_FORMAT_NAME |\n+\t\t\t\t\t  DIFF_FORMAT_NAME_STATUS)) {\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\tif (p->len)\n+\t\t\t\t\tshow_raw_diff(p, num_parent, rev);\n \t\t\t}\n-\t\t\topt->output_format = saved_format;\n \t\t\tputchar(opt->line_termination);\n \t\t}\n-\t\tfor (p = paths; p; p = p->next) {\n-\t\t\tshow_combined_diff(p, num_parent, dense, rev);\n+\t\tif (opt->output_format & DIFF_FORMAT_PATCH) {\n+\t\t\tfor (p = paths; p; p = p->next) {\n+\t\t\t\tif (p->len)\n+\t\t\t\t\tshow_patch_diff(p, num_parent, dense,\n+\t\t\t\t\t\t\trev);\n+\t\t\t}\n \t\t}\n \t}\n \ndiff --git a/diff.c b/diff.c\nindex f358546..bfed79c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1372,23 +1372,27 @@ 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_format & DIFF_FORMAT_NO_OUTPUT)\n+\t\toptions->output_format = 0;\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\toptions->output_format &= ~(DIFF_FORMAT_RAW |\n+\t\t\t\t\t    DIFF_FORMAT_DIFFSTAT |\n+\t\t\t\t\t    DIFF_FORMAT_SUMMARY |\n+\t\t\t\t\t    DIFF_FORMAT_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_format & (DIFF_FORMAT_PATCH |\n+\t\t\t\t      DIFF_FORMAT_DIFFSTAT |\n+\t\t\t\t      DIFF_FORMAT_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@@ -1460,22 +1464,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_format |= DIFF_FORMAT_PATCH;\n \telse if (opt_arg(arg, 'U', \"unified\", &options->context))\n-\t\toptions->output_format = DIFF_FORMAT_PATCH;\n+\t\toptions->output_format |= DIFF_FORMAT_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_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;\n \t}\n \telse if (!strcmp(arg, \"--stat\"))\n-\t\toptions->output_format = DIFF_FORMAT_DIFFSTAT;\n+\t\toptions->output_format |= DIFF_FORMAT_DIFFSTAT;\n \telse if (!strcmp(arg, \"--check\"))\n-\t\toptions->output_format = DIFF_FORMAT_CHECKDIFF;\n+\t\toptions->output_format |= DIFF_FORMAT_CHECKDIFF;\n \telse if (!strcmp(arg, \"--summary\"))\n-\t\toptions->summary = 1;\n+\t\toptions->output_format |= DIFF_FORMAT_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_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_DIFFSTAT;\n \t}\n \telse if (!strcmp(arg, \"-z\"))\n \t\toptions->line_termination = 0;\n@@ -1484,19 +1486,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_format |= DIFF_FORMAT_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_format |= DIFF_FORMAT_NAME;\n \telse if (!strcmp(arg, \"--name-status\"))\n-\t\toptions->output_format = DIFF_FORMAT_NAME_STATUS;\n+\t\toptions->output_format |= DIFF_FORMAT_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_format |= DIFF_FORMAT_NO_OUTPUT;\n+\t}\n \telse if (!strncmp(arg, \"-O\", 2))\n \t\toptions->orderfile = arg + 2;\n \telse if (!strncmp(arg, \"--diff-filter=\", 14))\n@@ -1671,15 +1674,17 @@ const char *diff_unique_abbrev(const uns\n }\n \n static void diff_flush_raw(struct diff_filepair *p,\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   struct diff_options *options)\n {\n \tint two_paths;\n \tchar status[10];\n \tint abbrev = options->abbrev;\n \tconst char *path_one, *path_two;\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 \n \tpath_one = p->one->path;\n \tpath_two = p->two->path;\n@@ -1708,7 +1713,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 (!(options->output_format & DIFF_FORMAT_NAME_STATUS)) {\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@@ -1917,48 +1922,30 @@ 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, line_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(struct diff_filepair *p, struct diff_options *opt)\n+{\n+\tint fmt = opt->output_format;\n+\n+\tif (fmt & DIFF_FORMAT_CHECKDIFF)\n+\t\tdiff_flush_checkdiff(p, opt);\n+\telse if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))\n+\t\tdiff_flush_raw(p, opt);\n+\telse if (fmt & DIFF_FORMAT_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@@ -2041,55 +2028,61 @@ 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_format = options->output_format;\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_format & (DIFF_FORMAT_RAW |\n+\t\t\t     DIFF_FORMAT_NAME |\n+\t\t\t     DIFF_FORMAT_NAME_STATUS |\n+\t\t\t     DIFF_FORMAT_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_format & DIFF_FORMAT_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\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_format & DIFF_FORMAT_SUMMARY) {\n+\t\tfor (i = 0; i < q->nr; i++)\n+\t\t\tdiff_summary(q->queue[i]);\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-\t\tdiff_free_filepair(q->queue[i]);\n+\tif (output_format & DIFF_FORMAT_PATCH) {\n+\t\tif (output_format & (DIFF_FORMAT_DIFFSTAT |\n+\t\t\t\t     DIFF_FORMAT_SUMMARY)) {\n+\t\t\tif (options->stat_sep)\n+\t\t\t\tfputs(options->stat_sep, stdout);\n+\t\t\telse\n+\t\t\t\tputchar(options->line_termination);\n+\t\t}\n+\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\tdiff_free_filepair(q->queue[i]);\n \tfree(q->queue);\n \tq->queue = NULL;\n \tq->nr = q->alloc = 0;\ndiff --git a/diff.h b/diff.h\nindex b61fdc8..2b6dc0c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -20,19 +20,31 @@ 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 DIFF_FORMAT_RAW\t\t0x0001\n+#define DIFF_FORMAT_DIFFSTAT\t0x0002\n+#define DIFF_FORMAT_SUMMARY\t0x0004\n+#define DIFF_FORMAT_PATCH\t0x0008\n+\n+/* These override all above */\n+#define DIFF_FORMAT_NAME\t0x0010\n+#define DIFF_FORMAT_NAME_STATUS\t0x0020\n+#define DIFF_FORMAT_CHECKDIFF\t0x0040\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\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@@ -151,15 +163,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..7d4c51f 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -163,8 +163,13 @@ int log_tree_diff_flush(struct rev_info \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_format & DIFF_FORMAT_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 }\n-- \n1.4.1.rc1.g8637\n"},{"id":"22459","messageId":"20060624202306.f540ac83.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 3/7] Make --raw option available for all diff commands","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:23:06Z","receivedAt":"2006-06-24T17:23:06Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\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 3b44296..91235a1 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_format = DIFF_FORMAT_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_format = DIFF_FORMAT_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_format = DIFF_FORMAT_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_format = DIFF_FORMAT_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_format = DIFF_FORMAT_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_format = DIFF_FORMAT_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 bfed79c..6e5ae77 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1467,6 +1467,8 @@ int diff_opt_parse(struct diff_options *\n \t\toptions->output_format |= DIFF_FORMAT_PATCH;\n \telse if (opt_arg(arg, 'U', \"unified\", &options->context))\n \t\toptions->output_format |= DIFF_FORMAT_PATCH;\n+\telse if (!strcmp(arg, \"--raw\"))\n+\t\toptions->output_format |= DIFF_FORMAT_RAW;\n \telse if (!strcmp(arg, \"--patch-with-raw\")) {\n \t\toptions->output_format |= DIFF_FORMAT_PATCH | DIFF_FORMAT_RAW;\n \t}\n-- \n1.4.1.rc1.g8637\n"},{"id":"22460","messageId":"20060624202414.d03cec6e.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 4/7] Set default diff output format after parsing command line","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:24:14Z","receivedAt":"2006-06-24T17:24:14Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Initialize output_format to 0 instead of DIFF_FORMAT_RAW so that we can see\nlater if any command line options changed it.  Default value is set only if\noutput format was not specified.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff-files.c  |    3 +++\n builtin-diff-index.c  |    3 +++\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, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex 5afc1d7..a655eea 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -36,6 +36,9 @@ int cmd_diff_files(int argc, const char \n \t\t\tusage(diff_files_usage);\n \t\targv++; argc--;\n \t}\n+\tif (!rev.diffopt.output_format)\n+\t\trev.diffopt.output_format = DIFF_FORMAT_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..b37c9e8 100644\n--- a/builtin-diff-index.c\n+++ b/builtin-diff-index.c\n@@ -28,6 +28,9 @@ int cmd_diff_index(int argc, const char \n \t\telse\n \t\t\tusage(diff_cache_usage);\n \t}\n+\tif (!rev.diffopt.output_format)\n+\t\trev.diffopt.output_format = DIFF_FORMAT_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..30931fe 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_format)\n+\t\tdiff_options.output_format = DIFF_FORMAT_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..ae1cde9 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_format)\n+\t\topt->diffopt.output_format = DIFF_FORMAT_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 91235a1..47e0a37 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -252,9 +252,11 @@ 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 \n \targc = setup_revisions(argc, argv, &rev, NULL);\n+\tif (!rev.diffopt.output_format)\n+\t\trev.diffopt.output_format = DIFF_FORMAT_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 5c656bc..65f9527 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_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_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_format)\n+\t\trev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_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 6e5ae77..6be31e7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1355,7 +1355,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_format = DIFF_FORMAT_RAW;\n \toptions->line_termination = '\\n';\n \toptions->break_opt = -1;\n \toptions->rename_limit = -1;\n-- \n1.4.1.rc1.g8637\n"},{"id":"22465","messageId":"20060624202508.b997e4e5.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 5/7] DIFF_FORMAT_RAW is not default anymore","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:25:08Z","receivedAt":"2006-06-24T17:25:08Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"diff_setup() used to initialize output_format to DIFF_FORMAT_RAW.  Now\nthe default is 0 (no output) so don't compare against DIFF_FORMAT_RAW to\nsee if any diff format command line flags were given.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-log.c |    5 +----\n revision.c    |    3 +--\n 2 files changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 65f9527..f173070 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -24,11 +24,8 @@ static int cmd_log_wc(int argc, const ch\n \trev->verbose_header = 1;\n \targc = setup_revisions(argc, argv, rev, \"HEAD\");\n \tif (rev->always_show_header) {\n-\t\tif (rev->diffopt.pickaxe || rev->diffopt.filter) {\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}\n \t}\n \n \tif (argc > 1)\ndiff --git a/revision.c b/revision.c\nindex b963f2a..ae4ca82 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -851,8 +851,7 @@ int setup_revisions(int argc, const char\n \t}\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\tif (revs->dense_combined_merges && !revs->diffopt.output_format)\n \t\t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n \t}\n \trevs->diffopt.abbrev = revs->abbrev;\n-- \n1.4.1.rc1.g8637\n"},{"id":"22461","messageId":"20060624202649.b2ed7f57.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 6/7] --name-only, --name-status, --check and -s are mutually exclusive","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:26:49Z","receivedAt":"2006-06-24T17:26:49Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n diff.c |   13 +++++++++++++\n 1 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 6be31e7..4b1b4eb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1366,6 +1366,19 @@ void diff_setup(struct diff_options *opt\n \n int diff_setup_done(struct diff_options *options)\n {\n+\tint count = 0;\n+\n+\tif (options->output_format & DIFF_FORMAT_NAME)\n+\t\tcount++;\n+\tif (options->output_format & DIFF_FORMAT_NAME_STATUS)\n+\t\tcount++;\n+\tif (options->output_format & DIFF_FORMAT_CHECKDIFF)\n+\t\tcount++;\n+\tif (options->output_format & DIFF_FORMAT_NO_OUTPUT)\n+\t\tcount++;\n+\tif (count > 1)\n+\t\tdie(\"--name-only, --name-status, --check and -s are mutually exclusive\");\n+\n \tif ((options->find_copies_harder &&\n \t     options->detect_rename != DIFF_DETECT_COPY) ||\n \t    (0 <= options->rename_limit && !options->detect_rename))\n-- \n1.4.1.rc1.g8637\n"},{"id":"22466","messageId":"20060624202842.61901710.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"[PATCH 7/7] Remove awkward compatibility warts","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T17:28:42Z","receivedAt":"2006-06-24T17:28:42Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-diff-files.c |    7 -------\n builtin-diff.c       |    7 -------\n 2 files changed, 0 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-diff-files.c b/builtin-diff-files.c\nindex a655eea..3361898 100644\n--- a/builtin-diff-files.c\n+++ b/builtin-diff-files.c\n@@ -47,12 +47,5 @@ int cmd_diff_files(int argc, const char \n \tif (rev.pending.nr ||\n \t    rev.min_age != -1 || rev.max_age != -1)\n \t\tusage(diff_files_usage);\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 */\n-\tif (rev.diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)\n-\t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \treturn run_diff_files(&rev, silent);\n }\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 47e0a37..076eb09 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -56,13 +56,6 @@ static int builtin_diff_files(struct rev\n \tif (revs->max_count < 0 &&\n \t    (revs->diffopt.output_format & DIFF_FORMAT_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 */\n-\tif (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)\n-\t\trevs->diffopt.output_format = DIFF_FORMAT_RAW;\n \treturn run_diff_files(revs, silent);\n }\n \n-- \n1.4.1.rc1.g8637\n"},{"id":"22480","messageId":"Pine.LNX.4.63.0606242219320.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4656","inReplyTo":"20060624202153.1001a66c.tihirvon@gmail.com","subject":"Re: [PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-24T20:52:25Z","receivedAt":"2006-06-24T20:52:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthank you very much for doing the extra step and using the original \nconstant names. I appreciate that.\n\nOn Sat, 24 Jun 2006, Timo Hirvonen wrote:\n\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_format & (DIFF_FORMAT_RAW |\n> +\t\t\t\t  DIFF_FORMAT_NAME |\n> +\t\t\t\t  DIFF_FORMAT_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_format & DIFF_FORMAT_PATCH) {\n\nNot that it matters, but this \"else\" could go. (Otherwise,  \"--raw -p\" \nwould be the same as \"--raw\", right?)\n\n>  \t\tshow_patch_diff(p, num_parent, dense, rev);\n> -\t\treturn;\n> -\tdefault:\n> -\t\treturn;\n>  \t}\n>  }\n\n> @@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c\n> [...]\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_format & DIFF_FORMAT_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_format & DIFF_FORMAT_DIFFSTAT)\n>  \t\t\tputchar('\\n');\n>  \t}\n\nJust a remark: this hunk actually changes behaviour. \"with_stat\" meant \nthat the stat was prepended before something like a patch, and therefore a \nseparator was needed. If you pass only \"--stat\", the separator will be \nprinted anyway now.\n\n> diff --git a/diff.c b/diff.c\n> index f358546..bfed79c 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1372,23 +1372,27 @@ 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_format & DIFF_FORMAT_NO_OUTPUT)\n> +\t\toptions->output_format = 0;\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\nThe DIFF_FORMAT_NO_OUTPUT here makes no sense (if it was set, you unset it \nabove).\n\n> @@ -1671,15 +1674,17 @@ const char *diff_unique_abbrev(const uns\n> [...]\n>  \n>  static void diff_flush_raw(struct diff_filepair *p,\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   struct diff_options *options)\n>  {\n>  \tint two_paths;\n>  \tchar status[10];\n>  \tint abbrev = options->abbrev;\n>  \tconst char *path_one, *path_two;\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\n<nit type=minor>\n\tThis should be part of patch 1/7.\n</nit>\n\n> @@ -2041,55 +2028,61 @@ static void diff_summary(struct diff_fil\n> [...]\n>  \n> -\tif (options->with_raw) {\n> +\tif (output_format & (DIFF_FORMAT_RAW |\n> +\t\t\t     DIFF_FORMAT_NAME |\n> +\t\t\t     DIFF_FORMAT_NAME_STATUS |\n> +\t\t\t     DIFF_FORMAT_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\nThis is a very nice cleanup.\n\n>  \t}\n> -\tif (options->with_stat) {\n> +\n> +\tif (output_format & DIFF_FORMAT_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\nAgain, very nice.\n\n>  \t\t}\n>  \t\tshow_stats(diffstat);\n>  \t\tfree(diffstat);\n\nWhy not go the full nine yards, and make diffstat not a pointer, but the \nstruct itself? You would avoid calloc()ing and free()ing. (Of course, \ninstead of calloc()ing you have to memset() it to 0.)\n\n> +\tif (output_format & DIFF_FORMAT_PATCH) {\n> +\t\tif (output_format & (DIFF_FORMAT_DIFFSTAT |\n> +\t\t\t\t     DIFF_FORMAT_SUMMARY)) {\n> +\t\t\tif (options->stat_sep)\n> +\t\t\t\tfputs(options->stat_sep, stdout);\n> +\t\t\telse\n> +\t\t\t\tputchar(options->line_termination);\n\nAre we sure we do not want something like\n\n\tif (output_format / DIFF_FORMAT_DIFFSTAT > 1)\n\t\t/* output separator */\n\nafter each format (this example being after the diffstat), the condition \nbeing: if there is still an output format to come, add the separator?\n\nAll in all, I like this patch.\n\nCiao,\nDscho\n"},{"id":"22482","messageId":"20060625005654.627e176b.tihirvon@gmail.com","threadId":"4656","inReplyTo":"Pine.LNX.4.63.0606242219320.29667@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-24T21:56:54Z","receivedAt":"2006-06-24T21:56:54Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n\n> Hi,\n> \n> thank you very much for doing the extra step and using the original \n> constant names. I appreciate that.\n> \n> On Sat, 24 Jun 2006, Timo Hirvonen wrote:\n> \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_format & (DIFF_FORMAT_RAW |\n> > +\t\t\t\t  DIFF_FORMAT_NAME |\n> > +\t\t\t\t  DIFF_FORMAT_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_format & DIFF_FORMAT_PATCH) {\n> \n> Not that it matters, but this \"else\" could go. (Otherwise,  \"--raw -p\" \n> would be the same as \"--raw\", right?)\n\nJust tested, ./git log -p --raw displays both raw and patch.  I think it\nworks because I changed diff_tree_combined() to use show_raw_diff() and\nshow_patch_diff() directly.\n\nIt feels 'wrong' to check flags and then call a function which checks\nthe flags again.  This combined diff stuff is confusing.\n\n> > @@ -856,19 +846,18 @@ void diff_tree_combined(const unsigned c\n> > [...]\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_format & DIFF_FORMAT_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_format & DIFF_FORMAT_DIFFSTAT)\n> >  \t\t\tputchar('\\n');\n> >  \t}\n> \n> Just a remark: this hunk actually changes behaviour. \"with_stat\" meant \n> that the stat was prepended before something like a patch, and therefore a \n> separator was needed. If you pass only \"--stat\", the separator will be \n> printed anyway now.\n\nYou are right, it now prints --- when it should print empty line.\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> \n> <nit type=minor>\n> \tThis should be part of patch 1/7.\n> </nit>\n\nThat clean up was possible only after I made other changes to the code,\nI think.  At least it wasn't obvious when I wrote 1/7.\n\n> >  \t\tshow_stats(diffstat);\n> >  \t\tfree(diffstat);\n> \n> Why not go the full nine yards, and make diffstat not a pointer, but the \n> struct itself? You would avoid calloc()ing and free()ing. (Of course, \n> instead of calloc()ing you have to memset() it to 0.)\n\nI was blind :)\n\n> > +\tif (output_format & DIFF_FORMAT_PATCH) {\n> > +\t\tif (output_format & (DIFF_FORMAT_DIFFSTAT |\n> > +\t\t\t\t     DIFF_FORMAT_SUMMARY)) {\n> > +\t\t\tif (options->stat_sep)\n> > +\t\t\t\tfputs(options->stat_sep, stdout);\n> > +\t\t\telse\n> > +\t\t\t\tputchar(options->line_termination);\n> \n> Are we sure we do not want something like\n> \n> \tif (output_format / DIFF_FORMAT_DIFFSTAT > 1)\n> \t\t/* output separator */\n> \n> after each format (this example being after the diffstat), the condition \n> being: if there is still an output format to come, add the separator?\n\nI'm not sure what you mean.\n\nIt outputs separator between (diffstat and/or summary) and patch.\nThere's no separator between diffstat and summary or raw and diffstat.\nShould there be one?\n\n\nThanks for your comments.  Should I patch the patch or send a fixed one?\nI'm currently too tired to write any code.\n\n-- \nhttp://onion.dynserv.net/~timo/\n"},{"id":"22485","messageId":"Pine.LNX.4.63.0606250113280.29667@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"4656","inReplyTo":"20060625005654.627e176b.tihirvon@gmail.com","subject":"Re: [PATCH 2/7] Merge with_raw, with_stat and summary variables to output_format","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-06-24T23:20:33Z","receivedAt":"2006-06-24T23:20:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jun 2006, Timo Hirvonen wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:\n> \n> > On Sat, 24 Jun 2006, Timo Hirvonen wrote:\n> > \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_format & (DIFF_FORMAT_RAW |\n> > > +\t\t\t\t  DIFF_FORMAT_NAME |\n> > > +\t\t\t\t  DIFF_FORMAT_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_format & DIFF_FORMAT_PATCH) {\n> > \n> > Not that it matters, but this \"else\" could go. (Otherwise,  \"--raw -p\" \n> > would be the same as \"--raw\", right?)\n> \n> Just tested, ./git log -p --raw displays both raw and patch.  I think it\n> works because I changed diff_tree_combined() to use show_raw_diff() and\n> show_patch_diff() directly.\n> \n> It feels 'wrong' to check flags and then call a function which checks\n> the flags again.  This combined diff stuff is confusing.\n\nSorry for not checking the result, but just the patch. I also find this \nbehaviour confusing. Junio?\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> > \n> > <nit type=minor>\n> > \tThis should be part of patch 1/7.\n> > </nit>\n> \n> That clean up was possible only after I made other changes to the code,\n> I think.  At least it wasn't obvious when I wrote 1/7.\n\nIt is just a minor nit, I do not think it is necessary to change the \npatch.\n\n> > >  \t\tshow_stats(diffstat);\n> > >  \t\tfree(diffstat);\n> > \n> > Why not go the full nine yards, and make diffstat not a pointer, but the \n> > struct itself? You would avoid calloc()ing and free()ing. (Of course, \n> > instead of calloc()ing you have to memset() it to 0.)\n> \n> I was blind :)\n\n;-) In my experience, after staring at the code too long, you turn blind. \nThis is why I like a second pair of eyeballs so much.\n\n> > > +\tif (output_format & DIFF_FORMAT_PATCH) {\n> > > +\t\tif (output_format & (DIFF_FORMAT_DIFFSTAT |\n> > > +\t\t\t\t     DIFF_FORMAT_SUMMARY)) {\n> > > +\t\t\tif (options->stat_sep)\n> > > +\t\t\t\tfputs(options->stat_sep, stdout);\n> > > +\t\t\telse\n> > > +\t\t\t\tputchar(options->line_termination);\n> > \n> > Are we sure we do not want something like\n> > \n> > \tif (output_format / DIFF_FORMAT_DIFFSTAT > 1)\n> > \t\t/* output separator */\n> > \n> > after each format (this example being after the diffstat), the condition \n> > being: if there is still an output format to come, add the separator?\n> \n> I'm not sure what you mean.\n> \n> It outputs separator between (diffstat and/or summary) and patch.\n> There's no separator between diffstat and summary or raw and diffstat.\n> Should there be one?\n\nIMHO there should be one.\n\n> Thanks for your comments.  Should I patch the patch or send a fixed one?\n\nI cannot speak for Junio, but I think an additional patch to clean things \nup would be the way to go.\n\nCiao,\nDscho\n"},{"id":"22518","messageId":"7vslltopzg.fsf@assigned-by-dhcp.cox.net","threadId":"4656","inReplyTo":"20060624201843.a5b4f7b9.tihirvon@gmail.com","subject":"Re: [PATCH 0/7] Rework diff options","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-25T03:48:03Z","receivedAt":"2006-06-25T03:48:03Z","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> This patch series cleans up diff output format options.\n>\n> This makes it possible to use any combination of --raw, -p, --stat and\n> --summary options and they work as you would expect.\n>\n> These patches passed all test and are for the next branch. Patches 6 and\n> 7 are optional.\n\nThanks, very nicely done.  Tentatively placed all of them in\n\"pu\"; the first \"clean-up\" is in \"master\".\n\nHere are a few problems I have seen:\n\n - \"git show --stat HEAD\" gives '---' marker as Johannes and you\n   have already discussed (I do not mind this that much though);\n\n - \"--cc\" seems to be quite broken.  \"git show v1.0.0\" nor \"git\n   diff-tree --pretty --cc v1.0.0\" does not give the log\n   message, and gives something quite confused instead.  I think\n   it is showing \"-m -p\" followed by \"--cc\".\n\nWe may find more minor breakages, in addition to these, but I am\nreasonably sure we should be able to fix them in-tree.\n"},{"id":"22542","messageId":"20060625135414.425580d1.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624202153.1001a66c.tihirvon@gmail.com","subject":"[PATCH] Add msg_sep to diff_options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-25T10:54:14Z","receivedAt":"2006-06-25T10:54:14Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Add msg_sep variable to struct diff_options.  msg_sep is printed after\ncommit message.  Default is \"\\n\", format-patch sets it to \"---\\n\".\n\nThis also removes the second argument from show_log() because all\ncallers derived it from the first argument:\n\n    show_log(rev, rev->loginfo, ...\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n\n  I'm not 100% sure if format-patch is the only one wanting \"---\\n\".\n  But I think \"\\n\" should be used for every command that doesn't create\n  patches.\n\n builtin-log.c  |    1 +\n combine-diff.c |    7 ++++---\n diff.c         |    1 +\n diff.h         |    1 +\n log-tree.c     |   15 ++++++---------\n log-tree.h     |    2 +-\n 6 files changed, 14 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex f173070..5b3fadc 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -175,6 +175,7 @@ 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.msg_sep = \"---\\n\";\n \n \tgit_config(git_format_config);\n \trev.extra_headers = extra_headers;\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 3daa8cb..39fb10c 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -701,7 +701,7 @@ static int show_patch_diff(struct combin\n \t\tconst char *abb;\n \n \t\tif (rev->loginfo)\n-\t\t\tshow_log(rev, rev->loginfo, \"\\n\");\n+\t\t\tshow_log(rev, opt->msg_sep);\n \t\tdump_quoted_path(dense ? \"diff --cc \" : \"diff --combined \", elem->path);\n \t\tprintf(\"index \");\n \t\tfor (i = 0; i < num_parent; i++) {\n@@ -769,7 +769,7 @@ static void show_raw_diff(struct combine\n \t\tinter_name_termination = 0;\n \n \tif (rev->loginfo)\n-\t\tshow_log(rev, rev->loginfo, \"\\n\");\n+\t\tshow_log(rev, opt->msg_sep);\n \n \tif (opt->output_format & DIFF_FORMAT_RAW) {\n \t\toffset = strlen(COLONS) - num_parent;\n@@ -855,7 +855,8 @@ void diff_tree_combined(const unsigned c\n \t\tpaths = intersect_paths(paths, i, num_parent);\n \n \t\tif (opt->output_format & DIFF_FORMAT_DIFFSTAT && rev->loginfo)\n-\t\t\tshow_log(rev, rev->loginfo, \"---\\n\");\n+\t\t\tshow_log(rev, opt->msg_sep);\n+\n \t\tdiff_flush(&diffopts);\n \t\tif (opt->output_format & DIFF_FORMAT_DIFFSTAT)\n \t\t\tputchar('\\n');\ndiff --git a/diff.c b/diff.c\nindex 4b1b4eb..cc2af30 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1359,6 +1359,7 @@ void diff_setup(struct diff_options *opt\n \toptions->break_opt = -1;\n \toptions->rename_limit = -1;\n \toptions->context = 3;\n+\toptions->msg_sep = \"\\n\";\n \n \toptions->change = diff_change;\n \toptions->add_remove = diff_addremove;\ndiff --git a/diff.h b/diff.h\nindex 2b6dc0c..729cd02 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -57,6 +57,7 @@ struct diff_options {\n \tint rename_limit;\n \tint setup;\n \tint abbrev;\n+\tconst char *msg_sep;\n \tconst char *stat_sep;\n \tlong xdl_opts;\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 7d4c51f..ab6b682 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -43,9 +43,10 @@ static int append_signoff(char *buf, int\n \treturn at;\n }\n \n-void show_log(struct rev_info *opt, struct log_info *log, const char *sep)\n+void show_log(struct rev_info *opt, const char *sep)\n {\n \tstatic char this_header[16384];\n+\tstruct log_info *log = opt->loginfo;\n \tstruct commit *commit = log->commit, *parent = log->parent;\n \tint abbrev = opt->diffopt.abbrev;\n \tint abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;\n@@ -163,13 +164,9 @@ int log_tree_diff_flush(struct rev_info \n \t\treturn 0;\n \t}\n \n-\tif (opt->loginfo && !opt->no_commit_id) {\n-\t\tif (opt->diffopt.output_format & DIFF_FORMAT_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+\tif (opt->loginfo && !opt->no_commit_id)\n+\t\tshow_log(opt, opt->diffopt.msg_sep);\n+\n \tdiff_flush(&opt->diffopt);\n \treturn 1;\n }\n@@ -266,7 +263,7 @@ int log_tree_commit(struct rev_info *opt\n \tshown = log_tree_diff(opt, commit, &log);\n \tif (!shown && opt->loginfo && opt->always_show_header) {\n \t\tlog.parent = NULL;\n-\t\tshow_log(opt, opt->loginfo, \"\");\n+\t\tshow_log(opt, \"\");\n \t\tshown = 1;\n \t}\n \topt->loginfo = NULL;\ndiff --git a/log-tree.h b/log-tree.h\nindex a26e484..e82b56a 100644\n--- a/log-tree.h\n+++ b/log-tree.h\n@@ -11,6 +11,6 @@ void init_log_tree_opt(struct rev_info *\n int log_tree_diff_flush(struct rev_info *);\n int log_tree_commit(struct rev_info *, struct commit *);\n int log_tree_opt_parse(struct rev_info *, const char **, int);\n-void show_log(struct rev_info *opt, struct log_info *log, const char *sep);\n+void show_log(struct rev_info *opt, const char *sep);\n \n #endif\n-- \n1.4.1.rc1.g35ee-dirty\n"},{"id":"22545","messageId":"20060625141102.b68a7cae.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624202153.1001a66c.tihirvon@gmail.com","subject":"[PATCH] whatchanged: Default to DIFF_FORMAT_RAW","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-25T11:11:02Z","receivedAt":"2006-06-25T11:11:02Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n builtin-log.c |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5b3fadc..8a39770 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -28,6 +28,15 @@ static int cmd_log_wc(int argc, const ch\n \t\t\trev->always_show_header = 0;\n \t}\n \n+\tif (!rev->diffopt.output_format && !rev->simplify_history) {\n+\t\t/* Ugly hack!\n+\t\t *\n+\t\t * rev->simplify_history == 0 -> whatchanged\n+\t\t * Can't do this before setup_revisions()\n+\t\t */\n+\t\trev->diffopt.output_format = DIFF_FORMAT_RAW;\n+\t}\n+\n \tif (argc > 1)\n \t\tdie(\"unrecognized argument: %s\", argv[1]);\n \n-- \n1.4.1.rc1.g6e272-dirty\n"},{"id":"22546","messageId":"20060625141307.7f007881.tihirvon@gmail.com","threadId":"4656","inReplyTo":"7vslltopzg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/7] Rework diff options","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-25T11:13:07Z","receivedAt":"2006-06-25T11:13:07Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Junio C Hamano <junkio@cox.net> wrote:\n\n> Here are a few problems I have seen:\n> \n>  - \"git show --stat HEAD\" gives '---' marker as Johannes and you\n>    have already discussed (I do not mind this that much though);\n\nThe patch I sent as a reply to 2/7 should fix this.\n\n>  - \"--cc\" seems to be quite broken.  \"git show v1.0.0\" nor \"git\n>    diff-tree --pretty --cc v1.0.0\" does not give the log\n>    message, and gives something quite confused instead.  I think\n>    it is showing \"-m -p\" followed by \"--cc\".\n\nSorry about that.  I don't understand the --cc stuff very well but I try\nto fix the bug.\n\n-- \nhttp://onion.dynserv.net/~timo/\n"},{"id":"22547","messageId":"20060625142819.209f3519.tihirvon@gmail.com","threadId":"4656","inReplyTo":"20060624202153.1001a66c.tihirvon@gmail.com","subject":"[PATCH] Don't xcalloc() struct diffstat_t","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-25T11:28:19Z","receivedAt":"2006-06-25T11:28:19Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n diff.c |   11 +++++------\n 1 files changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex cc2af30..8880150 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2062,17 +2062,16 @@ void diff_flush(struct diff_options *opt\n \t}\n \n \tif (output_format & DIFF_FORMAT_DIFFSTAT) {\n-\t\tstruct diffstat_t *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\tmemset(&diffstat, 0, sizeof(struct diffstat_t));\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\tif (check_pair_status(p))\n-\t\t\t\tdiff_flush_stat(p, options, diffstat);\n+\t\t\t\tdiff_flush_stat(p, options, &diffstat);\n \t\t}\n-\t\tshow_stats(diffstat);\n-\t\tfree(diffstat);\n+\t\tshow_stats(&diffstat);\n \t}\n \n \tif (output_format & DIFF_FORMAT_SUMMARY) {\n-- \n1.4.1.rc1.g5472-dirty\n"},{"id":"22549","messageId":"7v8xnlv4xx.fsf@assigned-by-dhcp.cox.net","threadId":"4656","inReplyTo":"20060625135414.425580d1.tihirvon@gmail.com","subject":"Re: [PATCH] Add msg_sep to diff_options","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-25T11:40:42Z","receivedAt":"2006-06-25T11:40:42Z","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> Add msg_sep variable to struct diff_options.  msg_sep is printed after\n> commit message.  Default is \"\\n\", format-patch sets it to \"---\\n\".\n>\n> This also removes the second argument from show_log() because all\n> callers derived it from the first argument:\n>\n>     show_log(rev, rev->loginfo, ...\n\nGood catch.  Thanks.\n\n> Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>\n> ---\n\nI often wonder if the separator should be \"\\n---\\n\" instead when\nI see something like the above, but do not change it yet please.\n\n>   I'm not 100% sure if format-patch is the only one wanting \"---\\n\".\n\ngit log --patch-with-stat should also show \"---\\n\".\n\n>   But I think \"\\n\" should be used for every command that doesn't create\n>   patches.\n\nThis sounds good.\n\nWe probably would want to have an output format testsuite to\ncatch regression.\n"},{"id":"22551","messageId":"7vy7vltppj.fsf@assigned-by-dhcp.cox.net","threadId":"4656","inReplyTo":"20060625141102.b68a7cae.tihirvon@gmail.com","subject":"Re: [PATCH] whatchanged: Default to DIFF_FORMAT_RAW","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-25T11:55:04Z","receivedAt":"2006-06-25T11:55:04Z","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> Signed-off-by: Timo Hirvonen <tihirvon@gmail.com>\n> ---\n>  builtin-log.c |    9 +++++++++\n>  1 files changed, 9 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-log.c b/builtin-log.c\n> index 5b3fadc..8a39770 100644\n> --- a/builtin-log.c\n> +++ b/builtin-log.c\n> @@ -28,6 +28,15 @@ static int cmd_log_wc(int argc, const ch\n>  \t\t\trev->always_show_header = 0;\n>  \t}\n>  \n> +\tif (!rev->diffopt.output_format && !rev->simplify_history) {\n> +\t\t/* Ugly hack!\n> +\t\t *\n> +\t\t * rev->simplify_history == 0 -> whatchanged\n> +\t\t * Can't do this before setup_revisions()\n> +\t\t */\n\nIndeed it is ugly.  Might it be a cleaner option to signal _wc\nfunction what command its caller is, by adding an extra\nparameter (or check argv -- ugh)?\n"},{"id":"22553","messageId":"20060625153935.6b1e485c.tihirvon@gmail.com","threadId":"4656","inReplyTo":"7vy7vltppj.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] whatchanged: Default to DIFF_FORMAT_RAW","fromName":"Timo Hirvonen","fromEmail":"tihirvon@gmail.com","sentAt":"2006-06-25T12:39:35Z","receivedAt":"2006-06-25T12:39:35Z","isPatch":true,"sender":{"key":"tihirvon@gmail.com","avatar":null},"body":"Split cmd_log_wc() to cmd_log_init() and cmd_log_walk() and set default\ndiff output format for whatchanged to DIFF_FORMAT_RAW.\n\nSigned-off-by: Timo Hirvonen <tihirvon@gmail.com>\n---\n\n  Forget the previous patch, this is cleaner.\n\n builtin-log.c |   27 ++++++++++++++++-----------\n 1 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-log.c b/builtin-log.c\nindex 5b3fadc..28cd8bf 100644\n--- a/builtin-log.c\n+++ b/builtin-log.c\n@@ -14,22 +14,22 @@ #include \"builtin.h\"\n /* this is in builtin-diff.c */\n void add_head(struct rev_info *revs);\n \n-static int cmd_log_wc(int argc, const char **argv, char **envp,\n+static void cmd_log_init(int argc, const char **argv, char **envp,\n \t\t      struct rev_info *rev)\n {\n-\tstruct commit *commit;\n-\n \trev->abbrev = DEFAULT_ABBREV;\n \trev->commit_format = CMIT_FMT_DEFAULT;\n \trev->verbose_header = 1;\n \targc = setup_revisions(argc, argv, rev, \"HEAD\");\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}\n-\n+\tif (rev->diffopt.pickaxe || rev->diffopt.filter)\n+\t\trev->always_show_header = 0;\n \tif (argc > 1)\n \t\tdie(\"unrecognized argument: %s\", argv[1]);\n+}\n+\n+static int cmd_log_walk(struct rev_info *rev)\n+{\n+\tstruct commit *commit;\n \n \tprepare_revision_walk(rev);\n \tsetup_pager();\n@@ -51,7 +51,10 @@ int cmd_whatchanged(int argc, const char\n \trev.diff = 1;\n \trev.diffopt.recursive = 1;\n \trev.simplify_history = 0;\n-\treturn cmd_log_wc(argc, argv, envp, &rev);\n+\tcmd_log_init(argc, argv, envp, &rev);\n+\tif (!rev.diffopt.output_format)\n+\t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n+\treturn cmd_log_walk(&rev);\n }\n \n int cmd_show(int argc, const char **argv, char **envp)\n@@ -66,7 +69,8 @@ int cmd_show(int argc, const char **argv\n \trev.always_show_header = 1;\n \trev.ignore_merges = 0;\n \trev.no_walk = 1;\n-\treturn cmd_log_wc(argc, argv, envp, &rev);\n+\tcmd_log_init(argc, argv, envp, &rev);\n+\treturn cmd_log_walk(&rev);\n }\n \n int cmd_log(int argc, const char **argv, char **envp)\n@@ -76,7 +80,8 @@ int cmd_log(int argc, const char **argv,\n \tinit_revisions(&rev);\n \trev.always_show_header = 1;\n \trev.diffopt.recursive = 1;\n-\treturn cmd_log_wc(argc, argv, envp, &rev);\n+\tcmd_log_init(argc, argv, envp, &rev);\n+\treturn cmd_log_walk(&rev);\n }\n \n static int istitlechar(char c)\n-- \n1.4.1.rc1.g39849-dirty\n"},{"id":"22611","messageId":"7v64inixm6.fsf@assigned-by-dhcp.cox.net","threadId":"4656","inReplyTo":"7vslltopzg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 0/7] Rework diff options","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-06-26T18:24:17Z","receivedAt":"2006-06-26T18:24:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Here are a few problems I have seen:\n>\n>  - \"git show --stat HEAD\" gives '---' marker as Johannes and you\n>    have already discussed (I do not mind this that much though);\n>\n>  - \"--cc\" seems to be quite broken.  \"git show v1.0.0\" nor \"git\n>    diff-tree --pretty --cc v1.0.0\" does not give the log\n>    message, and gives something quite confused instead.  I think\n>    it is showing \"-m -p\" followed by \"--cc\".\n>\n> We may find more minor breakages, in addition to these, but I am\n> reasonably sure we should be able to fix them in-tree.\n\nFurther impressions, while with a clean index and working tree.\n\nFirst the good ones (improvements).\n\n - \"git diff-index --patch-with-raw HEAD\" gives empty result;\n   the traditional one shows one empty line.\n\n - \"git diff-tree -p --stat\" and \"git diff-tree --stat -p\"\n   works, as you planned.\n\n - \"git diff-tree --root --patch-with-raw --summary\" works; the\n   traditional one misses --summary.\n\n - \"git show --name-only HEAD\" works; the traditional one always\n   does --cc -p; the same for \"git show -s HEAD\".\n\nRegressions, most of the minor.\n\n - \"git diff-index -p --stat HEAD\" gives one empty line; the\n   traditional one gives empty.\n\n - \"git diff-tree --patch-with-raw HEAD\" for a non-merge commit\n   misses the empty line between raw and patch.\n\n - \"git diff-tree --cc HEAD\" for an evil merge (a merge whose\n   result does not match either parents, e.g. v1.0.0) shows extra\n   two-tree diffs (presumably HEAD^1..HEAD and HEAD^2..HEAD)\n   before showing what is expected.  The same for \"git show\". \n\n - \"git show --name-only HEAD\" for an evil merge similarly shows\n   extra two-tree diffs in --name-only format before showing\n   what is expected.  Presumably the same bug as the above.\n\n - \"git diff-tree -c HEAD\" for an evil merge shows extra newline\n   after the output.\n\n - Neither \"git diff-tree -m -s HEAD\" for a merge, \"git diff-tree -s\n   HEAD\" for a non-merge does not squelch the output; same for\n   \"git whatchanged\".\n\n - \"git log --raw HEAD\" descends into subdirectories.  It\n   instead should show the top-level tree differences.\n\n - \"git diff-tree --pretty --patch-with-stat HEAD\" for a\n   non-merge misses \"---\\n\" before stat (I think you are aware\n   of this).\n\n - \"git show --cc HEAD\" for a merge should do \"---\\n\", followed\n   by a stat for diff between HEAD^1..HEAD, followed by dense\n   combined-diff for HEAD.\n"}]}