{"thread":{"id":"38163","subject":"[PATCH] added git-config support for diff.relative setting","startedAt":"2014-12-11T07:28:32Z","lastAt":"2015-01-07T20:26:41Z","messageCount":13,"participants":["Kelson","Duy Nguyen","kelson@shysecurity.com","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"253559","messageId":"548947A0.7000608@shysecurity.com","threadId":"38163","inReplyTo":null,"subject":"[PATCH] added git-config support for diff.relative setting","fromName":"Kelson","fromEmail":"kelson@shysecurity.com","sentAt":"2014-12-11T07:28:32Z","receivedAt":"2014-12-11T07:28:32Z","isPatch":true,"sender":{"key":"kelson@shysecurity.com","avatar":null},"body":"By default, git-diff shows changes and pathnames relative to the \nrepository root.\nSetting the diff.relative config option to \"true\" shows pathnames \nrelative to\nthe current directory and excludes changes outside this directory.\n---\n Documentation/diff-config.txt | 6 ++++++\n diff.c | 8 ++++++++\n t/t4045-diff-relative.sh | 21 +++++++++++++++++++++\n 3 files changed, 35 insertions(+)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex b001779..10f183f 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -182,3 +182,9 @@ diff.algorithm::\n \tlow-occurrence common elements\".\n --\n +\n+\n+diff.relative::\n+\tBy default, linkgit:git-diff[1] shows changes and pathnames\n+\trelative to the repository root. Setting this variable to\n+\t`true` shows pathnames relative to the current directory and\n+\texcludes changes outside this directory.\ndiff --git a/diff.c b/diff.c\nindex d1bd534..22daa2f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -270,6 +270,14 @@ int git_diff_basic_config(const char *var, const \nchar *value, void *cb)\n \t\treturn 0;\n \t}\n\n+\tif (!strcmp(var, \"diff.relative\")) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tDIFF_OPT_SET(&default_diff_options, RELATIVE_NAME);\n+\t\telse\n+\t\t\tDIFF_OPT_CLR(&default_diff_options, RELATIVE_NAME);\n+\t\treturn 0;\n+\t}\n+\n \tif (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex 3950f50..8c8fe0b 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -29,6 +29,23 @@ test_expect_success \"-p $*\" \"\n \"\n }\n\n+check_config() {\n+expect=$1; shift\n+cat >expected <../actual) &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected < wrote:\n> @@ -270,6 +270,14 @@ int git_diff_basic_config(const char *var, const char\n> *value, void *cb)\n> return 0;\n> }\n>\n> + if (!strcmp(var, \"diff.relative\")) {\n> + if (git_config_bool(var, value))\n> + DIFF_OPT_SET(&default_diff_options, RELATIVE_NAME);\n> + else\n> + DIFF_OPT_CLR(&default_diff_options, RELATIVE_NAME);\n> + return 0;\n> + }\n> +\n> if (starts_with(var, \"submodule.\"))\n> return parse_submodule_config_option(var, value);\n>\n\nThis affects more than just git-diff. git_diff_ui_config() may be a\nbetter place.\n-- \nDuy\n"},{"id":"253579","messageId":"548A0F73.80109@shysecurity.com","threadId":"38163","inReplyTo":"CACsJy8CBRzbVrKnKPACuqxLw2N3PiK2O2nE4-C_3jfEp6-2p=g@mail.gmail.com","subject":"Re: [PATCH] added git-config support for diff.relative setting","fromName":"Kelson","fromEmail":"kelson@shysecurity.com","sentAt":"2014-12-11T21:41:07Z","receivedAt":"2014-12-11T21:41:07Z","isPatch":true,"sender":{"key":"kelson@shysecurity.com","avatar":null},"body":"That is quite manageable. I was concerned that --relative changes the UI \n(relative paths) and behavior (excluding files outside the current \ndirectory), which might not be clear if placed in just the UI component. \nYou make a great point that git_diff_basic_config drives other commands \nthough, like git-bisect, which --relative would not effect.\n\n-----Original Message-----\nFrom: Duy Nguyen \nSent: 12/11/2014 08:37 AM\nTo: Kelson \nCC: Git Mailing List \nSubject: Re: [PATCH] added git-config support for diff.relative setting\n\nOn Thu, Dec 11, 2014 at 2:28 PM, Kelson wrote:\n> @@ -270,6 +270,14 @@ int git_diff_basic_config(const char *var, const char\n> *value, void *cb)\n> return 0;\n> }\n>\n> + if (!strcmp(var, \"diff.relative\")) {\n> + if (git_config_bool(var, value))\n> + DIFF_OPT_SET(&default_diff_options, RELATIVE_NAME);\n> + else\n> + DIFF_OPT_CLR(&default_diff_options, RELATIVE_NAME);\n> + return 0;\n> + }\n> +\n> if (starts_with(var, \"submodule.\"))\n> return parse_submodule_config_option(var, value);\n>\n\nThis affects more than just git-diff. git_diff_ui_config() may be a\nbetter place.\n"},{"id":"253676","messageId":"548B7967.3060201@shysecurity.com","threadId":"38163","inReplyTo":"548947A0.7000608@shysecurity.com","subject":"[PATCH v2] added git-config support for diff.relative setting","fromName":"Kelson","fromEmail":"kelson@shysecurity.com","sentAt":"2014-12-12T23:25:27Z","receivedAt":"2014-12-12T23:25:27Z","isPatch":true,"sender":{"key":"kelson@shysecurity.com","avatar":null},"body":"By default, git-diff shows changes and pathnames relative to the \nrepository root. Setting the diff.relative config option to \"true\" shows \npathnames relative to the current directory and excludes changes outside \nthis directory.\n\nSigned-off-by: Brandon Phillips \n---\n Documentation/diff-config.txt | 6 ++++++\n diff.c | 8 ++++++++\n t/t4045-diff-relative.sh | 21 +++++++++++++++++++++\n 3 files changed, 35 insertions(+)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex b001779..10f183f 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -182,3 +182,9 @@ diff.algorithm::\n \tlow-occurrence common elements\".\n --\n +\n+\n+diff.relative::\n+\tBy default, linkgit:git-diff[1] shows changes and pathnames\n+\trelative to the repository root. Setting this variable to\n+\t`true` shows pathnames relative to the current directory and\n+\texcludes changes outside this directory.\ndiff --git a/diff.c b/diff.c\nindex d1bd534..03697a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -223,6 +223,14 @@ int git_diff_ui_config(const char *var, const char \n*value, void *cb)\n \t\treturn 0;\n \t}\n\n+\tif (!strcmp(var, \"diff.relative\")) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tDIFF_OPT_SET(&default_diff_options, RELATIVE_NAME);\n+\t\telse\n+\t\t\tDIFF_OPT_CLR(&default_diff_options, RELATIVE_NAME);\n+\t\treturn 0;\n+\t}\n+\n \tif (git_color_config(var, value, cb) < 0)\n \t\treturn -1;\n\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex 3950f50..8c8fe0b 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -29,6 +29,23 @@ test_expect_success \"-p $*\" \"\n \"\n }\n\n+check_config() {\n+expect=$1; shift\n+cat >expected <../actual) &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected <expected <../actual) &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected <expected <../actual) &&\n+ test_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected < By default, git-diff shows changes and pathnames relative to the\n> repository root.\n> Setting diff.relative to \"true\" shows pathnames relative to the\n> current directory\n> and excludes changes outside this directory.\n\nThe above does not tell any lie, but it is mostly a description of\nwhat \"--relative\" does. I think the main point of this change is\nthat by configuring a variable \"diff.relative\" (and forget about it)\nyou do not have to keep saying \"--relative\" from the command line,\nso that is how you should \"sell\" this change to others, I think.\n\n> ---\n\nNo signoff?\n\n> Documentation/diff-config.txt | 5 +++++\n> diff.c | 8 ++++++++\n> t/t4045-diff-relative.sh | 21 +++++++++++++++++++++\n> 3 files changed, 34 insertions(+)\n>\n> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\n> index b001779..496e9b0 100644\n> --- a/Documentation/diff-config.txt\n> +++ b/Documentation/diff-config.txt\n> @@ -103,6 +103,11 @@ diff.orderfile::\n> one shell glob pattern per line.\n> Can be overridden by the '-O' option to linkgit:git-diff[1].\n\nWhitespace damaged patch? HTs are all gone. Please practice first\nby sending your patches to yourself, and then try to \"git am\" them\nby saving the e-mail messages you received from yourself.\n\n\"git grep -i thunder Documentation\" seem to tell me that there are\nhints on using Thunderbird in SubmittingPatches and format-patch\ndocumentation.\n\nI might have said this already, but I think this change, without\nsupport for \"--no-relative\", would be an incomplete one that can\nbreak the end-user experience, and it would be better to swap the\norder of the patches.\n\nThanks.\n"},{"id":"254210","messageId":"54A2FDC8.5010504@shysecurity.com","threadId":"38163","inReplyTo":"54A2E744.8010508@shysecurity.com","subject":"[PATCH 1/2] support for --no-relative and diff.relative","fromName":"","fromEmail":"kelson@shysecurity.com","sentAt":"2014-12-30T19:32:24Z","receivedAt":"2014-12-30T19:32:24Z","isPatch":true,"sender":{"key":"kelson@shysecurity.com","avatar":null},"body":"added --no-relative option for git-diff (code, documentation, and tests)\n\n--no-relative overrides --relative causing a return to standard behavior\n\nSigned-off-by: Brandon Phillips \n---\n Documentation/diff-options.txt | 4 ++++\n diff.c | 2 ++\n t/t4045-diff-relative.sh | 46 \n+++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 49 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 6cb083a..2b15050 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -448,6 +448,10 @@ ifndef::git-format-patch[]\n \tnot in a subdirectory (e.g. in a bare repository), you\n \tcan name which subdirectory to make the output relative\n \tto by giving a as an argument.\n+\n+--no-relative::\n+\tTurn off relative pathnames and include all changes in the\n+\trepository.\n endif::git-format-patch[]\n\n -a::\ndiff --git a/diff.c b/diff.c\nindex d1bd534..7bceba8 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options, \nconst char **av, int ac)\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n \t\toptions->prefix = arg;\n \t}\n+\telse if (!strcmp(arg, \"--no-relative\"))\n+\t\tDIFF_OPT_CLR(options, RELATIVE_NAME);\n\n \t/* xdiff options */\n \telse if (!strcmp(arg, \"--minimal\"))\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex 3950f50..ccd67c7 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -12,8 +12,8 @@ test_expect_success 'setup' '\n \tgit commit -m one\n '\n\n-check_diff() {\n-expect=$1; shift\n+store_diff_relative() {\n+expect=$1;\n cat >expected <expected <actual &&\n \ttest_cmp expected actual\n \"\n }\n\n+check_norel_pre() {\n+store_diff_relative $1; shift\n+test_expect_success \"-p --no-relative $*\" \"\n+\tgit diff -p --no-relative $* HEAD^ >actual &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n+check_norel_post() {\n+store_diff_absolute $1; shift\n+test_expect_success \"-p $* --no-relative\" \"\n+\tgit diff -p $* --no-relative HEAD^ >actual &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected <\n---\n Documentation/diff-options.txt | 4 ++++\n diff.c | 2 ++\n t/t4045-diff-relative.sh | 46 \n+++++++++++++++++++++++++++++++++++++++---\n 3 files changed, 49 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 6cb083a..2b15050 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -448,6 +448,10 @@ ifndef::git-format-patch[]\n \tnot in a subdirectory (e.g. in a bare repository), you\n \tcan name which subdirectory to make the output relative\n \tto by giving a as an argument.\n+\n+--no-relative::\n+\tTurn off relative pathnames and include all changes in the\n+\trepository.\n endif::git-format-patch[]\n\n -a::\ndiff --git a/diff.c b/diff.c\nindex d1bd534..7bceba8 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options, \nconst char **av, int ac)\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n \t\toptions->prefix = arg;\n \t}\n+\telse if (!strcmp(arg, \"--no-relative\"))\n+\t\tDIFF_OPT_CLR(options, RELATIVE_NAME);\n\n \t/* xdiff options */\n \telse if (!strcmp(arg, \"--minimal\"))\ndiff --git a/t/t4045-diff-relative.sh b/t/t4045-diff-relative.sh\nindex 3950f50..ccd67c7 100755\n--- a/t/t4045-diff-relative.sh\n+++ b/t/t4045-diff-relative.sh\n@@ -12,8 +12,8 @@ test_expect_success 'setup' '\n \tgit commit -m one\n '\n\n-check_diff() {\n-expect=$1; shift\n+store_diff_relative() {\n+expect=$1;\n cat >expected <expected <actual &&\n \ttest_cmp expected actual\n \"\n }\n\n+check_norel_pre() {\n+store_diff_relative $1; shift\n+test_expect_success \"-p --no-relative $*\" \"\n+\tgit diff -p --no-relative $* HEAD^ >actual &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n+check_norel_post() {\n+store_diff_absolute $1; shift\n+test_expect_success \"-p $* --no-relative\" \"\n+\tgit diff -p $* --no-relative HEAD^ >actual &&\n+\ttest_cmp expected actual\n+\"\n+}\n+\n check_numstat() {\n expect=$1; shift\n cat >expected < Content-Type: text/plain; charset=utf-8; format=flowed\n\nPlease. No format=flawed. Really.\n\n> Subject: Re: [PATCH 1/2] support for --no-relative and diff.relative\n\n\"diff: teach --no-relative to override earlier --relative\" or\nsomething. Saying the affeced area upfront, terminated with a colon\n':', will make it easier to spot in \"git shortlog\" output later.\n\nAlso this step is not about --no-relative and diff.relative but is\nonly about --no-relative option.\n\n> added --no-relative option for git-diff (code, documentation, and tests)\n\n\"Add --no-relative option...\"; we write in imperative, as if we are\ngiving an order to the project secretary to \"make the code do/be so\".\n\n> --no-relative overrides --relative causing a return to standard behavior\n\nOK (modulo missing full-stop).\n\n>\n> Signed-off-by: Brandon Phillips \n\nPlease also have\n\n\tFrom: Brandon Phillips \n\nas the first line of the body of your e-mail message, if you are\nletting your MUA only give your e-mail address without name.\nAlternatively, please ask/configure your MUA to put your name as\nwell as your address on From: header of the e-mail (which is\npreferrable).\n\n> diff --git a/diff.c b/diff.c\n> index d1bd534..7bceba8 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options,\n> const char **av, int ac)\n\nLine-wrapped.\n\n> \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n> \t\toptions->prefix = arg;\n> \t}\n> +\telse if (!strcmp(arg, \"--no-relative\"))\n> +\t\tDIFF_OPT_CLR(options, RELATIVE_NAME);\n>\n\nWhen \"--relative\" is given, options->prefix is set to something as\nwe can see above.\n\nThe --relative option is described as optionally taking in\nthe doc:\n\n --relative[=]::\n When run from a subdirectory of the project, it can be\n told to exclude changes outside the directory and show\n pathnames relative to it with this option. When you are\n not in a subdirectory (e.g. in a bare repository), you\n can name which subdirectory to make the output relative\n to by giving a as an argument.\n\nDoesn't \"--no-relative\" codepath have to undo the effect of that\nassignment to options->prefix?\n\nFor example, after applying this patch, shouldn't\n\n\t$ cd t\n\t$ git show --relative=Documentation --no-relative --relative\n\nwork the same way as\n\n\t$ cd t\n\t$ git show --relative\n\ni.e. limiting its output to the changes in the 't/' directory and\nnot to the changes in the 'Documentation/' directory?\n\nPatch 2/2 also seems to share similar line-wrapping breakages that\nmake it unappliable, but more importantly, the configuration that is\nsupposed to correspond to --relative option only parses a boolean.\nIs that the right design, or should it also be able to substitute a\ncommand line `--relative=` with an argument?\n\nThe last was a half-way rhetorical question and my answer is that\nboolean-only is the best you could do, because we cannot do the\nusual \"bool or string\" trick when \"string\" can be arbitrary. In\nother words, \"diff.relative=true\" could mean \"limit to the current\nsubdirectory\" aka \"--relative\" or it could mean \"limit to true/\nsubdirectory\" aka \"--relative=true\", and there is no good way to\ndisambiguate between the two [*1*].\n\nSo I can agree with the design decision but only after spending 6\nlines to think about it. For the end-users, this design decision\nneeds to be explained and spelled out in the documentation. Saying\n\"equivalent to `--relative`\" is not sufficient, because the way\n`--relative` option itself is described elsewhere. The option\nappears as `--relative[=]` (see above), so some people _will_\nread \"equivalent to `--relative`\" to mean \"Setting diff.relative=t\nshould be equivalent to --relative=t\", which is not what actually\nhappens.\n\n\n[Footnote]\n\n*1* Actually, you could declare that \"diff.relative=true/\" means the\n 'true/' directory while \"diff.relative=true\" means the boolean\n 'true' aka 'diff --relative', but I think it is too confusing.\n Let's not make it worse by going that route.\n"},{"id":"254411","messageId":"54AD7F1A.3060500@shysecurity.com","threadId":"38163","inReplyTo":"xmqqiogijwdp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] support for --no-relative and diff.relative","fromName":"","fromEmail":"kelson@shysecurity.com","sentAt":"2015-01-07T18:46:50Z","receivedAt":"2015-01-07T18:46:50Z","isPatch":true,"sender":{"key":"kelson@shysecurity.com","avatar":null},"body":">> Content-Type: text/plain; charset=utf-8; format=flowed\n>Please. No format=flawed. Really.\nI'll figure out the line-wrapping.\n\n> Also this step is not about --no-relative and diff.relative but is only about --no-relative option.\nShould I submit as two independent patches then? I took the approach of \nsplitting them out into 1/2 vs 2/2 to distinguish, but it sounds like \nthat isn't optimal.\n\n> When \"--relative\" is given, options->prefix is set to something as we can see above.\n>\n> The --relative option is described as optionally taking in the doc:\n...\n> Doesn't \"--no-relative\" codepath have to undo the effect of that assignment to options->prefix?\ndiff_setup_done NULLs options->prefix when DIFF_OPT_TST(options, \nRELATIVE_NAME) is cleared (--no-relative clears this option).\n\nOn review, this may be a bad approach though. Non-locality makes it \nharder to follow/understand and introduces a subtle bug.\ncurrent: \"git-diff --relative=path --no-relative --relative\" == \n\"git-diff --relative=path\"\nexpected: \"git-diff --relative=path --no-relative --relative\" == \n\"git-diff --relative\"\n\n> So I can agree with the design decision but only after spending 6\n> lines to think about it. For the end-users, this design decision\n> needs to be explained and spelled out in the documentation.\nAgreed; update to come.\n\n-----Original Message-----\nFrom: Junio C Hamano \nSent: 01/07/2015 01:09 PM\nTo: kelson@shysecurity.com\nCC: Git Mailing List , Philip Oakley \n, Duy Nguyen , Jonathan \nNieder \nSubject: Re: [PATCH 1/2] support for --no-relative and diff.relative\n\n> kelson@shysecurity.com writes:\n\n> Content-Type: text/plain; charset=utf-8; format=flowed\n\nPlease. No format=flawed. Really.\n\n> Subject: Re: [PATCH 1/2] support for --no-relative and diff.relative\n\n\"diff: teach --no-relative to override earlier --relative\" or\nsomething. Saying the affeced area upfront, terminated with a colon\n':', will make it easier to spot in \"git shortlog\" output later.\n\nAlso this step is not about --no-relative and diff.relative but is\nonly about --no-relative option.\n\n> added --no-relative option for git-diff (code, documentation, and tests)\n\n\"Add --no-relative option...\"; we write in imperative, as if we are\ngiving an order to the project secretary to \"make the code do/be so\".\n\n> --no-relative overrides --relative causing a return to standard behavior\n\nOK (modulo missing full-stop).\n\n>\n> Signed-off-by: Brandon Phillips \n\nPlease also have\n\n\tFrom: Brandon Phillips \n\nas the first line of the body of your e-mail message, if you are\nletting your MUA only give your e-mail address without name.\nAlternatively, please ask/configure your MUA to put your name as\nwell as your address on From: header of the e-mail (which is\npreferrable).\n\n> diff --git a/diff.c b/diff.c\n> index d1bd534..7bceba8 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options,\n> const char **av, int ac)\n\nLine-wrapped.\n\n> \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n> \t\toptions->prefix = arg;\n> \t}\n> +\telse if (!strcmp(arg, \"--no-relative\"))\n> +\t\tDIFF_OPT_CLR(options, RELATIVE_NAME);\n>\n\nWhen \"--relative\" is given, options->prefix is set to something as\nwe can see above.\n\nThe --relative option is described as optionally taking in\nthe doc:\n\n --relative[=]::\n When run from a subdirectory of the project, it can be\n told to exclude changes outside the directory and show\n pathnames relative to it with this option. When you are\n not in a subdirectory (e.g. in a bare repository), you\n can name which subdirectory to make the output relative\n to by giving a as an argument.\n\nDoesn't \"--no-relative\" codepath have to undo the effect of that\nassignment to options->prefix?\n\nFor example, after applying this patch, shouldn't\n\n\t$ cd t\n\t$ git show --relative=Documentation --no-relative --relative\n\nwork the same way as\n\n\t$ cd t\n\t$ git show --relative\n\ni.e. limiting its output to the changes in the 't/' directory and\nnot to the changes in the 'Documentation/' directory?\n\nPatch 2/2 also seems to share similar line-wrapping breakages that\nmake it unappliable, but more importantly, the configuration that is\nsupposed to correspond to --relative option only parses a boolean.\nIs that the right design, or should it also be able to substitute a\ncommand line `--relative=` with an argument?\n\nThe last was a half-way rhetorical question and my answer is that\nboolean-only is the best you could do, because we cannot do the\nusual \"bool or string\" trick when \"string\" can be arbitrary. In\nother words, \"diff.relative=true\" could mean \"limit to the current\nsubdirectory\" aka \"--relative\" or it could mean \"limit to true/\nsubdirectory\" aka \"--relative=true\", and there is no good way to\ndisambiguate between the two [*1*].\n\nSo I can agree with the design decision but only after spending 6\nlines to think about it. For the end-users, this design decision\nneeds to be explained and spelled out in the documentation. Saying\n\"equivalent to `--relative`\" is not sufficient, because the way\n`--relative` option itself is described elsewhere. The option\nappears as `--relative[=]` (see above), so some people _will_\nread \"equivalent to `--relative`\" to mean \"Setting diff.relative=t\nshould be equivalent to --relative=t\", which is not what actually\nhappens.\n\n\n[Footnote]\n\n*1* Actually, you could declare that \"diff.relative=true/\" means the\n 'true/' directory while \"diff.relative=true\" means the boolean\n 'true' aka 'diff --relative', but I think it is too confusing.\n Let's not make it worse by going that route.\n"},{"id":"254412","messageId":"xmqq7fwyjtxg.fsf@gitster.dls.corp.google.com","threadId":"38163","inReplyTo":"xmqqiogijwdp.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] support for --no-relative and diff.relative","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T19:02:19Z","receivedAt":"2015-01-07T19:02:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Patch 2/2 also seems to share similar line-wrapping breakages that\n> make it unappliable, but more importantly, the configuration that is\n> supposed to correspond to --relative option only parses a boolean.\n> Is that the right design, or should it also be able to substitute a\n> command line `--relative=` with an argument?\n>\n> The last was a half-way rhetorical question and my answer is that\n> boolean-only is the best you could do...\n> ...\n> [Footnote]\n>\n> *1* Actually, you could declare that \"diff.relative=true/\" means the\n> 'true/' directory while \"diff.relative=true\" means the boolean\n> 'true' aka 'diff --relative', but I think it is too confusing.\n> Let's not make it worse by going that route.\n\nAddendum.\n\nIt was only a \"half-way rhetorical question\", because I am willing\nto be persuaded that diff.relative=true/ vs diff.relative=true is\n*not* too subtle/confusing to be a good idea, if enough people whose\njudgement I trust agrees.\n"},{"id":"254424","messageId":"xmqqy4peibge.fsf@gitster.dls.corp.google.com","threadId":"38163","inReplyTo":"54AD7F1A.3060500@shysecurity.com","subject":"Re: [PATCH 1/2] support for --no-relative and diff.relative","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-01-07T20:26:41Z","receivedAt":"2015-01-07T20:26:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"kelson@shysecurity.com writes:\n\n>>> Content-Type: text/plain; charset=utf-8; format=flowed\n>>Please. No format=flawed. Really.\n> I'll figure out the line-wrapping.\n>\n>> Also this step is not about --no-relative and diff.relative but is\n>> only about --no-relative option.\n> Should I submit as two independent patches then? I took the approach\n> of splitting them out into 1/2 vs 2/2 to distinguish, but it sounds\n> like that isn't optimal.\n\nThey are indeed better to be 1/2 and 2/2; they do not have to share\nthe same subject, though. 1/2 now adds only --no-relative and makes\nsure an earlier --relative is cancelled without even knowing that\ndiff.relative might appear in the future (well, you may know that,\nbut the system with only 1/2 applied without 2/2 would work perfectly\nfine). 2/2 adds diff.relative and makes sure --no-relative cancels\nits effect as well.\n\n> On review, this may be a bad approach though. Non-locality makes it\n> harder to follow/understand and introduces a subtle bug.\n> current: \"git-diff --relative=path --no-relative --relative\" ==\n> \"git-diff --relative=path\"\n> expected: \"git-diff --relative=path --no-relative --relative\" ==\n> \"git-diff --relative\"\n\nExactly.\n"}]}