{"thread":{"id":"42716","subject":"[PATCH 0/3] diff: add --relative-names","startedAt":"2016-06-26T17:16:29Z","lastAt":"2016-06-27T19:39:55Z","messageCount":9,"participants":["Nguyễn Thái Ngọc Duy","Eric Sunshine","Jeff King","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"290194","messageId":"20160626171616.27948-1-pclouds@gmail.com","threadId":"42716","inReplyTo":null,"subject":"[PATCH 0/3] diff: add --relative-names","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-26T17:16:13Z","receivedAt":"2016-06-26T17:16:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"For more explanation head straight to 3/3. The first two patches are\npreparation.\n\nNguyễn Thái Ngọc Duy (3):\n diff.c: refactor strip_prefix()\n diff.c: separate \"prefix\" from RELATIVE_NAME (aka --relative)\n diff.c: add --relative-names to be used with --name-only\n\n Documentation/diff-options.txt | 4 ++++\n diff.c | 54 +++++++++++++++++++++++++++++-------------\n diff.h | 1 +\n 3 files changed, 42 insertions(+), 17 deletions(-)\n\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290195","messageId":"20160626171616.27948-2-pclouds@gmail.com","threadId":"42716","inReplyTo":"20160626171616.27948-1-pclouds@gmail.com","subject":"[PATCH 1/3] diff.c: refactor strip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-26T17:16:14Z","receivedAt":"2016-06-26T17:16:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"By passing \"struct diff_options *\" to strip_prefix(), we can do some\nmore intelligent and repeated logic at one place. The removal of\n\"if (opt->prefix_length)\" is just the beginning.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n diff.c | 24 ++++++++++++++----------\n 1 file changed, 14 insertions(+), 10 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex fa78fc1..b4949a2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3147,8 +3147,15 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n \t\thashclr(one->sha1);\n }\n \n-static void strip_prefix(int prefix_length, const char **namep, const char **otherp)\n+static void strip_prefix(struct diff_options *opt,\n+\t\t\t const char **namep,\n+\t\t\t const char **otherp)\n {\n+\tint prefix_length = opt->prefix_length;\n+\n+\tif (!prefix_length)\n+\t\treturn;\n+\n \t/* Strip the prefix but do not molest /dev/null and absolute paths */\n \tif (*namep && **namep != '/') {\n \t\t*namep += prefix_length;\n@@ -3175,8 +3182,7 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \tname = p->one->path;\n \tother = (strcmp(name, p->two->path) ? p->two->path : NULL);\n \tattr_path = name;\n-\tif (o->prefix_length)\n-\t\tstrip_prefix(o->prefix_length, &name, &other);\n+\tstrip_prefix(o, &name, &other);\n \n \tif (!DIFF_OPT_TST(o, ALLOW_EXTERNAL))\n \t\tpgm = NULL;\n@@ -3230,8 +3236,7 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,\n \tname = p->one->path;\n \tother = (strcmp(name, p->two->path) ? p->two->path : NULL);\n \n-\tif (o->prefix_length)\n-\t\tstrip_prefix(o->prefix_length, &name, &other);\n+\tstrip_prefix(o, &name, &other);\n \n \tdiff_fill_sha1_info(p->one);\n \tdiff_fill_sha1_info(p->two);\n@@ -3254,8 +3259,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)\n \tother = (strcmp(name, p->two->path) ? p->two->path : NULL);\n \tattr_path = other ? other : name;\n \n-\tif (o->prefix_length)\n-\t\tstrip_prefix(o->prefix_length, &name, &other);\n+\tstrip_prefix(o, &name, &other);\n \n \tdiff_fill_sha1_info(p->one);\n \tdiff_fill_sha1_info(p->two);\n@@ -4133,14 +4137,14 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)\n \t\tconst char *name_a, *name_b;\n \t\tname_a = p->one->path;\n \t\tname_b = p->two->path;\n-\t\tstrip_prefix(opt->prefix_length, &name_a, &name_b);\n+\t\tstrip_prefix(opt, &name_a, &name_b);\n \t\twrite_name_quoted(name_a, opt->file, inter_name_termination);\n \t\twrite_name_quoted(name_b, opt->file, line_termination);\n \t} else {\n \t\tconst char *name_a, *name_b;\n \t\tname_a = p->one->mode ? p->one->path : p->two->path;\n \t\tname_b = NULL;\n-\t\tstrip_prefix(opt->prefix_length, &name_a, &name_b);\n+\t\tstrip_prefix(opt, &name_a, &name_b);\n \t\twrite_name_quoted(name_a, opt->file, line_termination);\n \t}\n }\n@@ -4345,7 +4349,7 @@ static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n \t\tconst char *name_a, *name_b;\n \t\tname_a = p->two->path;\n \t\tname_b = NULL;\n-\t\tstrip_prefix(opt->prefix_length, &name_a, &name_b);\n+\t\tstrip_prefix(opt, &name_a, &name_b);\n \t\twrite_name_quoted(name_a, opt->file, opt->line_termination);\n \t}\n }\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290196","messageId":"20160626171616.27948-3-pclouds@gmail.com","threadId":"42716","inReplyTo":"20160626171616.27948-1-pclouds@gmail.com","subject":"[PATCH 2/3] diff.c: separate \"prefix\" from RELATIVE_NAME (aka --relative)","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-26T17:16:15Z","receivedAt":"2016-06-26T17:16:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"So far \"prefix\" and \"prefix_length\" in struct diff_options are tied to\nRELATIVE_NAME flag, making it tricky to add new features that need the\nprefix.\n\nThis change essentially follows the foot steps in cd676a5 (diff\n--relative: output paths as relative to the current subdirectory -\n2008-02-12) and makes sure that the prefix stripping/filtering only\nhappens when RELATIVE_NAME flag is set. The stripping is much simpler\nbecause all stripping now goes through strip_prefix(). So the patch is\nmostly about filtering.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n diff.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex b4949a2..bd5d190 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3153,7 +3153,7 @@ static void strip_prefix(struct diff_options *opt,\n {\n \tint prefix_length = opt->prefix_length;\n \n-\tif (!prefix_length)\n+\tif (!prefix_length || !DIFF_OPT_TST(opt, RELATIVE_NAME))\n \t\treturn;\n \n \t/* Strip the prefix but do not molest /dev/null and absolute paths */\n@@ -3335,8 +3335,6 @@ void diff_setup_done(struct diff_options *options)\n \tif (DIFF_OPT_TST(options, FIND_COPIES_HARDER))\n \t\toptions->detect_rename = DIFF_DETECT_COPY;\n \n-\tif (!DIFF_OPT_TST(options, RELATIVE_NAME))\n-\t\toptions->prefix = NULL;\n \tif (options->prefix)\n \t\toptions->prefix_length = strlen(options->prefix);\n \telse\n@@ -4976,7 +4974,8 @@ void diff_addremove(struct diff_options *options,\n \t\taddremove = (addremove == '+' ? '-' :\n \t\t\t addremove == '-' ? '+' : addremove);\n \n-\tif (options->prefix &&\n+\tif (DIFF_OPT_TST(options, RELATIVE_NAME) &&\n+\t options->prefix &&\n \t strncmp(concatpath, options->prefix, options->prefix_length))\n \t\treturn;\n \n@@ -5021,7 +5020,8 @@ void diff_change(struct diff_options *options,\n \t\t\tnew_dirty_submodule = tmp;\n \t}\n \n-\tif (options->prefix &&\n+\tif (DIFF_OPT_TST(options, RELATIVE_NAME) &&\n+\t options->prefix &&\n \t strncmp(concatpath, options->prefix, options->prefix_length))\n \t\treturn;\n \n@@ -5048,7 +5048,8 @@ struct diff_filepair *diff_unmerge(struct diff_options *options, const char *pat\n \tstruct diff_filepair *pair;\n \tstruct diff_filespec *one, *two;\n \n-\tif (options->prefix &&\n+\tif (DIFF_OPT_TST(options, RELATIVE_NAME) &&\n+\t options->prefix &&\n \t strncmp(path, options->prefix, options->prefix_length))\n \t\treturn NULL;\n \n-- \n2.8.2.531.gd073806\n\n"},{"id":"290197","messageId":"20160626171616.27948-4-pclouds@gmail.com","threadId":"42716","inReplyTo":"20160626171616.27948-1-pclouds@gmail.com","subject":"[PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-26T17:16:16Z","receivedAt":"2016-06-26T17:16:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The difference with --relative option is, this option does not filter\npaths outside cwd. You can add two more chars \" .\" on your command\nline for that.\n\nThis serves two purposes\n\n - user friendlier to copy/paste. When full paths are shown, you can\n still use \":/\" magic to get around it, but unless you're a lefty\n you need to move your right hand between the keyboard and mouse for\n each path. Not very efficient.\n\n - script friendly. Strictly speaking a very light form of scripting\n from command line prompt when you just pipe paths to the next\n command. Paths relative to cwd would be much easier to pip this\n way.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n It would be great if --relative could be used for this (and it feels\n weird that the option performs both actions at once, stripping _and_\n filtering where filtering could easily be done with pathspec). But\n it's too late to change --relative behavior now.\n\n Documentation/diff-options.txt | 4 ++++\n diff.c | 21 ++++++++++++++++++---\n diff.h | 1 +\n 3 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex d9ae681..1554008 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -481,6 +481,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+--relative-names::\n+\tWhen used with `--name-only`, paths are shown relative to\n+\tcurrent working directory.\n endif::git-format-patch[]\n \n -a::\ndiff --git a/diff.c b/diff.c\nindex bd5d190..3a938ac 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3335,6 +3335,13 @@ void diff_setup_done(struct diff_options *options)\n \tif (DIFF_OPT_TST(options, FIND_COPIES_HARDER))\n \t\toptions->detect_rename = DIFF_DETECT_COPY;\n \n+\t/*\n+\t * --relative can change \"prefix\", which may cause unintended\n+\t * consequences for --relative-names\n+\t */\n+\tif (DIFF_OPT_TST(options, RELATIVE_NAME) && options->relative_names)\n+\t\tdie(_(\"--relative and --relative-names are mutually exclusive\"));\n+\n \tif (options->prefix)\n \t\toptions->prefix_length = strlen(options->prefix);\n \telse\n@@ -3792,7 +3799,8 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (skip_prefix(arg, \"--relative=\", &arg)) {\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n \t\toptions->prefix = arg;\n-\t}\n+\t} else if (!strcmp(arg, \"--relative-names\"))\n+\t\toptions->relative_names = 1;\n \n \t/* xdiff options */\n \telse if (!strcmp(arg, \"--minimal\"))\n@@ -4347,8 +4355,15 @@ static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n \t\tconst char *name_a, *name_b;\n \t\tname_a = p->two->path;\n \t\tname_b = NULL;\n-\t\tstrip_prefix(opt, &name_a, &name_b);\n-\t\twrite_name_quoted(name_a, opt->file, opt->line_termination);\n+\t\tif (opt->relative_names)\n+\t\t\twrite_name_quoted_relative(name_a,\n+\t\t\t\t\t\t opt->prefix, opt->file,\n+\t\t\t\t\t\t opt->line_termination);\n+\t\telse {\n+\t\t\tstrip_prefix(opt, &name_a, &name_b);\n+\t\t\twrite_name_quoted(name_a, opt->file,\n+\t\t\t\t\t opt->line_termination);\n+\t\t}\n \t}\n }\n \ndiff --git a/diff.h b/diff.h\nindex 125447b..ad6423d 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -139,6 +139,7 @@ struct diff_options {\n \tint dirstat_permille;\n \tint setup;\n \tint abbrev;\n+\tint relative_names;\n /* white-space error highlighting */\n #define WSEH_NEW 1\n #define WSEH_CONTEXT 2\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290207","messageId":"CAPig+cRQynnXzNSkYGGzuZH25+UAovYHqeBWyaFjb03OHvuCFQ@mail.gmail.com","threadId":"42716","inReplyTo":"20160626171616.27948-4-pclouds@gmail.com","subject":"Re: [PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-26T23:05:28Z","receivedAt":"2016-06-26T23:05:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Jun 26, 2016 at 1:16 PM, Nguyễn Thái Ngọc Duy wrote:\n> The difference with --relative option is, this option does not filter\n> paths outside cwd. You can add two more chars \" .\" on your command\n> line for that.\n>\n> This serves two purposes\n>\n> - user friendlier to copy/paste. When full paths are shown, you can\n> still use \":/\" magic to get around it, but unless you're a lefty\n> you need to move your right hand between the keyboard and mouse for\n> each path. Not very efficient.\n>\n> - script friendly. Strictly speaking a very light form of scripting\n> from command line prompt when you just pipe paths to the next\n> command. Paths relative to cwd would be much easier to pip this\n\ns/pip/pipe/\n\n> way.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n"},{"id":"290315","messageId":"20160627192426.GA10877@sigill.intra.peff.net","threadId":"42716","inReplyTo":"20160626171616.27948-4-pclouds@gmail.com","subject":"Re: [PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-27T19:24:26Z","receivedAt":"2016-06-27T19:24:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 26, 2016 at 07:16:16PM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> The difference with --relative option is, this option does not filter\n> paths outside cwd. You can add two more chars \" .\" on your command\n> line for that.\n\nAnother difference seems to be that it applies only to --name-only, and\nnot to other forms. I can see how \"-p --relative-names\" might be weird,\nbecause you'll get:\n\n diff --git a/../foo/bar b/../foo/bar\n\nor something. But surely things like --name-status would want to support\nit?\n\n> It would be great if --relative could be used for this (and it feels\n> weird that the option performs both actions at once, stripping _and_\n> filtering where filtering could easily be done with pathspec). But\n> it's too late to change --relative behavior now.\n\nI suspect the \"..\" weirdness above is one of the reasons that --relative\nrestricted itself to the current directory (but regardless, I agree that\nwe cannot change it now).\n\n-Peff\n"},{"id":"290318","messageId":"CACsJy8BYsPCLiEM2syrGUa_VOCHnR3W+sSiG3sNFme1L0Hx7XA@mail.gmail.com","threadId":"42716","inReplyTo":"20160627192426.GA10877@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-27T19:33:18Z","receivedAt":"2016-06-27T19:33:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jun 27, 2016 at 9:24 PM, Jeff King wrote:\n> On Sun, Jun 26, 2016 at 07:16:16PM +0200, Nguyễn Thái Ngọc Duy wrote:\n>\n>> The difference with --relative option is, this option does not filter\n>> paths outside cwd. You can add two more chars \" .\" on your command\n>> line for that.\n>\n> Another difference seems to be that it applies only to --name-only, and\n> not to other forms. I can see how \"-p --relative-names\" might be weird,\n> because you'll get:\n>\n> diff --git a/../foo/bar b/../foo/bar\n>\n> or something. But surely things like --name-status would want to support\n> it?\n\nThat's my plan :) Anything that does not start with a/ or b/ should\nrespect this new option.\n-- \nDuy\n"},{"id":"290319","messageId":"20160627193542.GC10877@sigill.intra.peff.net","threadId":"42716","inReplyTo":"CACsJy8BYsPCLiEM2syrGUa_VOCHnR3W+sSiG3sNFme1L0Hx7XA@mail.gmail.com","subject":"Re: [PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-27T19:35:42Z","receivedAt":"2016-06-27T19:35:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 27, 2016 at 09:33:18PM +0200, Duy Nguyen wrote:\n\n> On Mon, Jun 27, 2016 at 9:24 PM, Jeff King wrote:\n> > On Sun, Jun 26, 2016 at 07:16:16PM +0200, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> The difference with --relative option is, this option does not filter\n> >> paths outside cwd. You can add two more chars \" .\" on your command\n> >> line for that.\n> >\n> > Another difference seems to be that it applies only to --name-only, and\n> > not to other forms. I can see how \"-p --relative-names\" might be weird,\n> > because you'll get:\n> >\n> > diff --git a/../foo/bar b/../foo/bar\n> >\n> > or something. But surely things like --name-status would want to support\n> > it?\n> \n> That's my plan :) Anything that does not start with a/ or b/ should\n> respect this new option.\n\nI guess it seems a bit unfortunate that:\n\n git log -p --relative-names .\n\nwould not be equivalent to:\n\n git log -p --relative\n\nI am not entirely convinced that \"a/../Documentation/git.txt\" is that\nunreasonable. Sure, it looks weird, and probably nobody will be able to\napply the patch. But it is what the user asked for.\n\n-Peff\n"},{"id":"290321","messageId":"CACsJy8AQt5SLBufuzGwjoAQVWjTZ3oSvTB+Yz4Kp8hKAy0sD9A@mail.gmail.com","threadId":"42716","inReplyTo":"20160627193542.GC10877@sigill.intra.peff.net","subject":"Re: [PATCH/RFC 3/3] diff.c: add --relative-names to be used with --name-only","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-27T19:39:21Z","receivedAt":"2016-06-27T19:39:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jun 27, 2016 at 9:35 PM, Jeff King wrote:\n> On Mon, Jun 27, 2016 at 09:33:18PM +0200, Duy Nguyen wrote:\n>\n>> On Mon, Jun 27, 2016 at 9:24 PM, Jeff King wrote:\n>> > On Sun, Jun 26, 2016 at 07:16:16PM +0200, Nguyễn Thái Ngọc Duy wrote:\n>> >\n>> >> The difference with --relative option is, this option does not filter\n>> >> paths outside cwd. You can add two more chars \" .\" on your command\n>> >> line for that.\n>> >\n>> > Another difference seems to be that it applies only to --name-only, and\n>> > not to other forms. I can see how \"-p --relative-names\" might be weird,\n>> > because you'll get:\n>> >\n>> > diff --git a/../foo/bar b/../foo/bar\n>> >\n>> > or something. But surely things like --name-status would want to support\n>> > it?\n>>\n>> That's my plan :) Anything that does not start with a/ or b/ should\n>> respect this new option.\n>\n> I guess it seems a bit unfortunate that:\n>\n> git log -p --relative-names .\n>\n> would not be equivalent to:\n>\n> git log -p --relative\n>\n> I am not entirely convinced that \"a/../Documentation/git.txt\" is that\n> unreasonable. Sure, it looks weird, and probably nobody will be able to\n> apply the patch. But it is what the user asked for.\n\nIf we go that far I would drop a/ and b/ because this is already\nun-patchable, and without the prefix, the user can happily copy and\npaste again. Gaah lots more work..\n-- \nDuy\n"}]}