{"thread":{"id":"31716","subject":"[PATCH 1/5] Documentation: move diff.wordRegex from config.txt to diff-config.txt","startedAt":"2012-10-02T16:51:05Z","lastAt":"2012-10-30T21:26:07Z","messageCount":14,"participants":["Ramkumar Ramachandra","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"200339","messageId":"1349196670-2844-1-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":null,"subject":"[PATCH 0/5] submodule: introduce diff.submoduleFormat","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:05Z","receivedAt":"2012-10-02T16:51:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nThe fourth patch is the crux of the series.  The first two patches\nclear up config.txt, diff-config.txt for the fourth patch.  The third\npatch turned out to be a prerequisite.  And the fifth patch is a\nbonus.\n\nThanks for reading.\n\nRam\n\nRamkumar Ramachandra (5):\n  Documentation: move diff.wordRegex from config.txt to diff-config.txt\n  Documentation: sort diff-config.txt alphabetically\n  diff: acknowledge --submodule=short command-line option\n  diff: introduce diff.submoduleFormat configuration variable\n  submodule: display summary header in bold\n\n Documentation/config.txt         |    6 ------\n Documentation/diff-config.txt    |   21 +++++++++++++++++----\n Documentation/diff-options.txt   |    3 ++-\n diff.c                           |   29 +++++++++++++++++++----------\n diff.h                           |   17 +++++++++--------\n submodule.c                      |    8 ++++----\n submodule.h                      |    2 +-\n t/t4041-diff-submodule-option.sh |   10 ++++++++++\n 8 files changed, 62 insertions(+), 34 deletions(-)\n\n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200337","messageId":"1349196670-2844-2-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":"1349196670-2844-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/5] Documentation: move diff.wordRegex from config.txt to diff-config.txt","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:06Z","receivedAt":"2012-10-02T16:51:06Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"19299a8 (Documentation: Move diff.<driver>.* from config.txt to\ndiff-config.txt, 2011-04-07) moved the diff configuration options to\ndiff-config.txt, but forgot about diff.wordRegex, which was left\nbehind in config.txt.  Fix this.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/config.txt      |    6 ------\n Documentation/diff-config.txt |    6 ++++++\n 2 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 11f320b..d1de857 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -962,12 +962,6 @@ difftool.<tool>.cmd::\n difftool.prompt::\n \tPrompt before each invocation of the diff tool.\n \n-diff.wordRegex::\n-\tA POSIX Extended Regular Expression used to determine what is a \"word\"\n-\twhen performing word-by-word difference calculations.  Character\n-\tsequences that match the regular expression are \"words\", all other\n-\tcharacters are *ignorable* whitespace.\n-\n fetch.recurseSubmodules::\n \tThis option can be either set to a boolean value or to 'on-demand'.\n \tSetting it to a boolean changes the behavior of fetch and pull to\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 67a90a8..c2b94f9 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -103,6 +103,12 @@ diff.suppressBlankEmpty::\n \tA boolean to inhibit the standard behavior of printing a space\n \tbefore each empty output line. Defaults to false.\n \n+diff.wordRegex::\n+\tA POSIX Extended Regular Expression used to determine what is a \"word\"\n+\twhen performing word-by-word difference calculations.  Character\n+\tsequences that match the regular expression are \"words\", all other\n+\tcharacters are *ignorable* whitespace.\n+\n diff.<driver>.command::\n \tThe custom diff driver command.  See linkgit:gitattributes[5]\n \tfor details.\n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200342","messageId":"1349196670-2844-3-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":"1349196670-2844-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/5] Documentation: sort diff-config.txt alphabetically","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:07Z","receivedAt":"2012-10-02T16:51:07Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"df44483a (diff --stat: add config option to limit graph width,\n2012-03-01) added the option diff.statGraphWidth, but did not place it\nin its appropriate lexical place in diff-config.txt.  Fix this.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/diff-config.txt |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex c2b94f9..04574f9 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -52,10 +52,6 @@ directories with less than 10% of the total amount of changed files,\n and accumulating child directory counts in the parent directories:\n `files,10,cumulative`.\n \n-diff.statGraphWidth::\n-\tLimit the width of the graph part in --stat output. If set, applies\n-\tto all commands generating --stat output except format-patch.\n-\n diff.external::\n \tIf this config variable is set, diff generation is not\n \tperformed using the internal diff machinery, but using the\n@@ -99,6 +95,10 @@ diff.renames::\n \twill enable basic rename detection.  If set to \"copies\" or\n \t\"copy\", it will detect copies, as well.\n \n+diff.statGraphWidth::\n+\tLimit the width of the graph part in --stat output. If set, applies\n+\tto all commands generating --stat output except format-patch.\n+\n diff.suppressBlankEmpty::\n \tA boolean to inhibit the standard behavior of printing a space\n \tbefore each empty output line. Defaults to false.\n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200338","messageId":"1349196670-2844-4-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":"1349196670-2844-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/5] diff: acknowledge --submodule=short command-line option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:08Z","receivedAt":"2012-10-02T16:51:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, the diff code does not differentiate between an explicit\n'--submodule=short' being passed, and no submodule option being passed\non the command line.  Making this differentiation will be important\nwhen the command-line option can be used to override a\n\"diff.submoduleFormat\" configuration variable introduced in the next\npatch.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n diff.c |    4 +++-\n diff.h |   17 +++++++++--------\n 2 files changed, 12 insertions(+), 9 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 35d3f07..8ea40f9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3647,7 +3647,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t} else if (!strcmp(arg, \"--submodule\"))\n \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n \telse if (!prefixcmp(arg, \"--submodule=\")) {\n-\t\tif (!strcmp(arg + 12, \"log\"))\n+\t\tif (!strcmp(arg + 12, \"short\"))\n+\t\t\tDIFF_OPT_SET(options, SUBMODULE_SHORT);\n+\t\telse if (!strcmp(arg + 12, \"log\"))\n \t\t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n \t}\n \ndiff --git a/diff.h b/diff.h\nindex a658f85..4115b49 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -77,14 +77,15 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)\n #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)\n #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)\n-#define DIFF_OPT_SUBMODULE_LOG       (1 << 23)\n-#define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)\n-#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)\n-#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)\n-#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n-#define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n-#define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n-#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n+#define DIFF_OPT_SUBMODULE_SHORT     (1 << 23)\n+#define DIFF_OPT_SUBMODULE_LOG       (1 << 24)\n+#define DIFF_OPT_DIRTY_SUBMODULES    (1 << 25)\n+#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 26)\n+#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 27)\n+#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 28)\n+#define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 29)\n+#define DIFF_OPT_FUNCCONTEXT         (1 << 30)\n+#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 31)\n \n #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200341","messageId":"1349196670-2844-5-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":"1349196670-2844-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:09Z","receivedAt":"2012-10-02T16:51:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Introduce a diff.submoduleFormat configuration variable corresponding\nto the '--submodule' command-line option of 'git diff'.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/diff-config.txt    |    7 +++++++\n Documentation/diff-options.txt   |    3 ++-\n diff.c                           |   25 ++++++++++++++++---------\n t/t4041-diff-submodule-option.sh |   10 ++++++++++\n 4 files changed, 35 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 04574f9..e445cc8 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -103,6 +103,13 @@ diff.suppressBlankEmpty::\n \tA boolean to inhibit the standard behavior of printing a space\n \tbefore each empty output line. Defaults to false.\n \n+diff.submoduleFormat::\n+\tSpecify the format in which differences in submodules are\n+\tshown.  The \"log\" format lists the commits in the range like\n+\tlinkgit:git-submodule[1] `summary` does.  The \"short\" format\n+\tformat just shows the names of the commits at the beginning\n+\tand end of the range.  Defaults to short.\n+\n diff.wordRegex::\n \tA POSIX Extended Regular Expression used to determine what is a \"word\"\n \twhen performing word-by-word difference calculations.  Character\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex cf4b216..034c4e7 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -170,7 +170,8 @@ any of those replacements occurred.\n \tthe commits in the range like linkgit:git-submodule[1] `summary` does.\n \tOmitting the `--submodule` option or specifying `--submodule=short`,\n \tuses the 'short' format. This format just shows the names of the commits\n-\tat the beginning and end of the range.\n+\tat the beginning and end of the range.  Can be tweaked via the\n+\t`diff.submoduleFormat` configuration variable.\n \n --color[=<when>]::\n \tShow colored diff.\ndiff --git a/diff.c b/diff.c\nindex 8ea40f9..4cb8dd2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -28,6 +28,7 @@ static int diff_suppress_blank_empty;\n static int diff_use_color_default = -1;\n static const char *diff_word_regex_cfg;\n static const char *external_diff_cmd_cfg;\n+static const char *submodule_format_cfg;\n int diff_auto_refresh_index = 1;\n static int diff_mnemonic_prefix;\n static int diff_no_prefix;\n@@ -161,6 +162,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\tdiff_stat_graph_width = git_config_int(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"diff.submoduleformat\"))\n+\t\treturn git_config_string(&submodule_format_cfg, var, value);\n \tif (!strcmp(var, \"diff.external\"))\n \t\treturn git_config_string(&external_diff_cmd_cfg, var, value);\n \tif (!strcmp(var, \"diff.wordregex\"))\n@@ -2227,15 +2230,19 @@ static void builtin_diff(const char *name_a,\n \t\tline_prefix = msgbuf->buf;\n \t}\n \n-\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n-\t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n-\t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n-\t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n-\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n-\t\tshow_submodule_summary(o->file, one ? one->path : two->path,\n-\t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n-\t\t\t\tdel, add, reset);\n-\t\treturn;\n+\tif (!DIFF_OPT_TST(o, SUBMODULE_SHORT) &&\n+\t\t((!one->mode || S_ISGITLINK(one->mode)) &&\n+\t\t\t(!two->mode || S_ISGITLINK(two->mode)))) {\n+\t\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) ||\n+\t\t\t(submodule_format_cfg &&\n+\t\t\t\t!strcmp(submodule_format_cfg, \"log\"))) {\n+\t\t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n+\t\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n+\t\t\tshow_submodule_summary(o->file, one ? one->path : two->path,\n+\t\t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n+\t\t\t\t\tdel, add, reset);\n+\t\t\treturn;\n+\t\t}\n \t}\n \n \tif (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {\ndiff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\nindex 6c01d0c..ed4f3a8 100755\n--- a/t/t4041-diff-submodule-option.sh\n+++ b/t/t4041-diff-submodule-option.sh\n@@ -43,6 +43,16 @@ EOF\n \ttest_cmp expected actual\n \"\n \n+test_expect_success 'added submodule, set diff.submoduleFormat' \"\n+\tgit config diff.submoduleFormat log &&\n+\tgit add sm1 &&\n+\tgit diff --cached >actual &&\n+\tcat >expected <<-EOF &&\n+Submodule sm1 0000000...$head1 (new submodule)\n+EOF\n+\ttest_cmp expected actual\n+\"\n+\n commit_file sm1 &&\n head2=$(add_file sm1 foo3)\n \n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200340","messageId":"1349196670-2844-6-git-send-email-artagnon@gmail.com","threadId":"31716","inReplyTo":"1349196670-2844-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/5] submodule: display summary header in bold","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-02T16:51:10Z","receivedAt":"2012-10-02T16:51:10Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, 'git diff --submodule' displays output with a bold diff\nheader for non-submodules.  So this part is in bold:\n\n    diff --git a/file1 b/file1\n    index 30b2f6c..2638038 100644\n    --- a/file1\n    +++ b/file1\n\nFor submodules, the header looks like this:\n\n    Submodule submodule1 012b072..248d0fd:\n\nUnfortunately, it's easy to miss in the output because it's not bold.\nChange this.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n diff.c      |    2 +-\n submodule.c |    8 ++++----\n submodule.h |    2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4cb8dd2..cf369c5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2240,7 +2240,7 @@ static void builtin_diff(const char *name_a,\n \t\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n \t\t\tshow_submodule_summary(o->file, one ? one->path : two->path,\n \t\t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n-\t\t\t\t\tdel, add, reset);\n+\t\t\t\t\tset, del, add, reset);\n \t\t\treturn;\n \t\t}\n \t}\ndiff --git a/submodule.c b/submodule.c\nindex 50f213e..be60e89 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -258,7 +258,7 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n \n void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n-\t\tunsigned dirty_submodule,\n+\t\tunsigned dirty_submodule, const char *set,\n \t\tconst char *del, const char *add, const char *reset)\n {\n \tstruct rev_info rev;\n@@ -292,15 +292,15 @@ void show_submodule_summary(FILE *f, const char *path,\n \t\treturn;\n \t}\n \n-\tstrbuf_addf(&sb, \"Submodule %s %s..\", path,\n+\tstrbuf_addf(&sb, \"%sSubmodule %s %s..\", set, path,\n \t\t\tfind_unique_abbrev(one, DEFAULT_ABBREV));\n \tif (!fast_backward && !fast_forward)\n \t\tstrbuf_addch(&sb, '.');\n \tstrbuf_addf(&sb, \"%s\", find_unique_abbrev(two, DEFAULT_ABBREV));\n \tif (message)\n-\t\tstrbuf_addf(&sb, \" %s\\n\", message);\n+\t\tstrbuf_addf(&sb, \" %s%s\\n\", message, reset);\n \telse\n-\t\tstrbuf_addf(&sb, \"%s:\\n\", fast_backward ? \" (rewind)\" : \"\");\n+\t\tstrbuf_addf(&sb, \"%s:%s\\n\", fast_backward ? \" (rewind)\" : \"\", reset);\n \tfwrite(sb.buf, sb.len, 1, f);\n \n \tif (!message) {\ndiff --git a/submodule.h b/submodule.h\nindex 594b50d..ef5aab6 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -20,7 +20,7 @@ void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tunsigned char one[20], unsigned char two[20],\n-\t\tunsigned dirty_submodule,\n+\t\tunsigned dirty_submodule, const char *set,\n \t\tconst char *del, const char *add, const char *reset);\n void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n-- \n1.7.8.1.362.g5d6df.dirty\n"},{"id":"200360","messageId":"506B3F74.3020208@web.de","threadId":"31716","inReplyTo":"1349196670-2844-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-02T19:24:36Z","receivedAt":"2012-10-02T19:24:36Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n> Currently, the diff code does not differentiate between an explicit\n> '--submodule=short' being passed, and no submodule option being passed\n> on the command line.  Making this differentiation will be important\n> when the command-line option can be used to override a\n> \"diff.submoduleFormat\" configuration variable introduced in the next\n> patch.\n\nWouldn't it be sufficient here to simply reset the log flag by using\n\"DIFF_OPT_CLR(options, SUBMODULE_LOG)\"? This would avoid having to\nuse the last bit of the diffopt flags. And if I read the code correctly,\ndiff_opt_parse() is called by setup_revisions() which is called after\ngit_config(), so that should be safe. (And \"textconv\" uses the same\napproach)\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  diff.c |    4 +++-\n>  diff.h |   17 +++++++++--------\n>  2 files changed, 12 insertions(+), 9 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index 35d3f07..8ea40f9 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3647,7 +3647,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n>  \t} else if (!strcmp(arg, \"--submodule\"))\n>  \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n>  \telse if (!prefixcmp(arg, \"--submodule=\")) {\n> -\t\tif (!strcmp(arg + 12, \"log\"))\n> +\t\tif (!strcmp(arg + 12, \"short\"))\n> +\t\t\tDIFF_OPT_SET(options, SUBMODULE_SHORT);\n> +\t\telse if (!strcmp(arg + 12, \"log\"))\n>  \t\t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n>  \t}\n>  \n> diff --git a/diff.h b/diff.h\n> index a658f85..4115b49 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -77,14 +77,15 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n>  #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)\n>  #define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)\n>  #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)\n> -#define DIFF_OPT_SUBMODULE_LOG       (1 << 23)\n> -#define DIFF_OPT_DIRTY_SUBMODULES    (1 << 24)\n> -#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)\n> -#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)\n> -#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 27)\n> -#define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 28)\n> -#define DIFF_OPT_FUNCCONTEXT         (1 << 29)\n> -#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 30)\n> +#define DIFF_OPT_SUBMODULE_SHORT     (1 << 23)\n> +#define DIFF_OPT_SUBMODULE_LOG       (1 << 24)\n> +#define DIFF_OPT_DIRTY_SUBMODULES    (1 << 25)\n> +#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 26)\n> +#define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 27)\n> +#define DIFF_OPT_OVERRIDE_SUBMODULE_CONFIG (1 << 28)\n> +#define DIFF_OPT_DIRSTAT_BY_LINE     (1 << 29)\n> +#define DIFF_OPT_FUNCCONTEXT         (1 << 30)\n> +#define DIFF_OPT_PICKAXE_IGNORE_CASE (1 << 31)\n>  \n>  #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)\n>  #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)\n> \n"},{"id":"200364","messageId":"506B4418.2050700@web.de","threadId":"31716","inReplyTo":"1349196670-2844-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-02T19:44:24Z","receivedAt":"2012-10-02T19:44:24Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n> Introduce a diff.submoduleFormat configuration variable corresponding\n> to the '--submodule' command-line option of 'git diff'.\n\nNice. Maybe a better name would be \"diff.submodule\", as this sets the\ndefault for the \"--submodule\" option of diff?\n\nAnd I think you should also test in t4041 that \"--submodule=short\"\noverrides the config setting.\n\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  Documentation/diff-config.txt    |    7 +++++++\n>  Documentation/diff-options.txt   |    3 ++-\n>  diff.c                           |   25 ++++++++++++++++---------\n>  t/t4041-diff-submodule-option.sh |   10 ++++++++++\n>  4 files changed, 35 insertions(+), 10 deletions(-)\n> \n> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\n> index 04574f9..e445cc8 100644\n> --- a/Documentation/diff-config.txt\n> +++ b/Documentation/diff-config.txt\n> @@ -103,6 +103,13 @@ diff.suppressBlankEmpty::\n>  \tA boolean to inhibit the standard behavior of printing a space\n>  \tbefore each empty output line. Defaults to false.\n>  \n> +diff.submoduleFormat::\n> +\tSpecify the format in which differences in submodules are\n> +\tshown.  The \"log\" format lists the commits in the range like\n> +\tlinkgit:git-submodule[1] `summary` does.  The \"short\" format\n> +\tformat just shows the names of the commits at the beginning\n> +\tand end of the range.  Defaults to short.\n> +\n>  diff.wordRegex::\n>  \tA POSIX Extended Regular Expression used to determine what is a \"word\"\n>  \twhen performing word-by-word difference calculations.  Character\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index cf4b216..034c4e7 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -170,7 +170,8 @@ any of those replacements occurred.\n>  \tthe commits in the range like linkgit:git-submodule[1] `summary` does.\n>  \tOmitting the `--submodule` option or specifying `--submodule=short`,\n>  \tuses the 'short' format. This format just shows the names of the commits\n> -\tat the beginning and end of the range.\n> +\tat the beginning and end of the range.  Can be tweaked via the\n> +\t`diff.submoduleFormat` configuration variable.\n>  \n>  --color[=<when>]::\n>  \tShow colored diff.\n> diff --git a/diff.c b/diff.c\n> index 8ea40f9..4cb8dd2 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -28,6 +28,7 @@ static int diff_suppress_blank_empty;\n>  static int diff_use_color_default = -1;\n>  static const char *diff_word_regex_cfg;\n>  static const char *external_diff_cmd_cfg;\n> +static const char *submodule_format_cfg;\n>  int diff_auto_refresh_index = 1;\n>  static int diff_mnemonic_prefix;\n>  static int diff_no_prefix;\n> @@ -161,6 +162,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n>  \t\tdiff_stat_graph_width = git_config_int(var, value);\n>  \t\treturn 0;\n>  \t}\n> +\tif (!strcmp(var, \"diff.submoduleformat\"))\n> +\t\treturn git_config_string(&submodule_format_cfg, var, value);\n>  \tif (!strcmp(var, \"diff.external\"))\n>  \t\treturn git_config_string(&external_diff_cmd_cfg, var, value);\n>  \tif (!strcmp(var, \"diff.wordregex\"))\n> @@ -2227,15 +2230,19 @@ static void builtin_diff(const char *name_a,\n>  \t\tline_prefix = msgbuf->buf;\n>  \t}\n>  \n> -\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) &&\n> -\t\t\t(!one->mode || S_ISGITLINK(one->mode)) &&\n> -\t\t\t(!two->mode || S_ISGITLINK(two->mode))) {\n> -\t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n> -\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n> -\t\tshow_submodule_summary(o->file, one ? one->path : two->path,\n> -\t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n> -\t\t\t\tdel, add, reset);\n> -\t\treturn;\n> +\tif (!DIFF_OPT_TST(o, SUBMODULE_SHORT) &&\n> +\t\t((!one->mode || S_ISGITLINK(one->mode)) &&\n> +\t\t\t(!two->mode || S_ISGITLINK(two->mode)))) {\n> +\t\tif (DIFF_OPT_TST(o, SUBMODULE_LOG) ||\n> +\t\t\t(submodule_format_cfg &&\n> +\t\t\t\t!strcmp(submodule_format_cfg, \"log\"))) {\n> +\t\t\tconst char *del = diff_get_color_opt(o, DIFF_FILE_OLD);\n> +\t\t\tconst char *add = diff_get_color_opt(o, DIFF_FILE_NEW);\n> +\t\t\tshow_submodule_summary(o->file, one ? one->path : two->path,\n> +\t\t\t\t\tone->sha1, two->sha1, two->dirty_submodule,\n> +\t\t\t\t\tdel, add, reset);\n> +\t\t\treturn;\n> +\t\t}\n>  \t}\n>  \n>  \tif (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {\n> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\n> index 6c01d0c..ed4f3a8 100755\n> --- a/t/t4041-diff-submodule-option.sh\n> +++ b/t/t4041-diff-submodule-option.sh\n> @@ -43,6 +43,16 @@ EOF\n>  \ttest_cmp expected actual\n>  \"\n>  \n> +test_expect_success 'added submodule, set diff.submoduleFormat' \"\n> +\tgit config diff.submoduleFormat log &&\n> +\tgit add sm1 &&\n> +\tgit diff --cached >actual &&\n> +\tcat >expected <<-EOF &&\n> +Submodule sm1 0000000...$head1 (new submodule)\n> +EOF\n> +\ttest_cmp expected actual\n> +\"\n> +\n>  commit_file sm1 &&\n>  head2=$(add_file sm1 foo3)\n>  \n> \n"},{"id":"200416","messageId":"506C4161.3040201@web.de","threadId":"31716","inReplyTo":"506B4418.2050700@web.de","subject":"Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-03T13:45:05Z","receivedAt":"2012-10-03T13:45:05Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 02.10.2012 21:44, schrieb Jens Lehmann:\n> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n>> Introduce a diff.submoduleFormat configuration variable corresponding\n>> to the '--submodule' command-line option of 'git diff'.\n> \n> Nice. Maybe a better name would be \"diff.submodule\", as this sets the\n> default for the \"--submodule\" option of diff?\n> \n> And I think you should also test in t4041 that \"--submodule=short\"\n> overrides the config setting.\n\nWe also need tests which show that setting that config to \"log\" does\nnot break one of the many users of \"git diff\" (\"stash\", \"rebase\" and\n\"format-patch\" come to mind, most probably I missed some others). I\nsuspect we'll have to add \"--submodule=short\" options to some call\nsites to keep them working with submodule changes.\n"},{"id":"200698","messageId":"CALkWK0mbQZjYUR73rdFP2BtOzdL9gQqxnC5fjtsCYq2F29=8qg@mail.gmail.com","threadId":"31716","inReplyTo":"506B3F74.3020208@web.de","subject":"Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-07T15:22:05Z","receivedAt":"2012-10-07T15:22:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n>> Currently, the diff code does not differentiate between an explicit\n>> '--submodule=short' being passed, and no submodule option being passed\n>> on the command line.  Making this differentiation will be important\n>> when the command-line option can be used to override a\n>> \"diff.submoduleFormat\" configuration variable introduced in the next\n>> patch.\n>\n> Wouldn't it be sufficient here to simply reset the log flag by using\n> \"DIFF_OPT_CLR(options, SUBMODULE_LOG)\"? This would avoid having to\n> use the last bit of the diffopt flags. And if I read the code correctly,\n> diff_opt_parse() is called by setup_revisions() which is called after\n> git_config(), so that should be safe. (And \"textconv\" uses the same\n> approach)\n\nHow is it sufficient?  In git_diff_ui_config(), I set\nsubmodule_format_cfg, which has nothing to do with SUBMODULE_LOG.  In\nbuiltin_diff(), I'll have to check SUBMODULE_LOG and\nsubmodule_format_cfg.  The tricky bit is that I should check\nsubmodule_format_cfg if and only if \"--submodule=short\" was NOT passed\non the command line-  now, that's not the same thing is checking if\nSUBMODULE_LOG is unset, because SUBMODULE_LOG is unset (or cleared) if\nno argument was passed or if \"--submodule=short\" is passed.\nTherefore, I need a SUBMODULE_SHORT to differentiate between the two\ncases.\n\nWhat am I missing?\n\nRam\n"},{"id":"200705","messageId":"5071DCC8.50004@web.de","threadId":"31716","inReplyTo":"CALkWK0mbQZjYUR73rdFP2BtOzdL9gQqxnC5fjtsCYq2F29=8qg@mail.gmail.com","subject":"Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-07T19:49:28Z","receivedAt":"2012-10-07T19:49:28Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.10.2012 17:22, schrieb Ramkumar Ramachandra:\n> Jens Lehmann wrote:\n>> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n>>> Currently, the diff code does not differentiate between an explicit\n>>> '--submodule=short' being passed, and no submodule option being passed\n>>> on the command line.  Making this differentiation will be important\n>>> when the command-line option can be used to override a\n>>> \"diff.submoduleFormat\" configuration variable introduced in the next\n>>> patch.\n>>\n>> Wouldn't it be sufficient here to simply reset the log flag by using\n>> \"DIFF_OPT_CLR(options, SUBMODULE_LOG)\"? This would avoid having to\n>> use the last bit of the diffopt flags. And if I read the code correctly,\n>> diff_opt_parse() is called by setup_revisions() which is called after\n>> git_config(), so that should be safe. (And \"textconv\" uses the same\n>> approach)\n> \n> How is it sufficient?  In git_diff_ui_config(), I set\n> submodule_format_cfg, which has nothing to do with SUBMODULE_LOG.  In\n> builtin_diff(), I'll have to check SUBMODULE_LOG and\n> submodule_format_cfg.  The tricky bit is that I should check\n> submodule_format_cfg if and only if \"--submodule=short\" was NOT passed\n> on the command line-  now, that's not the same thing is checking if\n> SUBMODULE_LOG is unset, because SUBMODULE_LOG is unset (or cleared) if\n> no argument was passed or if \"--submodule=short\" is passed.\n> Therefore, I need a SUBMODULE_SHORT to differentiate between the two\n> cases.\n> \n> What am I missing?\n\nI forgot to mention that testing submodule_format_cfg would have to\nhappen in cmd_diff() (between reading the config and parsing the\ncommand line options) instead of builtin_diff(). Something like this\nshould do the trick (untested):\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 9650be2..180bf44 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -297,6 +297,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix\n        DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);\n        DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);\n\n+       if (submodule_format_cfg && !strcmp(submodule_format_cfg, \"log\"))\n+               DIFF_OPT_SET(options, SUBMODULE_LOG);\n+\n        if (nongit)\n                die(_(\"Not a git repository\"));\n        argc = setup_revisions(argc, argv, &rev, NULL);\n"},{"id":"200706","messageId":"5071DE3C.5080206@web.de","threadId":"31716","inReplyTo":"5071DCC8.50004@web.de","subject":"Re: [PATCH 3/5] diff: acknowledge --submodule=short command-line option","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-07T19:55:40Z","receivedAt":"2012-10-07T19:55:40Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 07.10.2012 21:49, schrieb Jens Lehmann:\n> I forgot to mention that testing submodule_format_cfg would have to\n> happen in cmd_diff() (between reading the config and parsing the\n> command line options) instead of builtin_diff(). Something like this\n> should do the trick (untested):\n> \n> diff --git a/builtin/diff.c b/builtin/diff.c\n> index 9650be2..180bf44 100644\n> --- a/builtin/diff.c\n> +++ b/builtin/diff.c\n> @@ -297,6 +297,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix\n>         DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);\n>         DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);\n> \n> +       if (submodule_format_cfg && !strcmp(submodule_format_cfg, \"log\"))\n> +               DIFF_OPT_SET(options, SUBMODULE_LOG);\n\nObviously not even compile tested, the line above should read:\n\n+               DIFF_OPT_SET(&rev.diffopt, SUBMODULE_LOG);\n\n> +\n>         if (nongit)\n>                 die(_(\"Not a git repository\"));\n>         argc = setup_revisions(argc, argv, &rev, NULL);\n"},{"id":"202111","messageId":"CALkWK0nPJ-8UvyYN800-Tj+mLQtv-vF3SJScb1O8EEiY238Z6w@mail.gmail.com","threadId":"31716","inReplyTo":"506C4161.3040201@web.de","subject":"Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2012-10-29T10:30:23Z","receivedAt":"2012-10-29T10:30:23Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> Am 02.10.2012 21:44, schrieb Jens Lehmann:\n>> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n>>> Introduce a diff.submoduleFormat configuration variable corresponding\n>>> to the '--submodule' command-line option of 'git diff'.\n>>\n>> Nice. Maybe a better name would be \"diff.submodule\", as this sets the\n>> default for the \"--submodule\" option of diff?\n>>\n>> And I think you should also test in t4041 that \"--submodule=short\"\n>> overrides the config setting.\n>\n> We also need tests which show that setting that config to \"log\" does\n> not break one of the many users of \"git diff\" (\"stash\", \"rebase\" and\n> \"format-patch\" come to mind, most probably I missed some others). I\n> suspect we'll have to add \"--submodule=short\" options to some call\n> sites to keep them working with submodule changes.\n\nUm, why would \"stash\", \"rebase\" or \"format-patch\" be affected by this\nsetting?  They don't operate on submodules at all.  To be sure, I ran\nall the tests with the following diff and nothing broke.\n\nConfused,\n\nRam\n\n-- 8< --\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 514282c..904a81c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -608,6 +608,8 @@ fi\n # in subprocesses like git equals our $PWD (for pathname comparisons).\n cd -P \"$test\" || exit 1\n\n+git config test.submodule log\n+\n this_test=${0##*/}\n this_test=${this_test%%-*}\n for skp in $GIT_SKIP_TESTS\n"},{"id":"202240","messageId":"509045EF.9030401@web.de","threadId":"31716","inReplyTo":"CALkWK0nPJ-8UvyYN800-Tj+mLQtv-vF3SJScb1O8EEiY238Z6w@mail.gmail.com","subject":"Re: [PATCH 4/5] diff: introduce diff.submoduleFormat configuration variable","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-10-30T21:26:07Z","receivedAt":"2012-10-30T21:26:07Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 29.10.2012 11:30, schrieb Ramkumar Ramachandra:\n> Jens Lehmann wrote:\n>> Am 02.10.2012 21:44, schrieb Jens Lehmann:\n>>> Am 02.10.2012 18:51, schrieb Ramkumar Ramachandra:\n>>>> Introduce a diff.submoduleFormat configuration variable corresponding\n>>>> to the '--submodule' command-line option of 'git diff'.\n>>>\n>>> Nice. Maybe a better name would be \"diff.submodule\", as this sets the\n>>> default for the \"--submodule\" option of diff?\n>>>\n>>> And I think you should also test in t4041 that \"--submodule=short\"\n>>> overrides the config setting.\n>>\n>> We also need tests which show that setting that config to \"log\" does\n>> not break one of the many users of \"git diff\" (\"stash\", \"rebase\" and\n>> \"format-patch\" come to mind, most probably I missed some others). I\n>> suspect we'll have to add \"--submodule=short\" options to some call\n>> sites to keep them working with submodule changes.\n> \n> Um, why would \"stash\", \"rebase\" or \"format-patch\" be affected by this\n> setting?  They don't operate on submodules at all.  To be sure, I ran\n> all the tests with the following diff and nothing broke.\n\nThey do operate on the submodule commits too (while they don't touch\nsubmodule work trees) and IIRC rebase applies diffs, so that could\nbreak when the output of diff changes due to the new config option.\nBut it looks like your test did prove that nothing goes wrong there,\nI assume they they use plumbing diff commands which aren't affected\nby the new option.\n"}]}