{"thread":{"id":"28831","subject":"[PATCH] log: support --full-diff=","startedAt":"2011-11-03T10:01:14Z","lastAt":"2011-11-05T03:25:42Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"178763","messageId":"1320314474-19536-1-git-send-email-pclouds@gmail.com","threadId":"28831","inReplyTo":null,"subject":"[PATCH] log: support --full-diff=","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-03T10:01:14Z","receivedAt":"2011-11-03T10:01:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I wanted to check if any patches that update builtin/fsck.c also\nupdate 't' directory and git seemed not support this case\n(true?). With this I can do\n\n git log --patch --full-diff=\"'builtin/fsck.c' 't'\" -- builtin/fsck.c\n\nI guess this may be something people find useful.\n\nThis patch is a bit inconvenient though because is parsed\nwith sq_dequote_to_argv() and all arguments must be wrapped by ''.\nAlso \"full-diff\" name does not make much sense when it comes with\npathspec.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n revision.c | 19 +++++++++++++++++++\n revision.h | 1 +\n 2 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 8764dde..f508953 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -13,6 +13,7 @@\n #include \"decorate.h\"\n #include \"log-tree.h\"\n #include \"string-list.h\"\n+#include \"quote.h\"\n \n volatile show_early_output_fn_t show_early_output;\n \n@@ -1531,6 +1532,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--full-diff\")) {\n \t\trevs->diff = 1;\n \t\trevs->full_diff = 1;\n+\t} else if (!prefixcmp(arg, \"--full-diff=\")) {\n+\t\trevs->diff = 1;\n+\t\trevs->full_diff = 1;\n+\t\trevs->full_diff_opt = arg + strlen(\"--full-diff=\");\n \t} else if (!strcmp(arg, \"--full-history\")) {\n \t\trevs->simplify_history = 0;\n \t} else if (!strcmp(arg, \"--relative-date\")) {\n@@ -1819,6 +1824,20 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\trevs->prune = 1;\n \t\tif (!revs->full_diff)\n \t\t\tdiff_tree_setup_paths(revs->prune_data.raw, &revs->diffopt);\n+\t\telse if (revs->full_diff_opt) {\n+\t\t\tconst char **argv = NULL;\n+\t\t\tint alloc = 0, nr = 0;\n+\t\t\tchar *arg;\n+\n+\t\t\targ = xstrdup(revs->full_diff_opt);\n+\t\t\tsq_dequote_to_argv(arg, &argv, &nr, &alloc);\n+\n+\t\t\tALLOC_GROW(argv, nr + 1, alloc);\n+\t\t\targv[nr] = NULL;\n+\t\t\targv = get_pathspec(revs->prefix, argv);\n+\n+\t\t\tdiff_tree_setup_paths(argv, &revs->diffopt);\n+\t\t}\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\ndiff --git a/revision.h b/revision.h\nindex 6aa53d1..baa709c 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -137,6 +137,7 @@ struct rev_info {\n \tconst char\t*subject_prefix;\n \tint\t\tno_inline;\n \tint\t\tshow_log_size;\n+\tconst char *full_diff_opt;\n \n \t/* Filter by commit log message */\n \tstruct grep_opt\tgrep_filter;\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"178769","messageId":"1320322556-32675-1-git-send-email-pclouds@gmail.com","threadId":"28831","inReplyTo":"1320314474-19536-1-git-send-email-pclouds@gmail.com","subject":"[PATCH] log: allow to specify diff pathspec in addition to prune pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-03T12:15:56Z","receivedAt":"2011-11-03T12:15:56Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Pathspec in \"git log -p \" is used for both commit pruning\nand diff generation. If --full-diff is given, then diff pathspec is\nreset to generate complete diff.\n\nThis patch gives more control to diff generation. The first pathspec\nin \"git log -p -- -- \" is used as commit pruning\nas usual. The second one is used for diff generation. So --full-diff\nnow is essentially \"git log -p -- --\".\n\nThis form requires specifying \"--\" twice. If a file name happens to be\n\"--\", it may be misunderstood as the second \"--\" marker. This is an\nunfortunate consequence for this syntax. Users can still use \"./--\" or\nsimilar to workaround this.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Version 2. Now it looks more acceptable.\n\n Documentation/git-log.txt | 9 +++++++--\n revision.c | 28 +++++++++++++++++++++++++---\n 2 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 249fc87..8e00dbc 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -9,7 +9,7 @@ git-log - Show commit logs\n SYNOPSIS\n --------\n [verse]\n-'git log' [] [..] [[\\--] ...]\n+'git log' [] [..] [[\\--] ... [\\-- ...]]\n \n DESCRIPTION\n -----------\n@@ -52,11 +52,12 @@ OPTIONS\n \tcommit was reached.\n \n --full-diff::\n-\tWithout this flag, \"git log -p ...\" shows commits that\n+\tWithout this flag, `git log -p ...` shows commits that\n \ttouch the specified paths, and diffs about the same specified\n \tpaths. With this, the full diff is shown for commits that touch\n \tthe specified paths; this means that \"...\" limits only\n \tcommits, and doesn't limit diff for those commits.\n+\tIt is equivalent to `git log -p \\-- ... \\--`.\n +\n Note that this affects all diff-based output types, e.g. those\n produced by --stat etc.\n@@ -76,6 +77,10 @@ produced by --stat etc.\n +\n To prevent confusion with options and branch names, paths may need to\n be prefixed with \"\\-- \" to separate them from options or refnames.\n++\n+If the second \"\\--\" is found, the following pathspec is used to limit\n+diff generation. Note that this affects all diff-based output types,\n+e.g. those produced by --stat etc.\n \n include::rev-list-options.txt[]\n \ndiff --git a/revision.c b/revision.c\nindex 8764dde..f560647 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1682,20 +1682,37 @@ static int handle_revision_pseudo_opt(const char *submodule,\n */\n int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)\n {\n-\tint i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0;\n+\tint i, flags, left, read_from_stdin, got_rev_arg = 0;\n+\tint seen_dashdash, seen_second_dashdash;\n \tstruct cmdline_pathspec prune_data;\n+\tstruct cmdline_pathspec diff_data;\n \tconst char *submodule = NULL;\n \n \tmemset(&prune_data, 0, sizeof(prune_data));\n+\tmemset(&diff_data, 0, sizeof(diff_data));\n \tif (opt)\n \t\tsubmodule = opt->submodule;\n \n \t/* First, search for \"--\" */\n-\tseen_dashdash = 0;\n+\tseen_dashdash = seen_second_dashdash = 0;\n \tfor (i = 1; i < argc; i++) {\n+\t\tint i2;\n \t\tconst char *arg = argv[i];\n \t\tif (strcmp(arg, \"--\"))\n \t\t\tcontinue;\n+\n+\t\t/* Search for second \"--\" */\n+\t\tfor (i2 = i + 1; i2 < argc; i2++) {\n+\t\t\tconst char *arg = argv[i2];\n+\t\t\tif (strcmp(arg, \"--\"))\n+\t\t\t\tcontinue;\n+\t\t\targv[i2] = NULL;\n+\t\t\tif (argv[i2 + 1])\n+\t\t\t\tappend_prune_data(&diff_data, argv + i2 + 1);\n+\t\t\tseen_second_dashdash = 1;\n+\t\t\tbreak;\n+\t\t}\n+\n \t\targv[i] = NULL;\n \t\targc = i;\n \t\tif (argv[i + 1])\n@@ -1817,7 +1834,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t/* Can't prune commits with rename following: the paths change.. */\n \t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n \t\t\trevs->prune = 1;\n-\t\tif (!revs->full_diff)\n+\t\tif (seen_second_dashdash) {\n+\t\t\tALLOC_GROW(diff_data.path, diff_data.nr+1, diff_data.alloc);\n+\t\t\tdiff_data.path[diff_data.nr++] = NULL;\n+\t\t\tdiff_tree_setup_paths(diff_data.path, &revs->diffopt);\n+\t\t}\n+\t\telse if (!revs->full_diff)\n \t\t\tdiff_tree_setup_paths(revs->prune_data.raw, &revs->diffopt);\n \t}\n \tif (revs->combine_merges)\n-- \n1.7.4.74.g639db\n"},{"id":"178858","messageId":"7vwrbfa6j6.fsf@alter.siamese.dyndns.org","threadId":"28831","inReplyTo":"1320322556-32675-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-04T22:04:29Z","receivedAt":"2011-11-04T22:04:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Pathspec in \"git log -p \" is used for both commit pruning\n> and diff generation. If --full-diff is given, then diff pathspec is\n> reset to generate complete diff.\n>\n> This patch gives more control to diff generation. The first pathspec in\n> \"git log -p -- -- \" is used as commit pruning\n> as usual. The second one is used for diff generation. So --full-diff\n> now is essentially \"git log -p -- --\".\n\nI agree that giving more control to diff generation is a good idea, and\nthis certainly is better than the previous round that nobody reviewed\nbefore you rerolled this round.\n\nBut I have doubts about declaring \"--full-diff is equivalent to giving the\n'output' pathspec that is empty\".\n\nHave you thought about the interaction between this and -M/-C options?\n"},{"id":"178859","messageId":"7vsjm3a6dc.fsf@alter.siamese.dyndns.org","threadId":"28831","inReplyTo":"1320322556-32675-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-04T22:07:59Z","receivedAt":"2011-11-04T22:07:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> This form requires specifying \"--\" twice. If a file name happens to be\n> \"--\", it may be misunderstood as the second \"--\" marker. This is an\n> unfortunate consequence for this syntax. Users can still use \"./--\" or\n> similar to workaround this.\n\nThis is not a new \"problem\" (and it is probably not a problem to begin\nwith). If you had \"--\" in the working tree and you wanted to treat it as\na path, you said either one of these:\n\n $ git log HEAD ./--\n $ git log HEAD -- --\n\nThe latter is what your patch breaks, I suspect.\n\nAlso it forces existing scripts and programs that knew \"everything in this\narray is a pathspec\" and added \"--\" before adding the contents of the\narray to form a command line to drive \"git log\" family of commands to be\nupdated.\n"},{"id":"178868","messageId":"CACsJy8CYEN5Li43C0H2hLbkAzEmGSUBiN+qyQ8tcOLx=3Cox0Q@mail.gmail.com","threadId":"28831","inReplyTo":"7vwrbfa6j6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] log: allow to specify diff pathspec in addition to prune pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-11-05T03:25:42Z","receivedAt":"2011-11-05T03:25:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/11/5 Junio C Hamano :\n> Nguyễn Thái Ngọc Duy   writes:\n>\n>> Pathspec in \"git log -p \" is used for both commit pruning\n>> and diff generation. If --full-diff is given, then diff pathspec is\n>> reset to generate complete diff.\n>>\n>> This patch gives more control to diff generation.  The first pathspec in\n>> \"git log -p -- -- \" is used as commit pruning\n>> as usual. The second one is used for diff generation. So --full-diff\n>> now is essentially \"git log -p -- --\".\n>\n> I agree that giving more control to diff generation is a good idea, and\n> this certainly is better than the previous round that nobody reviewed\n> before you rerolled this round.\n>\n> But I have doubts about declaring \"--full-diff is equivalent to giving the\n> 'output' pathspec that is empty\".\n\nThe equivalent command should be \"git log -p -- -- :/\" (in\ncase you're in subdir). What doubts specifically?\n\n> Have you thought about the interaction between this and -M/-C options?\n\nNo, yes I may have problems there.\n\n> This is not a new \"problem\" (and it is probably not a problem to begin\n> with). If you had \"--\" in the working tree and you wanted to treat it as\n> a path, you said either one of these:\n>\n> $ git log HEAD ./--\n> $ git log HEAD -- --\n>\n> The latter is what your patch breaks, I suspect.\n>\n> Also it forces existing scripts and programs that knew \"everything in this\n> array is a pathspec\" and added \"--\" before adding the contents of the\n> array to form a command line to drive \"git log\" family of commands to be\n> updated.\n\nWe could only recognize the second \"--\" when a new argument is given\nso it won't break existing use. But I'll need to look at -C/-M first.\nSmells like --follow problem.\n-- \nDuy\n"}]}