{"thread":{"id":"55116","subject":"[PATCH] git-difftool-helper.sh: learn a new way skip to save point","startedAt":"2021-02-07T15:23:36Z","lastAt":"2021-02-25T19:02:46Z","messageCount":37,"participants":["阿德烈 via GitGitGadget","Junio C Hamano","胡哲宁","ZheNing Hu via GitGitGadget","David Aguilar","ZheNing Hu","Junio C Hamano via GitGitGadget","Denton Liu","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"416355","messageId":"pull.870.git.1612711153591.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":null,"subject":"[PATCH] git-difftool-helper.sh: learn a new way skip to save point","fromName":"阿德烈 via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-07T15:19:13Z","receivedAt":"2021-02-07T15:23:36Z","isPatch":true,"sender":{"key":"name:阿德烈","avatar":null},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view In turn.\nIf there is a commit with many files and we exit in search,\nWe will have to traverse list again to get the file diff which\nwe want to see.Therefore,here is a new method:every time before\nwe view the file diff,the current coordinates will be stored in\n`GIT_DIR/difftool_skip_to`,this file will be deleted after\nsuccessful traversing.But if an unexpected exit occurred midway,\ngit will view the coordinates in the save point,ask user if they\nwant continue from the last saved point.This will improve the\nuser experience.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    git-difftool-helper.sh: learn a new way skip to save point\n    \n    this patch's origin discuss is here:\n    https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n    \n    git user may should travel the diff list to choice file diff to view, if\n    they exit in midway,they must travel it again. I’m on the basis of the\n    \"difftool_skip_to\" suggested by Junio,Provides a possibility for this\n    user-friendly solution.\n    \n    Thanks!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\n git-difftool--helper.sh | 32 ++++++++++++++++++++++++++++++++\n 1 file changed, 32 insertions(+)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b718..56ec1d38a7a1 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -6,6 +6,7 @@\n # Copyright (c) 2009, 2010 David Aguilar\n \n TOOL_MODE=diff\n+GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n . git-mergetool--lib\n \n # difftool.prompt controls the default prompt/no-prompt behavior\n@@ -40,6 +41,31 @@ launch_merge_tool () {\n \t# the user with the real $MERGED name before launching $merge_tool.\n \tif should_prompt\n \tthen\n+\t\tif test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n+\t\tthen\n+\t\t\tSAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n+\t\t\tif test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n+\t\t\t\ttest $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n+\t\t\tthen\n+\t\t\t\t# choice skip or not skip when check first file.\n+\t\t\t\tif test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n+\t\t\t\tthen\n+\t\t\t\t\tprintf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n+\t\t\t\t\tread skip_ans || return\n+\t\t\t\t\tif test \"$skip_ans\" = y\n+\t\t\t\t\tthen\n+\t\t\t\t\t\treturn\n+\t\t\t\t\tfi\n+\t\t\t\telse\n+\t\t\t\t\treturn\n+\t\t\t\tfi\n+\t\t\tfi\n+\t\tfi\n+\t\t# write the current coordinates to .git/difftool-skip-to\n+\t\tif test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER\n+\t\tthen\n+\t\t\techo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE\n+\t\tfi\n \t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n \t\tif use_ext_cmd\n@@ -102,4 +128,10 @@ else\n \tdone\n fi\n \n+if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&\n+\ttest $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL\n+then\n+\trm $GIT_DIFFTOOL_SKIP_TO_FILE\n+\n+fi\n exit 0\n\nbase-commit: e6362826a0409539642a5738db61827e5978e2e4\n-- \ngitgitgadget\n"},{"id":"416367","messageId":"xmqqczxb91el.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.git.1612711153591.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-difftool-helper.sh: learn a new way skip to save point","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-07T18:29:54Z","receivedAt":"2021-02-07T18:31:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"阿德烈 via GitGitGadget\"  <gitgitgadget@gmail.com> writes:\n\n> From: ZheNing Hu <adlternative@gmail.com>\n\nI am a bit confused.  Are 胡哲宁 and 阿德烈 and ZeNing Hu all the\nsame person (I am asking that an earlier question came under the\nname first listed in this sentence, and the patch uses the latter\ntwo names, where I guess )?\n\n>\n> `git difftool` only allow us to select file to view In turn.\n> If there is a commit with many files and we exit in search,\n> We will have to traverse list again to get the file diff which\n> we want to see.Therefore,here is a new method:every time before\n> we view the file diff,the current coordinates will be stored in\n> `GIT_DIR/difftool_skip_to`,this file will be deleted after\n> successful traversing.But if an unexpected exit occurred midway,\n> git will view the coordinates in the save point,ask user if they\n> want continue from the last saved point.This will improve the\n> user experience.\n>\n> Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> ---\n>     git-difftool-helper.sh: learn a new way skip to save point\n>     \n>     this patch's origin discuss is here:\n>     https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n>     \n>     git user may should travel the diff list to choice file diff to view, if\n>     they exit in midway,they must travel it again. I’m on the basis of the\n>     \"difftool_skip_to\" suggested by Junio,Provides a possibility for this\n>     user-friendly solution.\n>     \n>     Thanks!\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/870\n>\n>  git-difftool--helper.sh | 32 ++++++++++++++++++++++++++++++++\n>  1 file changed, 32 insertions(+)\n>\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 46af3e60b718..56ec1d38a7a1 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -6,6 +6,7 @@\n>  # Copyright (c) 2009, 2010 David Aguilar\n>  \n>  TOOL_MODE=diff\n> +GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n>  . git-mergetool--lib\n>  \n>  # difftool.prompt controls the default prompt/no-prompt behavior\n> @@ -40,6 +41,31 @@ launch_merge_tool () {\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n>  \tif should_prompt\n>  \tthen\n> +\t\tif test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n> +\t\tthen\n> +\t\t\tSAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n> +\t\t\tif test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n> +\t\t\t\ttest $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n> +\t\t\tthen\n> +\t\t\t\t# choice skip or not skip when check first file.\n> +\t\t\t\tif test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n> +\t\t\t\tthen\n> +\t\t\t\t\tprintf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n> +\t\t\t\t\tread skip_ans || return\n> +\t\t\t\t\tif test \"$skip_ans\" = y\n> +\t\t\t\t\tthen\n> +\t\t\t\t\t\treturn\n> +\t\t\t\t\tfi\n> +\t\t\t\telse\n> +\t\t\t\t\treturn\n> +\t\t\t\tfi\n> +\t\t\tfi\n> +\t\tfi\n> +\t\t# write the current coordinates to .git/difftool-skip-to\n> +\t\tif test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER\n> +\t\tthen\n> +\t\t\techo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE\n> +\t\tfi\n>  \t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n>  \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n>  \t\tif use_ext_cmd\n> @@ -102,4 +128,10 @@ else\n>  \tdone\n>  fi\n>  \n> +if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&\n> +\ttest $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL\n> +then\n> +\trm $GIT_DIFFTOOL_SKIP_TO_FILE\n> +\n> +fi\n>  exit 0\n>\n> base-commit: e6362826a0409539642a5738db61827e5978e2e4\n"},{"id":"416374","messageId":"xmqq8s7z8zsg.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.git.1612711153591.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-difftool-helper.sh: learn a new way skip to save point","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-07T19:04:47Z","receivedAt":"2021-02-07T19:05:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sorry, but a not-yet-written reply went out by accident; please\ndiscard it.\n\n> `git difftool` only allow us to select file to view In turn.\n\nFunny capitalization \"In\"?\n\n> If there is a commit with many files and we exit in search,\n> We will have to traverse list again to get the file diff which\n> we want to see.Therefore,here is a new method:every time before\n\nIt makes it hard to lack SP after punctuation like '.', ',', and\n':'.\n\n> we view the file diff,the current coordinates will be stored in\n> `GIT_DIR/difftool_skip_to`,this file will be deleted after\n> successful traversing.But if an unexpected exit occurred midway,\n> git will view the coordinates in the save point,ask user if they\n> want continue from the last saved point.This will improve the\n> user experience.\n\nI think the idea sounds good.  Admittedly I do not use difftool\nmyself, so I do not even know if and how the current end user\nexperience is so bad to require a patch like this (e.g. I do not\nknow how \"unexpected exit\" is \"unexpected\"---isn't it the end user\ninitiated action to \"quit\", or does the tool crash or something?).\n\nSo I won't be the best qualified person to judge if the solution\npresented is the best one for the problem.  \n\n    $ git shortlog --no-merges git-diff-helper.sh\n\nmight be a good way to find whom to ask for review and help.\n\nHaving said that, I do have one opinion on the \"skip-to\" filename.\nI do not think it is wise to call it after the purpose you want to\nuse it for (i.e. \"I want to use it to skip to the recorded\nposition\").  Instead, if the file records \"the last visited\nposition\", it is better to name it after that\n(e.g. \"difftool-last-position\".  If it records \"the next file to be\nvisited\", then \"difftool-next-file\" may be a good name).\n\nThe reason is because your first design may be to visit the file the\nuser was visiting before the \"crash\" happened, but you may later\nwant to revise the design to allow the user to say \"start at one\nfile before the file I was visiting\" etc.  The location recorded in\nthe file may still be used to decide where the code skips to when\nrestarting, but no longer exactly where the code \"skips to\".  If you\nname it after what it is, not what it is (currently) used for, the\ndesign would become clearer.\n\n\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 46af3e60b718..56ec1d38a7a1 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -6,6 +6,7 @@\n>  # Copyright (c) 2009, 2010 David Aguilar\n>  \n>  TOOL_MODE=diff\n> +GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n>  . git-mergetool--lib\n>  \n>  # difftool.prompt controls the default prompt/no-prompt behavior\n> @@ -40,6 +41,31 @@ launch_merge_tool () {\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n>  \tif should_prompt\n>  \tthen\n> +\t\tif test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n> +\t\tthen\n> +\t\t\tSAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n\nYou can avoid the TOCTTOU race by\n\n\t\tif SAVE_POINT=$(cat 2>/dev/null \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n\t\tthen\n\nbut that wouldn't probably matter in this application.\n\n> +\t\t\tif test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n> +\t\t\t\ttest $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n\nThink what happens if the file is corrupt and SAVE_POINT_NUM has (1)\nan empty string, (2) garbage that has $IFS whitespace, (3) non\nnumber, in it.  At least, quoting the variable inside double-quotes,\ni.e. \"$SAVE_POINT_NUM\", would help an error condition reported\ncorrectly at the runtime.\n\n> +\t\t\tthen\n> +\t\t\t\t# choice skip or not skip when check first file.\n\nA bit funny language.  Isn't the code clear enough without this comment?\n\n> +\t\t\t\tif test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n\nNo need to quote the constant \"1\"; quoting the variable side may be\na good practice, even though I think in this codepath we know\nGIT_DIFF_PATH_COUNTER is a well-formatted number.\n\n> +\t\t\t\tthen\n> +\t\t\t\t\tprintf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n\n\"Skip\" is probably an implementation detail that the user does not\nhave to know.  \"Do you want to start from the last file you were\nviewing?\", perhaps?\n\n> +\t\t\t\t\tread skip_ans || return\n> +\t\t\t\t\tif test \"$skip_ans\" = y\n> +\t\t\t\t\tthen\n> +\t\t\t\t\t\treturn\n> +\t\t\t\t\tfi\n> +\t\t\t\telse\n> +\t\t\t\t\treturn\n> +\t\t\t\tfi\n> +\t\t\tfi\n> +\t\tfi\n> +\t\t# write the current coordinates to .git/difftool-skip-to\n> +\t\tif test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER\n\nHave this code been tested?  I think \"test\" is missing after the\n\"||\", and I am not quite sure what you are trying to check with\n\"test !$SAVE_POINT_NUM\", either.  The \"test\" utility, when given a\nnon-operator string (like \"!23\" this one is checking when the last\nvisited path was the 23rd one), returns true if the string is not an\nempty string, and by definition a string made by appending anything\nafter \"!\" would not be empty, so the entire \"|| $SAVE_POINT_NUM ...\"\nhave been skipped in your test, I think.\n\nIs writing the current position to the file unconditionally good\nenough?  After all, we are about to go interactive with the user, so\nthe body of this \"if\" statement won't be performance critical in any\nsense, no?  Or is there something more subtle going on and\ncorrectness of the code depends on this condition?  I cannot quite\ntell.\n\n> +\t\tthen\n> +\t\t\techo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE\n\n\t\techo \"$GIT_DIFF_PATH_COUNTER\" >\"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n\ncf. Documentation/CodingGuidelines\n\n - Redirection operators should be written with space before, but no\n   space after them.  In other words, write 'echo test >\"$file\"'\n   instead of 'echo test> $file' or 'echo test > $file'.  Note that\n   even though it is not required by POSIX to double-quote the\n   redirection target in a variable (as shown above), our code does so\n   because some versions of bash issue a warning without the quotes.\n\n\t(incorrect)\n\tcat hello > world < universe\n\techo hello >$world\n\n\t(correct)\n\tcat hello >world <universe\n\techo hello >\"$world\"\n\n\n\n\n> +\t\tfi\n>  \t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n>  \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n>  \t\tif use_ext_cmd\n> @@ -102,4 +128,10 @@ else\n>  \tdone\n>  fi\n>  \n> +if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&\n> +\ttest $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL\n> +then\n> +\trm $GIT_DIFFTOOL_SKIP_TO_FILE\n> +\n> +fi\n>  exit 0\n\nWouldn't it be simpler to clear when we have reached at the end, i.e.\n\n\tif test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$GIT_DIFF_PATH_TOTAL\"\n\tthen\n\t\trm -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n\tfi\n\nThanks.\n"},{"id":"416398","messageId":"CAOLTT8TnkzU397Bnx1NdpJY-P4fYpTPzjtuzwPzLEpE_Si4Fjw@mail.gmail.com","threadId":"55116","inReplyTo":"xmqq8s7z8zsg.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] git-difftool-helper.sh: learn a new way skip to save point","fromName":"胡哲宁","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-08T08:06:20Z","receivedAt":"2021-02-08T08:06:53Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月8日周一 上午3:04写道：\n>\n> Sorry, but a not-yet-written reply went out by accident; please\n> discard it.\n>\n\nNever mind. I have synchronized different signatures of git, gmail,\ngithub.\n\n> > `git difftool` only allow us to select file to view In turn.\n>\n> Funny capitalization \"In\"?\n>\n> > If there is a commit with many files and we exit in search,\n> > We will have to traverse list again to get the file diff which\n> > we want to see.Therefore,here is a new method:every time before\n>\n> It makes it hard to lack SP after punctuation like '.', ',', and\n> ':'.\n>\n> > we view the file diff,the current coordinates will be stored in\n> > `GIT_DIR/difftool_skip_to`,this file will be deleted after\n> > successful traversing.But if an unexpected exit occurred midway,\n> > git will view the coordinates in the save point,ask user if they\n> > want continue from the last saved point.This will improve the\n> > user experience.\n>\n> I think the idea sounds good.  Admittedly I do not use difftool\n> myself, so I do not even know if and how the current end user\n> experience is so bad to require a patch like this (e.g. I do not\n> know how \"unexpected exit\" is \"unexpected\"---isn't it the end user\n> initiated action to \"quit\", or does the tool crash or something?).\n>\n\nGenerally speaking, It is the user of git manually use [Ctrl+c].\nHowever, if the program itself fails and causes the exit, I think\nthis \"save point\" can also be well recorded, because it will be\nstored before view the diff.\n\n> So I won't be the best qualified person to judge if the solution\n> presented is the best one for the problem.\n>\n>     $ git shortlog --no-merges git-diff-helper.sh\n>\n> might be a good way to find whom to ask for review and help.\n>\n\nThanks for reminding, I will -cc these authors.\n\n> Having said that, I do have one opinion on the \"skip-to\" filename.\n> I do not think it is wise to call it after the purpose you want to\n> use it for (i.e. \"I want to use it to skip to the recorded\n> position\").  Instead, if the file records \"the last visited\n> position\", it is better to name it after that\n> (e.g. \"difftool-last-position\".  If it records \"the next file to be\n> visited\", then \"difftool-next-file\" may be a good name).\n>\n\nIndeed, \"last-position\" can better express this patch function.\nI will modify it according to your suggestions.\n\n> The reason is because your first design may be to visit the file the\n> user was visiting before the \"crash\" happened, but you may later\n> want to revise the design to allow the user to say \"start at one\n> file before the file I was visiting\" etc.  The location recorded in\n> the file may still be used to decide where the code skips to when\n> restarting, but no longer exactly where the code \"skips to\".  If you\n> name it after what it is, not what it is (currently) used for, the\n> design would become clearer.\n>\n\nYou are right,But I think based on this patch, the function of \"skip to\"\nmay can be added later.\n\n>\n> > diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> > index 46af3e60b718..56ec1d38a7a1 100755\n> > --- a/git-difftool--helper.sh\n> > +++ b/git-difftool--helper.sh\n> > @@ -6,6 +6,7 @@\n> >  # Copyright (c) 2009, 2010 David Aguilar\n> >\n> >  TOOL_MODE=diff\n> > +GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n> >  . git-mergetool--lib\n> >\n> >  # difftool.prompt controls the default prompt/no-prompt behavior\n> > @@ -40,6 +41,31 @@ launch_merge_tool () {\n> >       # the user with the real $MERGED name before launching $merge_tool.\n> >       if should_prompt\n> >       then\n> > +             if test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n> > +             then\n> > +                     SAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n>\n> You can avoid the TOCTTOU race by\n>\n>                 if SAVE_POINT=$(cat 2>/dev/null \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n>                 then\n>\n> but that wouldn't probably matter in this application.\n>\n> > +                     if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n> > +                             test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n>\n> Think what happens if the file is corrupt and SAVE_POINT_NUM has (1)\n> an empty string, (2) garbage that has $IFS whitespace, (3) non\n> number, in it.  At least, quoting the variable inside double-quotes,\n> i.e. \"$SAVE_POINT_NUM\", would help an error condition reported\n> correctly at the runtime.\n\nUnderstand now.A variable with '\"\"'can show correct error usage when\nthese error conditions occur.\n\n>\n> > +                     then\n> > +                             # choice skip or not skip when check first file.\n>\n> A bit funny language.  Isn't the code clear enough without this comment?\n>\n> > +                             if test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n>\n> No need to quote the constant \"1\"; quoting the variable side may be\n> a good practice, even though I think in this codepath we know\n> GIT_DIFF_PATH_COUNTER is a well-formatted number.\n\nTruly. I will use \"DIFFTOOL_FIRST_NUM\" instread of \"1\".\n\n>\n> > +                             then\n> > +                                     printf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n>\n> \"Skip\" is probably an implementation detail that the user does not\n> have to know.  \"Do you want to start from the last file you were\n> viewing?\", perhaps?\n\nYeah. Because users may choice another totally different diff,\nI will use \"Do you want to start from the possible last file you\nwere viewing?\".\n\n>\n> > +                                     read skip_ans || return\n> > +                                     if test \"$skip_ans\" = y\n> > +                                     then\n> > +                                             return\n> > +                                     fi\n> > +                             else\n> > +                                     return\n> > +                             fi\n> > +                     fi\n> > +             fi\n> > +             # write the current coordinates to .git/difftool-skip-to\n> > +             if test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER\n>\n> Have this code been tested?  I think \"test\" is missing after the\n> \"||\", and I am not quite sure what you are trying to check with\n> \"test !$SAVE_POINT_NUM\", either.  The \"test\" utility, when given a\n> non-operator string (like \"!23\" this one is checking when the last\n> visited path was the 23rd one), returns true if the string is not an\n> empty string, and by definition a string made by appending anything\n> after \"!\" would not be empty, so the entire \"|| $SAVE_POINT_NUM ...\"\n> have been skipped in your test, I think.\n>\n\nIs indeed a mistake of mine, `test -z \"$SAVE_POINT_NUM\"` will be fine.\nShell script syntax I will pay more attention.\n\n> Is writing the current position to the file unconditionally good\n> enough?  After all, we are about to go interactive with the user, so\n> the body of this \"if\" statement won't be performance critical in any\n> sense, no?  Or is there something more subtle going on and\n> correctness of the code depends on this condition?  I cannot quite\n> tell.\n>\n> > +             then\n> > +                     echo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE\n>\n>                 echo \"$GIT_DIFF_PATH_COUNTER\" >\"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n>\n> cf. Documentation/CodingGuidelines\n>\n>  - Redirection operators should be written with space before, but no\n>    space after them.  In other words, write 'echo test >\"$file\"'\n>    instead of 'echo test> $file' or 'echo test > $file'.  Note that\n>    even though it is not required by POSIX to double-quote the\n>    redirection target in a variable (as shown above), our code does so\n>    because some versions of bash issue a warning without the quotes.\n>\n>         (incorrect)\n>         cat hello > world < universe\n>         echo hello >$world\n>\n>         (correct)\n>         cat hello >world <universe\n>         echo hello >\"$world\"\n>\n>\n>\n>\n\nOK, I will read Documentation/CodingGuidelines more times.\n\n> > +             fi\n> >               printf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n> >                       \"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n> >               if use_ext_cmd\n> > @@ -102,4 +128,10 @@ else\n> >       done\n> >  fi\n> >\n> > +if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&\n> > +     test $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL\n> > +then\n> > +     rm $GIT_DIFFTOOL_SKIP_TO_FILE\n> > +\n> > +fi\n> >  exit 0\n>\n> Wouldn't it be simpler to clear when we have reached at the end, i.e.\n>\n>         if test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$GIT_DIFF_PATH_TOTAL\"\n>         then\n>                 rm -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n>         fi\n>\n> Thanks.\n\nThanks for the advice and correct, Junio.\n"},{"id":"416418","messageId":"pull.870.v2.git.1612803744188.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.git.1612711153591.gitgitgadget@gmail.com","subject":"[PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-08T17:02:23Z","receivedAt":"2021-02-08T17:10:33Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view in turn.\nIf there is a commit with many files and we exit in the search,\nWe will have to traverse list again to get the file diff which\nwe want to see. Therefore, here is a new method: every time before\nwe view the file diff, the current coordinates will be stored in\n`GIT_DIR/difftool-last-position`, this file will be deleted after\nsuccessful traversing. But if an unexpected exit occurred midway or\nusers similar to using \"ctrl+c\" kill the process,and the user wants\nto redo the same `git difftoool`, git will view the coordinates in\nthe save point, ask user if they want continue from the last position.\nThis will improve the user experience.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    git-difftool-helper.sh: learn a new way skip to save point\n    \n    git user may should travel the diff list to choice file diff to view, if\n    they exit in midway,they must travel it again. By saving current\n    coordinates in GIT_DIR/difftool-last-position method, provides a\n    possibility for this user-friendly solution.\n    \n    this patch's origin discuss is here:\n    https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n    \n    Thanks!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\nRange-diff vs v1:\n\n 1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point\n     @@ Metadata\n      Author: ZheNing Hu <adlternative@gmail.com>\n      \n       ## Commit message ##\n     -    git-difftool-helper.sh: learn a new way skip to save point\n     +    git-difftool-helper.sh: learn a new way go back to last save point\n      \n     -    `git difftool` only allow us to select file to view In turn.\n     -    If there is a commit with many files and we exit in search,\n     +    `git difftool` only allow us to select file to view in turn.\n     +    If there is a commit with many files and we exit in the search,\n          We will have to traverse list again to get the file diff which\n     -    we want to see.Therefore,here is a new method:every time before\n     -    we view the file diff,the current coordinates will be stored in\n     -    `GIT_DIR/difftool_skip_to`,this file will be deleted after\n     -    successful traversing.But if an unexpected exit occurred midway,\n     -    git will view the coordinates in the save point,ask user if they\n     -    want continue from the last saved point.This will improve the\n     -    user experience.\n     +    we want to see. Therefore, here is a new method: every time before\n     +    we view the file diff, the current coordinates will be stored in\n     +    `GIT_DIR/difftool-last-position`, this file will be deleted after\n     +    successful traversing. But if an unexpected exit occurred midway or\n     +    users similar to using \"ctrl+c\" kill the process,and the user wants\n     +    to redo the same `git difftoool`, git will view the coordinates in\n     +    the save point, ask user if they want continue from the last position.\n     +    This will improve the user experience.\n      \n          Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n      \n     @@ git-difftool--helper.sh\n       # Copyright (c) 2009, 2010 David Aguilar\n       \n       TOOL_MODE=diff\n     -+GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n     ++GIT_DIFFTOOL_LAST_POSITION=\"$GIT_DIR/difftool-last-position\"\n     ++DIFFTOOL_FIRST_NUM=\"1\"\n       . git-mergetool--lib\n       \n       # difftool.prompt controls the default prompt/no-prompt behavior\n     @@ git-difftool--helper.sh: launch_merge_tool () {\n       \t# the user with the real $MERGED name before launching $merge_tool.\n       \tif should_prompt\n       \tthen\n     -+\t\tif test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n     ++\t\tif test -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n      +\t\tthen\n     -+\t\t\tSAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n     -+\t\t\tif test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n     -+\t\t\t\ttest $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n     ++\t\t\tif SAVE_POINT_NUM=$(cat 2>/dev/null \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n     ++\t\t\t\ttest \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n     ++\t\t\t\t\ttest \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n      +\t\t\tthen\n     -+\t\t\t\t# choice skip or not skip when check first file.\n     -+\t\t\t\tif test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n     ++\t\t\t\tif test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$DIFFTOOL_FIRST_NUM\"\n      +\t\t\t\tthen\n     -+\t\t\t\t\tprintf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n     ++\t\t\t\t\tprintf \"Do you want to start from the possible last file you were viewing? [Y/n]?\"\n      +\t\t\t\t\tread skip_ans || return\n      +\t\t\t\t\tif test \"$skip_ans\" = y\n      +\t\t\t\t\tthen\n     @@ git-difftool--helper.sh: launch_merge_tool () {\n      +\t\t\t\tfi\n      +\t\t\tfi\n      +\t\tfi\n     -+\t\t# write the current coordinates to .git/difftool-skip-to\n     -+\t\tif test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER\n     ++\t\tif test -z \"$SAVE_POINT_NUM\" ||\n     ++\t\t\ttest \"$SAVE_POINT_NUM\" -ne \"$GIT_DIFF_PATH_COUNTER\"\n      +\t\tthen\n     -+\t\t\techo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE\n     ++\t\t\techo \"$GIT_DIFF_PATH_COUNTER\" >\"$GIT_DIFFTOOL_LAST_POSITION\"\n      +\t\tfi\n       \t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n       \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n     @@ git-difftool--helper.sh: else\n       \tdone\n       fi\n       \n     -+if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&\n     -+\ttest $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL\n     ++if test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$GIT_DIFF_PATH_TOTAL\"\n      +then\n     -+\trm $GIT_DIFFTOOL_SKIP_TO_FILE\n     ++\trm -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n      +\n      +fi\n       exit 0\n\n\n git-difftool--helper.sh | 31 +++++++++++++++++++++++++++++++\n 1 file changed, 31 insertions(+)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 46af3e60b718..a01aa7c9d551 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -6,6 +6,8 @@\n # Copyright (c) 2009, 2010 David Aguilar\n \n TOOL_MODE=diff\n+GIT_DIFFTOOL_LAST_POSITION=\"$GIT_DIR/difftool-last-position\"\n+DIFFTOOL_FIRST_NUM=\"1\"\n . git-mergetool--lib\n \n # difftool.prompt controls the default prompt/no-prompt behavior\n@@ -40,6 +42,30 @@ launch_merge_tool () {\n \t# the user with the real $MERGED name before launching $merge_tool.\n \tif should_prompt\n \tthen\n+\t\tif test -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n+\t\tthen\n+\t\t\tif SAVE_POINT_NUM=$(cat 2>/dev/null \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n+\t\t\t\ttest \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n+\t\t\t\t\ttest \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n+\t\t\tthen\n+\t\t\t\tif test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$DIFFTOOL_FIRST_NUM\"\n+\t\t\t\tthen\n+\t\t\t\t\tprintf \"Do you want to start from the possible last file you were viewing? [Y/n]?\"\n+\t\t\t\t\tread skip_ans || return\n+\t\t\t\t\tif test \"$skip_ans\" = y\n+\t\t\t\t\tthen\n+\t\t\t\t\t\treturn\n+\t\t\t\t\tfi\n+\t\t\t\telse\n+\t\t\t\t\treturn\n+\t\t\t\tfi\n+\t\t\tfi\n+\t\tfi\n+\t\tif test -z \"$SAVE_POINT_NUM\" ||\n+\t\t\ttest \"$SAVE_POINT_NUM\" -ne \"$GIT_DIFF_PATH_COUNTER\"\n+\t\tthen\n+\t\t\techo \"$GIT_DIFF_PATH_COUNTER\" >\"$GIT_DIFFTOOL_LAST_POSITION\"\n+\t\tfi\n \t\tprintf \"\\nViewing (%s/%s): '%s'\\n\" \"$GIT_DIFF_PATH_COUNTER\" \\\n \t\t\t\"$GIT_DIFF_PATH_TOTAL\" \"$MERGED\"\n \t\tif use_ext_cmd\n@@ -102,4 +128,9 @@ else\n \tdone\n fi\n \n+if test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$GIT_DIFF_PATH_TOTAL\"\n+then\n+\trm -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n+\n+fi\n exit 0\n\nbase-commit: e6362826a0409539642a5738db61827e5978e2e4\n-- \ngitgitgadget\n"},{"id":"416441","messageId":"xmqqczxa5nct.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.v2.git.1612803744188.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-08T20:13:54Z","receivedAt":"2021-02-08T20:16:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\n> index 46af3e60b718..a01aa7c9d551 100755\n> --- a/git-difftool--helper.sh\n> +++ b/git-difftool--helper.sh\n> @@ -6,6 +6,8 @@\n>  # Copyright (c) 2009, 2010 David Aguilar\n>  \n>  TOOL_MODE=diff\n> +GIT_DIFFTOOL_LAST_POSITION=\"$GIT_DIR/difftool-last-position\"\n> +DIFFTOOL_FIRST_NUM=\"1\"\n\nDo we need this constant?  I do not think it makes the resulting\ncode easier to follow.\n\n>  . git-mergetool--lib\n>  \n>  # difftool.prompt controls the default prompt/no-prompt behavior\n> @@ -40,6 +42,30 @@ launch_merge_tool () {\n>  \t# the user with the real $MERGED name before launching $merge_tool.\n>  \tif should_prompt\n>  \tthen\n> +\t\tif test -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n> +\t\tthen\n> +\t\t\tif SAVE_POINT_NUM=$(cat 2>/dev/null \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n> +\t\t\t\ttest \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n> +\t\t\t\t\ttest \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n> +\t\t\tthen\n\nNo need to push the subsequent lines that far to the right,\nespecially when your variable names are already overly long.  It\njust makes things harder to read.\n\n\t\t\tif test -r \"$GIT_DIFFTOOL_LAST_POSITION\" &&\n\t\t\t   SAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n\t\t\t   test \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n\t\t\t   test \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n\t\t\tthen\n\n> +\t\t\t\tif test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$DIFFTOOL_FIRST_NUM\"\n> +\t\t\t\tthen\n> +\t\t\t\t\tprintf \"Do you want to start from the possible last file you were viewing? [Y/n]?\"\n\nWhere does that \"possible\" come from?  If the reason is \"We might\nhave miscomputed or misrecorded an incorrect last position\", we\nprobably should work harder to make sure we don't ;-)\n\nAt this point in the code, do we have the _name_ of the file we are\ngoing to skip to readily available, or do we actually need to seek\nto that position before we can find it out?\n\n\tYou were looking at 'hello-world.txt' the last time.\n\tDo you want to restart from there [Y/n]?\n\nwould be far easier to answer for an end-user than an unspecified\n\"possible last file\".\n\n> +\t\tfi\n> +\t\tif test -z \"$SAVE_POINT_NUM\" ||\n> +\t\t\ttest \"$SAVE_POINT_NUM\" -ne \"$GIT_DIFF_PATH_COUNTER\"\n\nDitto about indentation.\n\nIs this behaviour something we can write a test for in\nt/t7800-difftool.sh, by the way?\n\nOther than these, i.e. (cosmetic) indentation and overly long lines\n(ui) giving filename is easier to work with for the users and (dev)\nlack of tests, looking quite good.\n\nThanks.\n"},{"id":"416449","messageId":"CAJDDKr4AyxuS-MQ+62XGnK4UvJ+cFDdnMwOK1GSn-oiFrWoxyw@mail.gmail.com","threadId":"55116","inReplyTo":"pull.870.v2.git.1612803744188.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2021-02-08T22:15:28Z","receivedAt":"2021-02-08T22:16:48Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"(cc'd Ryan since the thread involving him was mentioned in the commit message)\n\nOn Mon, Feb 8, 2021 at 9:02 AM ZheNing Hu via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> `git difftool` only allow us to select file to view in turn.\n> If there is a commit with many files and we exit in the search,\n> We will have to traverse list again to get the file diff which\n> we want to see. Therefore, here is a new method: every time before\n> we view the file diff, the current coordinates will be stored in\n> `GIT_DIR/difftool-last-position`, this file will be deleted after\n> successful traversing. But if an unexpected exit occurred midway or\n> users similar to using \"ctrl+c\" kill the process,and the user wants\n> to redo the same `git difftoool`, git will view the coordinates in\n> the save point, ask user if they want continue from the last position.\n> This will improve the user experience.\n>\n> Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> ---\n>     git-difftool-helper.sh: learn a new way skip to save point\n>\n>     git user may should travel the diff list to choice file diff to view, if\n>     they exit in midway,they must travel it again. By saving current\n>     coordinates in GIT_DIR/difftool-last-position method, provides a\n>     possibility for this user-friendly solution.\n>\n>     this patch's origin discuss is here:\n>     https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n>\n>     Thanks!\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/870\n>\n> Range-diff vs v1:\n>\n>  1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point\n>      @@ Metadata\n>       Author: ZheNing Hu <adlternative@gmail.com>\n>\n>        ## Commit message ##\n>      -    git-difftool-helper.sh: learn a new way skip to save point\n>      +    git-difftool-helper.sh: learn a new way go back to last save point\n>\n>      -    `git difftool` only allow us to select file to view In turn.\n>      -    If there is a commit with many files and we exit in search,\n>      +    `git difftool` only allow us to select file to view in turn.\n>      +    If there is a commit with many files and we exit in the search,\n>           We will have to traverse list again to get the file diff which\n>      -    we want to see.Therefore,here is a new method:every time before\n>      -    we view the file diff,the current coordinates will be stored in\n>      -    `GIT_DIR/difftool_skip_to`,this file will be deleted after\n>      -    successful traversing.But if an unexpected exit occurred midway,\n>      -    git will view the coordinates in the save point,ask user if they\n>      -    want continue from the last saved point.This will improve the\n>      -    user experience.\n>      +    we want to see. Therefore, here is a new method: every time before\n>      +    we view the file diff, the current coordinates will be stored in\n>      +    `GIT_DIR/difftool-last-position`, this file will be deleted after\n>      +    successful traversing. But if an unexpected exit occurred midway or\n>      +    users similar to using \"ctrl+c\" kill the process,and the user wants\n>      +    to redo the same `git difftoool`, git will view the coordinates in\n>      +    the save point, ask user if they want continue from the last position.\n>      +    This will improve the user experience.\n>\n>           Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n>\n>      @@ git-difftool--helper.sh\n>        # Copyright (c) 2009, 2010 David Aguilar\n>\n>        TOOL_MODE=diff\n>      -+GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n>      ++GIT_DIFFTOOL_LAST_POSITION=\"$GIT_DIR/difftool-last-position\"\n>      ++DIFFTOOL_FIRST_NUM=\"1\"\n>        . git-mergetool--lib\n>\n>        # difftool.prompt controls the default prompt/no-prompt behavior\n>      @@ git-difftool--helper.sh: launch_merge_tool () {\n>         # the user with the real $MERGED name before launching $merge_tool.\n>         if should_prompt\n>         then\n>      -+         if test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n>      ++         if test -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n>       +         then\n>      -+                 SAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n>      -+                 if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n>      -+                         test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n>      ++                 if SAVE_POINT_NUM=$(cat 2>/dev/null \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n>      ++                         test \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n>      ++                                 test \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n>       +                 then\n>      -+                         # choice skip or not skip when check first file.\n>      -+                         if test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n>      ++                         if test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$DIFFTOOL_FIRST_NUM\"\n>       +                         then\n>      -+                                 printf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n>      ++                                 printf \"Do you want to start from the possible last file you were viewing? [Y/n]?\"\n>       +                                 read skip_ans || return\n>       +                                 if test \"$skip_ans\" = y\n>       +                                 then\n>      @@ git-difftool--helper.sh: launch_merge_tool () {\n>       +                         fi\n>       +                 fi\n>       +         fi\n\n\nSimilar to Junio's question about, \"where does this possible come\nfrom?\", I wasn't able to make out the behavior in the following\nsituation.\n\nWhat about when the user switches branches or specifies a pathspec on\nthe command-line or some other avenue that ends up with the number of\nfiles to diff being very different than the last difftool invocation?\n\nWill difftool, for example, skip over a smaller set of files on\ninvocation 2 if invocation 1 involved many files and we exited out\nwith a counter number that is very high?\n\nOne thing that's not too good about having state files in .git/ is\nthat they're global data and we also have to think about, \"what if the\nuser has multiple difftools running?\" and those kind of complexities.\n\nI don't want this to seem like I'm trying to be dismissive of this\nfeature which does seem like a useful thing in general, so I'll try to\ncome up with an alternative interface that is slightly more general\nbut a admittedly a little bit more cumbersome because it's not as\nautomatic.\n\nWhat if instead of global state, maybe the user could specify a path\nthat difftool could skip forward to?   For example, we could teach\ndifftool to resume by telling it where we last left off:\n\n   git difftool --resume-from=foo/bar099.txt\n\nThen we don't need the global counter state file?\n\n\nFinally, I'm going to plug what I believe to be the right tool for the\njob here.  Have you tried git cola?[1]  Difftool is tightly\nintegrated, and the UI is such that you can trivially choose any of\nthe modified/staged files and difftool them by using the Ctrl-d\nhotkey.\n\nhttps://github.com/git-cola/git-cola/\n\nCola is purpose-built for driving difftool, and for interactive\nstaging, so not mentioning it in the context of wanting a better UI\nfor difftool would be a disservice to difftool users.\n-- \nDavid\n"},{"id":"416459","messageId":"xmqq4kim3zhq.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"CAJDDKr4AyxuS-MQ+62XGnK4UvJ+cFDdnMwOK1GSn-oiFrWoxyw@mail.gmail.com","subject":"Re: [PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-08T23:34:41Z","receivedAt":"2021-02-08T23:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> What if instead of global state, maybe the user could specify a path\n> that difftool could skip forward to?   For example, we could teach\n> difftool to resume by telling it where we last left off:\n>\n>    git difftool --resume-from=foo/bar099.txt\n>\n> Then we don't need the global counter state file?\n\nDoes it have to be the second and subsequent invocation to pass the\nnew \"resume-from\" option?  As we do not have \"global\" state, I would\npresume that we do not even know if it is the first invocation, so\nperhaps a better name would be \"--start-from=$pathname\"?\n\n> Finally, I'm going to plug what I believe to be the right tool for the\n> job here.  Have you tried git cola?[1]  Difftool is tightly\n> integrated, and the UI is such that you can trivially choose any of\n> the modified/staged files and difftool them by using the Ctrl-d\n> hotkey.\n>\n> https://github.com/git-cola/git-cola/\n>\n> Cola is purpose-built for driving difftool, and for interactive\n> staging, so not mentioning it in the context of wanting a better UI\n> for difftool would be a disservice to difftool users.\n\n;-)\n"},{"id":"416471","messageId":"CAOLTT8QBmP8AzTBFMqhAU4yHrY1cvusv5EiXY0Qh402m_wTgoQ@mail.gmail.com","threadId":"55116","inReplyTo":"CAJDDKr4AyxuS-MQ+62XGnK4UvJ+cFDdnMwOK1GSn-oiFrWoxyw@mail.gmail.com","subject":"Re: [PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"胡哲宁","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-09T06:04:40Z","receivedAt":"2021-02-09T06:03:26Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"David Aguilar <davvid@gmail.com> 于2021年2月9日周二 上午6:16写道：\n>\n> (cc'd Ryan since the thread involving him was mentioned in the commit message)\n>\n> On Mon, Feb 8, 2021 at 9:02 AM ZheNing Hu via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> >\n> > From: ZheNing Hu <adlternative@gmail.com>\n> >\n> > `git difftool` only allow us to select file to view in turn.\n> > If there is a commit with many files and we exit in the search,\n> > We will have to traverse list again to get the file diff which\n> > we want to see. Therefore, here is a new method: every time before\n> > we view the file diff, the current coordinates will be stored in\n> > `GIT_DIR/difftool-last-position`, this file will be deleted after\n> > successful traversing. But if an unexpected exit occurred midway or\n> > users similar to using \"ctrl+c\" kill the process,and the user wants\n> > to redo the same `git difftoool`, git will view the coordinates in\n> > the save point, ask user if they want continue from the last position.\n> > This will improve the user experience.\n> >\n> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> > ---\n> >     git-difftool-helper.sh: learn a new way skip to save point\n> >\n> >     git user may should travel the diff list to choice file diff to view, if\n> >     they exit in midway,they must travel it again. By saving current\n> >     coordinates in GIT_DIR/difftool-last-position method, provides a\n> >     possibility for this user-friendly solution.\n> >\n> >     this patch's origin discuss is here:\n> >     https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n> >\n> >     Thanks!\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/870\n> >\n> > Range-diff vs v1:\n> >\n> >  1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point\n> >      @@ Metadata\n> >       Author: ZheNing Hu <adlternative@gmail.com>\n> >\n> >        ## Commit message ##\n> >      -    git-difftool-helper.sh: learn a new way skip to save point\n> >      +    git-difftool-helper.sh: learn a new way go back to last save point\n> >\n> >      -    `git difftool` only allow us to select file to view In turn.\n> >      -    If there is a commit with many files and we exit in search,\n> >      +    `git difftool` only allow us to select file to view in turn.\n> >      +    If there is a commit with many files and we exit in the search,\n> >           We will have to traverse list again to get the file diff which\n> >      -    we want to see.Therefore,here is a new method:every time before\n> >      -    we view the file diff,the current coordinates will be stored in\n> >      -    `GIT_DIR/difftool_skip_to`,this file will be deleted after\n> >      -    successful traversing.But if an unexpected exit occurred midway,\n> >      -    git will view the coordinates in the save point,ask user if they\n> >      -    want continue from the last saved point.This will improve the\n> >      -    user experience.\n> >      +    we want to see. Therefore, here is a new method: every time before\n> >      +    we view the file diff, the current coordinates will be stored in\n> >      +    `GIT_DIR/difftool-last-position`, this file will be deleted after\n> >      +    successful traversing. But if an unexpected exit occurred midway or\n> >      +    users similar to using \"ctrl+c\" kill the process,and the user wants\n> >      +    to redo the same `git difftoool`, git will view the coordinates in\n> >      +    the save point, ask user if they want continue from the last position.\n> >      +    This will improve the user experience.\n> >\n> >           Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> >\n> >      @@ git-difftool--helper.sh\n> >        # Copyright (c) 2009, 2010 David Aguilar\n> >\n> >        TOOL_MODE=diff\n> >      -+GIT_DIFFTOOL_SKIP_TO_FILE=\"$GIT_DIR/difftool-skip-to\"\n> >      ++GIT_DIFFTOOL_LAST_POSITION=\"$GIT_DIR/difftool-last-position\"\n> >      ++DIFFTOOL_FIRST_NUM=\"1\"\n> >        . git-mergetool--lib\n> >\n> >        # difftool.prompt controls the default prompt/no-prompt behavior\n> >      @@ git-difftool--helper.sh: launch_merge_tool () {\n> >         # the user with the real $MERGED name before launching $merge_tool.\n> >         if should_prompt\n> >         then\n> >      -+         if test -f \"$GIT_DIFFTOOL_SKIP_TO_FILE\"\n> >      ++         if test -f \"$GIT_DIFFTOOL_LAST_POSITION\"\n> >       +         then\n> >      -+                 SAVE_POINT_NUM=$(cat \"$GIT_DIFFTOOL_SKIP_TO_FILE\")\n> >      -+                 if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&\n> >      -+                         test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER\n> >      ++                 if SAVE_POINT_NUM=$(cat 2>/dev/null \"$GIT_DIFFTOOL_LAST_POSITION\") &&\n> >      ++                         test \"$SAVE_POINT_NUM\" -le \"$GIT_DIFF_PATH_TOTAL\" &&\n> >      ++                                 test \"$SAVE_POINT_NUM\" -gt \"$GIT_DIFF_PATH_COUNTER\"\n> >       +                 then\n> >      -+                         # choice skip or not skip when check first file.\n> >      -+                         if test $GIT_DIFF_PATH_COUNTER -eq \"1\"\n> >      ++                         if test \"$GIT_DIFF_PATH_COUNTER\" -eq \"$DIFFTOOL_FIRST_NUM\"\n> >       +                         then\n> >      -+                                 printf \"do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?\"\n> >      ++                                 printf \"Do you want to start from the possible last file you were viewing? [Y/n]?\"\n> >       +                                 read skip_ans || return\n> >       +                                 if test \"$skip_ans\" = y\n> >       +                                 then\n> >      @@ git-difftool--helper.sh: launch_merge_tool () {\n> >       +                         fi\n> >       +                 fi\n> >       +         fi\n>\n>\n> Similar to Junio's question about, \"where does this possible come\n> from?\", I wasn't able to make out the behavior in the following\n> situation.\n>\n> What about when the user switches branches or specifies a pathspec on\n> the command-line or some other avenue that ends up with the number of\n> files to diff being very different than the last difftool invocation?\n>\n> Will difftool, for example, skip over a smaller set of files on\n> invocation 2 if invocation 1 involved many files and we exited out\n> with a counter number that is very high?\n>\nThis is what I worry about.\n> One thing that's not too good about having state files in .git/ is\n> that they're global data and we also have to think about, \"what if the\n> user has multiple difftools running?\" and those kind of complexities.\n>\nI admit that I did not consider the situation where multiple `git difftool`\nprocesses are going on at the same time.\n> I don't want this to seem like I'm trying to be dismissive of this\n> feature which does seem like a useful thing in general, so I'll try to\n> come up with an alternative interface that is slightly more general\n> but a admittedly a little bit more cumbersome because it's not as\n> automatic.\n>\n> What if instead of global state, maybe the user could specify a path\n> that difftool could skip forward to?   For example, we could teach\n> difftool to resume by telling it where we last left off:\n>\n>    git difftool --resume-from=foo/bar099.txt\n>\n> Then we don't need the global counter state file?\n>\nWonderful idea.But as Junio said, there may be no global state support,\n`start-from` will be more applicable.\n>\n> Finally, I'm going to plug what I believe to be the right tool for the\n> job here.  Have you tried git cola?[1]  Difftool is tightly\n> integrated, and the UI is such that you can trivially choose any of\n> the modified/staged files and difftool them by using the Ctrl-d\n> hotkey.\n>\n> https://github.com/git-cola/git-cola/\n>\n> Cola is purpose-built for driving difftool, and for interactive\n> staging, so not mentioning it in the context of wanting a better UI\n> for difftool would be a disservice to difftool users.\nI saw the difftool UI of git-cola, and it is great to view the differences\nby selecting files.I have been using vscode's git plugin before, and it\nworks well tool.\n> --\n> David\nThanks for help!\n--\nZheNing Hu\n"},{"id":"416472","messageId":"CAOLTT8StBRZcgLopTQ5i9pRbCbbGochv5fTfMPVF3xt7BaG8iQ@mail.gmail.com","threadId":"55116","inReplyTo":"xmqq4kim3zhq.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] git-difftool-helper.sh: learn a new way go back to last save point","fromName":"胡哲宁","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-09T06:19:13Z","receivedAt":"2021-02-09T06:17:47Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月9日周二 上午7:34写道：\n>\n> David Aguilar <davvid@gmail.com> writes:\n>\n> > What if instead of global state, maybe the user could specify a path\n> > that difftool could skip forward to?   For example, we could teach\n> > difftool to resume by telling it where we last left off:\n> >\n> >    git difftool --resume-from=foo/bar099.txt\n> >\n> > Then we don't need the global counter state file?\n>\n> Does it have to be the second and subsequent invocation to pass the\n> new \"resume-from\" option?  As we do not have \"global\" state, I would\n> presume that we do not even know if it is the first invocation, so\n> perhaps a better name would be \"--start-from=$pathname\"?\n>\nThank you for your thinking, I agree with your point of view.\nAs you said before: an accurate file name may be more suitable\nfor users than `possible last file`.\nHowever, without the support of the global `difftool-save-point`,\nit may not be possible to know the last exit point of the user.\nEven if the global state is allowed, it may need to do more work\nto avoid the competition for the global state under multiple\nprocesses.\nSo \"--start-from=$pathname\" is more suitable to provide users\nwith a way to quickly index to specified files.\nThen I shift the front and start thinking about how to realize it! :)\n> > Finally, I'm going to plug what I believe to be the right tool for the\n> > job here.  Have you tried git cola?[1]  Difftool is tightly\n> > integrated, and the UI is such that you can trivially choose any of\n> > the modified/staged files and difftool them by using the Ctrl-d\n> > hotkey.\n> >\n> > https://github.com/git-cola/git-cola/\n> >\n> > Cola is purpose-built for driving difftool, and for interactive\n> > staging, so not mentioning it in the context of wanting a better UI\n> > for difftool would be a disservice to difftool users.\n>\n> ;-)\nThanks again!\n"},{"id":"416512","messageId":"pull.870.v3.git.1612884654078.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v2.git.1612803744188.gitgitgadget@gmail.com","subject":"[PATCH v3] difftool.c: learn a new way start from specified file","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-09T15:30:53Z","receivedAt":"2021-02-09T15:31:56Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view in turn.\nIf there is a commit with many files and we exit in the search,\nWe will have to traverse list again to get the file diff which\nwe want to see. Therefore, here is a new method: user can use\n`git difftool --start-from=<filename>` to start viewing from\nthe specified file. This will improve the user experience.\nAt the same time, turn bit field constants into bit shift format\nin `diff.h`.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    difftool.c: learn a new way start at specified file\n    \n    git user may should travel the diff list to choice file diff to view, if\n    they exit in midway,they must travel it again. By starting from the\n    specified file method, provides a possibility for this user-friendly\n    solution.\n    \n    this patch's origin discuss is here:\n    https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n    \n    Maybe this patch is more like skip to in Junio's original thread than\n    the previous versions.\n    \n    Thanks!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\nRange-diff vs v2:\n\n 1:  2468eaff322b < -:  ------------ git-difftool-helper.sh: learn a new way go back to last save point\n -:  ------------ > 1:  29fc6b4bc08f difftool.c: learn a new way start from specified file\n\n\n Documentation/git-difftool.txt |  3 +++\n builtin/difftool.c             |  7 ++++++-\n diff.c                         |  9 +++++++++\n diff.h                         | 20 ++++++++++----------\n t/t7800-difftool.sh            | 12 ++++++++++++\n 5 files changed, 40 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 484c485fd06c..552be097dfea 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -34,6 +34,9 @@ OPTIONS\n \tThis is the default behaviour; the option is provided to\n \toverride any configuration settings.\n \n+--start-from::\n+\tStart viewing diff from the specified file.\n+\n -t <tool>::\n --tool=<tool>::\n \tUse the diff tool specified by <tool>.  Valid values include\ndiff --git a/builtin/difftool.c b/builtin/difftool.c\nindex 6e18e623fddf..67d2befa1210 100644\n--- a/builtin/difftool.c\n+++ b/builtin/difftool.c\n@@ -690,7 +690,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n {\n \tint use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,\n \t    tool_help = 0, no_index = 0;\n-\tstatic char *difftool_cmd = NULL, *extcmd = NULL;\n+\tstatic char *difftool_cmd = NULL, *extcmd = NULL, *start_file = NULL;\n \tstruct option builtin_difftool_options[] = {\n \t\tOPT_BOOL('g', \"gui\", &use_gui_tool,\n \t\t\t N_(\"use `diff.guitool` instead of `diff.tool`\")),\n@@ -714,6 +714,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING('x', \"extcmd\", &extcmd, N_(\"command\"),\n \t\t\t   N_(\"specify a custom command for viewing diffs\")),\n \t\tOPT_ARGUMENT(\"no-index\", &no_index, N_(\"passed to `diff`\")),\n+\t\tOPT_STRING(0, \"start-from\", &start_file, N_(\"start-from\"),\n+\t\t\t   N_(\"start viewing diff from the specified file\")),\n \t\tOPT_END()\n \t};\n \n@@ -724,6 +726,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n \t\t\t     builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |\n \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n \n+\tif (start_file)\n+\t\tsetenv(\"START_FILE\", start_file, 1);\n+\n \tif (tool_help)\n \t\treturn print_tool_help();\n \ndiff --git a/diff.c b/diff.c\nindex 69e3bc00ed8f..cdad26f99063 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4249,6 +4249,7 @@ static void run_external_diff(const char *pgm,\n \t\t\t      const char *xfrm_msg,\n \t\t\t      struct diff_options *o)\n {\n+\tconst char *start_file = NULL;\n \tstruct strvec argv = STRVEC_INIT;\n \tstruct strvec env = STRVEC_INIT;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n@@ -4272,9 +4273,17 @@ static void run_external_diff(const char *pgm,\n \n \tdiff_free_filespec_data(one);\n \tdiff_free_filespec_data(two);\n+\n+\tstart_file = xstrdup_or_null(getenv(\"START_FILE\"));\n+\tif (start_file) {\n+\t\tif (strcmp(start_file, name))\n+\t\t\tgoto finish;\n+\t\tunsetenv(\"START_FILE\");\n+\t}\n \tif (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v))\n \t\tdie(_(\"external diff died, stopping at %s\"), name);\n \n+finish:\n \tremove_tempfile();\n \tstrvec_clear(&argv);\n \tstrvec_clear(&env);\ndiff --git a/diff.h b/diff.h\nindex 2ff2b1c7f2ca..f67c43f5af95 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -86,18 +86,18 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n \n typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n \n-#define DIFF_FORMAT_RAW\t\t0x0001\n-#define DIFF_FORMAT_DIFFSTAT\t0x0002\n-#define DIFF_FORMAT_NUMSTAT\t0x0004\n-#define DIFF_FORMAT_SUMMARY\t0x0008\n-#define DIFF_FORMAT_PATCH\t0x0010\n-#define DIFF_FORMAT_SHORTSTAT\t0x0020\n-#define DIFF_FORMAT_DIRSTAT\t0x0040\n+#define DIFF_FORMAT_RAW\t\t(1U<<0)\n+#define DIFF_FORMAT_DIFFSTAT\t(1U<<1)\n+#define DIFF_FORMAT_NUMSTAT\t(1U<<2)\n+#define DIFF_FORMAT_SUMMARY\t(1U<<3)\n+#define DIFF_FORMAT_PATCH\t(1U<<4)\n+#define DIFF_FORMAT_SHORTSTAT\t(1U<<5)\n+#define DIFF_FORMAT_DIRSTAT\t(1U<<6)\n \n /* These override all above */\n-#define DIFF_FORMAT_NAME\t0x0100\n-#define DIFF_FORMAT_NAME_STATUS\t0x0200\n-#define DIFF_FORMAT_CHECKDIFF\t0x0400\n+#define DIFF_FORMAT_NAME\t(1U<<8)\n+#define DIFF_FORMAT_NAME_STATUS\t(1U<<9)\n+#define DIFF_FORMAT_CHECKDIFF\t(1U<<10)\n \n /* Same as output_format = 0 but we know that -s flag was given\n  * and we should not give default value to output_format.\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 9662abc1e784..74baac96a23f 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -762,4 +762,16 @@ test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n \ttest_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n '\n \n+test_expect_success 'difftool --start-from' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\techo 1 >1 &&\n+\techo 2 >2 &&\n+\techo 3 >3 &&\n+\tgit add 1 2 3 &&\n+\tgit commit -a -m \"123\" &&\n+\tgit difftool --start-from=\"2\" HEAD^ 2>&1 >output &&\n+\ttest_line_count = 4 output\n+'\n+\n test_done\n\nbase-commit: e6362826a0409539642a5738db61827e5978e2e4\n-- \ngitgitgadget\n"},{"id":"416544","messageId":"xmqqeehp2jis.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.v3.git.1612884654078.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] difftool.c: learn a new way start from specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-09T18:17:15Z","receivedAt":"2021-02-09T18:22:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\n> index 484c485fd06c..552be097dfea 100644\n> --- a/Documentation/git-difftool.txt\n> +++ b/Documentation/git-difftool.txt\n> @@ -34,6 +34,9 @@ OPTIONS\n>  \tThis is the default behaviour; the option is provided to\n>  \toverride any configuration settings.\n>  \n> +--start-from::\n> +\tStart viewing diff from the specified file.\n> +\n\nThere is nothing that specifies a file in the above ;-)\n\n\t--start-from=<file>::\n\t\tStart viewing ...\n\nThis is even more important as SYNOPSIS section of this manual page\ndoes not duplicate the list of options and their arguments.\n\n>  -t <tool>::\n>  --tool=<tool>::\n>  \tUse the diff tool specified by <tool>.  Valid values include\n\nThere are many things I dislike about this patch, but do not take it\nas a personal attack.  I'll try to suggest an alternative at the end,\nbut read along.\n\n> diff --git a/builtin/difftool.c b/builtin/difftool.c\n> index 6e18e623fddf..67d2befa1210 100644\n> --- a/builtin/difftool.c\n> +++ b/builtin/difftool.c\n> @@ -690,7 +690,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,\n>  \t    tool_help = 0, no_index = 0;\n> -\tstatic char *difftool_cmd = NULL, *extcmd = NULL;\n> +\tstatic char *difftool_cmd = NULL, *extcmd = NULL, *start_file = NULL;\n>  \tstruct option builtin_difftool_options[] = {\n>  \t\tOPT_BOOL('g', \"gui\", &use_gui_tool,\n>  \t\t\t N_(\"use `diff.guitool` instead of `diff.tool`\")),\n> @@ -714,6 +714,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n>  \t\tOPT_STRING('x', \"extcmd\", &extcmd, N_(\"command\"),\n>  \t\t\t   N_(\"specify a custom command for viewing diffs\")),\n>  \t\tOPT_ARGUMENT(\"no-index\", &no_index, N_(\"passed to `diff`\")),\n> +\t\tOPT_STRING(0, \"start-from\", &start_file, N_(\"start-from\"),\n> +\t\t\t   N_(\"start viewing diff from the specified file\")),\n>  \t\tOPT_END()\n>  \t};\n\nThis may be a good UI to \"difftool\".\n\n> @@ -724,6 +726,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n>  \t\t\t     builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |\n>  \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n>  \n> +\tif (start_file)\n> +\t\tsetenv(\"START_FILE\", start_file, 1);\n\nUnacceptable.  Nothing gives Git the right to squat on such a\ngeneric name, and there is no hint that it is used to specify the\nstarting point of what operation.  In addition, I do not see a good\nreason why we need to use an environment variable in the first\nplace.  We run \"diff\" as an external process, with GIT_EXTERNAL_DIFF\nenvironment pointing back at us, no?  This information should be\npassed from its command line.\n\n> diff --git a/diff.c b/diff.c\n> index 69e3bc00ed8f..cdad26f99063 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -4249,6 +4249,7 @@ static void run_external_diff(const char *pgm,\n>  \t\t\t      const char *xfrm_msg,\n>  \t\t\t      struct diff_options *o)\n>  {\n> +\tconst char *start_file = NULL;\n>  \tstruct strvec argv = STRVEC_INIT;\n>  \tstruct strvec env = STRVEC_INIT;\n>  \tstruct diff_queue_struct *q = &diff_queued_diff;\n> @@ -4272,9 +4273,17 @@ static void run_external_diff(const char *pgm,\n>  \n>  \tdiff_free_filespec_data(one);\n>  \tdiff_free_filespec_data(two);\n> +\n> +\tstart_file = xstrdup_or_null(getenv(\"START_FILE\"));\n> +\tif (start_file) {\n> +\t\tif (strcmp(start_file, name))\n> +\t\t\tgoto finish;\n> +\t\tunsetenv(\"START_FILE\");\n> +\t}\n\nAgain, an unacceptable \"hack\".  \"start the diff output showing from\nthis path\" would plausibly a good feature even outside the scope of\n\"difftool\", and the feature should not be limited to the external\ndiff interface.  More on this later.\n\n>  \tif (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v))\n>  \t\tdie(_(\"external diff died, stopping at %s\"), name);\n>  \n> +finish:\n>  \tremove_tempfile();\n>  \tstrvec_clear(&argv);\n>  \tstrvec_clear(&env);\n> diff --git a/diff.h b/diff.h\n> index 2ff2b1c7f2ca..f67c43f5af95 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -86,18 +86,18 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n>  \n>  typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n>  \n> -#define DIFF_FORMAT_RAW\t\t0x0001\n> -#define DIFF_FORMAT_DIFFSTAT\t0x0002\n> -#define DIFF_FORMAT_NUMSTAT\t0x0004\n> -#define DIFF_FORMAT_SUMMARY\t0x0008\n> -#define DIFF_FORMAT_PATCH\t0x0010\n> -#define DIFF_FORMAT_SHORTSTAT\t0x0020\n> -#define DIFF_FORMAT_DIRSTAT\t0x0040\n> +#define DIFF_FORMAT_RAW\t\t(1U<<0)\n> +#define DIFF_FORMAT_DIFFSTAT\t(1U<<1)\n> +#define DIFF_FORMAT_NUMSTAT\t(1U<<2)\n> +#define DIFF_FORMAT_SUMMARY\t(1U<<3)\n> +#define DIFF_FORMAT_PATCH\t(1U<<4)\n> +#define DIFF_FORMAT_SHORTSTAT\t(1U<<5)\n> +#define DIFF_FORMAT_DIRSTAT\t(1U<<6)\n>  \n>  /* These override all above */\n> -#define DIFF_FORMAT_NAME\t0x0100\n> -#define DIFF_FORMAT_NAME_STATUS\t0x0200\n> -#define DIFF_FORMAT_CHECKDIFF\t0x0400\n> +#define DIFF_FORMAT_NAME\t(1U<<8)\n> +#define DIFF_FORMAT_NAME_STATUS\t(1U<<9)\n> +#define DIFF_FORMAT_CHECKDIFF\t(1U<<10)\n\nDo we need these changes for the new feature to work?\n\nI also find this \"skip and discard ones earlier than the given path\"\nmakes the utility of the feature artificially narrower than needed,\nwhen I imagine how else this feature, or a variant of it, would be\nuseful in other situations.  For example, consider that there are 5\npaths, and you've seen 3 of them so far before you went off, so you\nare restarting from 4th file.  But wouldn't it be more useful, after\nshowing the 4th and 5th file, if the tool wraps around to show 1st,\n2nd, and 3rd file if the user kept going?\n\nI suspect that this feature fits the overall system much better if\nit is implemented as a new step in the diffcore transformation.  The\nway our diff subsystem works is roughly:\n\n * The front-end \"diff\", \"diff-files\", \"diff-index\" and \"diff-trees\"\n   are given two \"tree like things\" to compare, and feeds bunch of\n   <old, new> tuples to the diff internal machinery.  Each of these\n   tuples are called \"filepairs\", and a \"pair\" has two \"filespecs\",\n   one describing the contents, the mode, and the path in the \"old\"\n   side of the \"tree like things\", the other describing the\n   contents, the mode, and the path in the \"new\" side of the \"tree\n   like things\".\n\n * The series of filepairs are given to the \"diffcore\" machinery,\n   where they may be broken (i.e. a filepair that says that the old\n   contents of \"hello.txt\" was X, and the new contents of\n   \"hello.txt\" is Y, may become two filepairs, one that says that\n   the \"hello.txt\" file with contents X used to be in the old tree\n   but there is nothing corresponding to it in the new tree, and the\n   other says that a new \"hello.txt\" with contents Y appeared\n   without corresponding thing in the old tree), matched (i.e. there\n   may originally be two filepairs, one that says path A.txt appears\n   on the old side but disappeared on the new side, and the other\n   that says path sub/A.txt did not exist on the old side but\n   appears on the new side---these two filepairs may be merged to\n   express \"A.txt on the old side got renamed to sub/A.txt on the\n   new side\"), etc.\n\n * The set of filepairs processed in the \"diffcore\" machinery is\n   given to the backend and each filepair will be shown in the\n   output, as a series of patches, a diffstat, etc.\n\nThere is a step in the \"diffcore\" machinery called \"diffcore-order\".\nThe front-ends all feed the filepairs alphabetically to the\n\"diffcore\" machinery, but \"diffcore-order\" can reorder them, so that\nthe original order that the \"git diff HEAD --\" frontend found the\nchanges may be to \"hello.c\" and then \"hello.h\" (because .c sorts\nbefore .h), but the users can specify with the \"-O<orderfile>\"\noption that they want to view the header files before the source\nfiles.  When the set of filepairs exits the diffcore machinery, the\noriginal \"hello.c\" then \"hello.h\" order may get modified to show\n\"hello.h\" then \"hello.c\".  It probably is the simplest to model this\nnew feature after how \"diffcore-order\" does it.\n\nSo, perhaps we can introduce a new \"diffcore-rotate\" step, where the\nfilepairs before the specified location are rotated out to the end\nof the filepairs?\n\nThe following is just a quick draft that is only lightly tested by\nrunning itself with the \"--rotate-to=diffcore-rotate.c\" option, but\nit should be sufficient to get you started.  You should add a way to\ndiagnose that the name given to the \"--start-file\" option actually\nexists in the diff on the \"difftool\" side, because a path that does\nnot exist in the patch with the following code is simply ignored\n(and that is very much deliberate, because we do not want it to die\nwhile running \"git log -p --rotate-to=X\" where some changes may or\nmay not touch X).\n\n\n$ ./git diff --stat -p --rotate-to=diffcore-rotate.c HEAD\n\n diffcore-rotate.c | 35 +++++++++++++++++++++++++++++++++++\n diffcore.h        |  1 +\n Makefile          |  1 +\n diff.c            |  4 ++++\n diff.h            |  1 +\n 5 files changed, 42 insertions(+)\n\ndiff --git c/diffcore-rotate.c w/diffcore-rotate.c\nnew file mode 100644\nindex 0000000000..0d17901616\n--- /dev/null\n+++ w/diffcore-rotate.c\n@@ -0,0 +1,35 @@\n+/*\n+ * Copyright (C) 2021, Google LLC.\n+ * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano\n+ */\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+\n+void diffcore_rotate(const char *rotate_to_filename)\n+{\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct diff_queue_struct outq;\n+\tint rotate_to, i;\n+\n+\tif (!q->nr)\n+\t\treturn;\n+\n+\tfor (i = 0; i < q->nr; i++)\n+\t\tif (!strcmp(rotate_to_filename, q->queue[i]->two->path))\n+\t\t\tbreak;\n+\t/* we did not find the specified path */\n+\tif (q->nr <= i)\n+\t\treturn;\n+\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\trotate_to = i;\n+\n+\tfor (i = rotate_to; i < q->nr; i++)\n+\t\tdiff_q(&outq, q->queue[i]);\n+\tfor (i = 0; i < rotate_to; i++)\n+\t\tdiff_q(&outq, q->queue[i]);\n+\n+\tfree(q->queue);\n+\t*q = outq;\n+}\ndiff --git c/diffcore.h w/diffcore.h\nindex d2a63c5c71..bd5959375b 100644\n--- c/diffcore.h\n+++ w/diffcore.h\n@@ -164,6 +164,7 @@ void diffcore_rename(struct diff_options *);\n void diffcore_merge_broken(void);\n void diffcore_pickaxe(struct diff_options *);\n void diffcore_order(const char *orderfile);\n+void diffcore_rotate(const char *rotate_to_filename);\n \n /* low-level interface to diffcore_order */\n struct obj_order {\ndiff --git c/Makefile w/Makefile\nindex b797033c58..031b0b88e6 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -869,6 +869,7 @@ LIB_OBJS += diffcore-delta.o\n LIB_OBJS += diffcore-order.o\n LIB_OBJS += diffcore-pickaxe.o\n LIB_OBJS += diffcore-rename.o\n+LIB_OBJS += diffcore-rotate.o\n LIB_OBJS += dir-iterator.o\n LIB_OBJS += dir.o\n LIB_OBJS += editor.o\ndiff --git c/diff.c w/diff.c\nindex 69e3bc00ed..90a8d5abd0 100644\n--- c/diff.c\n+++ w/diff.c\n@@ -5599,6 +5599,8 @@ static void prep_parse_options(struct diff_options *options)\n \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t     N_(\"control the order in which files appear in the output\")),\n+\t\tOPT_STRING(0, \"rotate-to\", &options->rotate_to, N_(\"<path>\"),\n+\t\t\t   N_(\"show the change in the specified path first\")),\n \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n \t\t\t       N_(\"look for differences that change the number of occurrences of the specified object\"),\n \t\t\t       PARSE_OPT_NONEG, diff_opt_find_object),\n@@ -6669,6 +6671,8 @@ void diffcore_std(struct diff_options *options)\n \t\tdiffcore_pickaxe(options);\n \tif (options->orderfile)\n \t\tdiffcore_order(options->orderfile);\n+\tif (options->rotate_to)\n+\t\tdiffcore_rotate(options->rotate_to);\n \tif (!options->found_follow)\n \t\t/* See try_to_follow_renames() in tree-diff.c */\n \t\tdiff_resolve_rename_copy();\ndiff --git c/diff.h w/diff.h\nindex 2ff2b1c7f2..0801469e63 100644\n--- c/diff.h\n+++ w/diff.h\n@@ -226,6 +226,7 @@ enum diff_submodule_format {\n  */\n struct diff_options {\n \tconst char *orderfile;\n+\tconst char *rotate_to;\n \n \t/**\n \t * A constant string (can and typically does contain newlines to look for\n"},{"id":"416644","messageId":"CAOLTT8QbutZ2pHZ7Zg7vEJAy=d66YKP12rVW=RSJV+8fH6RRMw@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqeehp2jis.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] difftool.c: learn a new way start from specified file","fromName":"胡哲宁","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-10T17:00:41Z","receivedAt":"2021-02-10T16:59:53Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月10日周三 上午2:17写道：\n>\n> \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\n> > index 484c485fd06c..552be097dfea 100644\n> > --- a/Documentation/git-difftool.txt\n> > +++ b/Documentation/git-difftool.txt\n> > @@ -34,6 +34,9 @@ OPTIONS\n> >       This is the default behaviour; the option is provided to\n> >       override any configuration settings.\n> >\n> > +--start-from::\n> > +     Start viewing diff from the specified file.\n> > +\n>\n> There is nothing that specifies a file in the above ;-)\n>\n>         --start-from=<file>::\n>                 Start viewing ...\n>\n> This is even more important as SYNOPSIS section of this manual page\n> does not duplicate the list of options and their arguments.\n>\nI admit my mistake,thanks for reminding.\n> >  -t <tool>::\n> >  --tool=<tool>::\n> >       Use the diff tool specified by <tool>.  Valid values include\n>\n> There are many things I dislike about this patch, but do not take it\n> as a personal attack.  I'll try to suggest an alternative at the end,\n> but read along.\n>\nOf course I humbly accepted your criticism,I am only a sophomore in\nuniversity after all,It is extremely important to listen to your suggestions.\n> > diff --git a/builtin/difftool.c b/builtin/difftool.c\n> > index 6e18e623fddf..67d2befa1210 100644\n> > --- a/builtin/difftool.c\n> > +++ b/builtin/difftool.c\n> > @@ -690,7 +690,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n> >  {\n> >       int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,\n> >           tool_help = 0, no_index = 0;\n> > -     static char *difftool_cmd = NULL, *extcmd = NULL;\n> > +     static char *difftool_cmd = NULL, *extcmd = NULL, *start_file = NULL;\n> >       struct option builtin_difftool_options[] = {\n> >               OPT_BOOL('g', \"gui\", &use_gui_tool,\n> >                        N_(\"use `diff.guitool` instead of `diff.tool`\")),\n> > @@ -714,6 +714,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n> >               OPT_STRING('x', \"extcmd\", &extcmd, N_(\"command\"),\n> >                          N_(\"specify a custom command for viewing diffs\")),\n> >               OPT_ARGUMENT(\"no-index\", &no_index, N_(\"passed to `diff`\")),\n> > +             OPT_STRING(0, \"start-from\", &start_file, N_(\"start-from\"),\n> > +                        N_(\"start viewing diff from the specified file\")),\n> >               OPT_END()\n> >       };\n>\n> This may be a good UI to \"difftool\".\n>\n> > @@ -724,6 +726,9 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)\n> >                            builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |\n> >                            PARSE_OPT_KEEP_DASHDASH);\n> >\n> > +     if (start_file)\n> > +             setenv(\"START_FILE\", start_file, 1);\n>\n> Unacceptable.  Nothing gives Git the right to squat on such a\n> generic name, and there is no hint that it is used to specify the\n> starting point of what operation.  In addition, I do not see a good\n> reason why we need to use an environment variable in the first\n> place.  We run \"diff\" as an external process, with GIT_EXTERNAL_DIFF\n> environment pointing back at us, no?  This information should be\n> passed from its command line.\nSure,I using environment variables originally hoped can be used in\ngit--difftool-helper.sh, later I found it was not easy to deal with, so I am\nattempt to use environment variables to transmit information in\n`run_external_diff`,now it seems that this method is not very good ...\n>\n> > diff --git a/diff.c b/diff.c\n> > index 69e3bc00ed8f..cdad26f99063 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -4249,6 +4249,7 @@ static void run_external_diff(const char *pgm,\n> >                             const char *xfrm_msg,\n> >                             struct diff_options *o)\n> >  {\n> > +     const char *start_file = NULL;\n> >       struct strvec argv = STRVEC_INIT;\n> >       struct strvec env = STRVEC_INIT;\n> >       struct diff_queue_struct *q = &diff_queued_diff;\n> > @@ -4272,9 +4273,17 @@ static void run_external_diff(const char *pgm,\n> >\n> >       diff_free_filespec_data(one);\n> >       diff_free_filespec_data(two);\n> > +\n> > +     start_file = xstrdup_or_null(getenv(\"START_FILE\"));\n> > +     if (start_file) {\n> > +             if (strcmp(start_file, name))\n> > +                     goto finish;\n> > +             unsetenv(\"START_FILE\");\n> > +     }\n>\n> Again, an unacceptable \"hack\".  \"start the diff output showing from\n> this path\" would plausibly a good feature even outside the scope of\n> \"difftool\", and the feature should not be limited to the external\n> diff interface.  More on this later.\nWhat you said `from its command line` is using `git difftool\n--rotate-to=<file>`,\nMaybe I didn’t know whether to deal with the subcommands of `diff`, now I admit\nthat passing from parameters is better than passing environment variables.\n>\n> >       if (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v))\n> >               die(_(\"external diff died, stopping at %s\"), name);\n> >\n> > +finish:\n> >       remove_tempfile();\n> >       strvec_clear(&argv);\n> >       strvec_clear(&env);\n> > diff --git a/diff.h b/diff.h\n> > index 2ff2b1c7f2ca..f67c43f5af95 100644\n> > --- a/diff.h\n> > +++ b/diff.h\n> > @@ -86,18 +86,18 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n> >\n> >  typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n> >\n> > -#define DIFF_FORMAT_RAW              0x0001\n> > -#define DIFF_FORMAT_DIFFSTAT 0x0002\n> > -#define DIFF_FORMAT_NUMSTAT  0x0004\n> > -#define DIFF_FORMAT_SUMMARY  0x0008\n> > -#define DIFF_FORMAT_PATCH    0x0010\n> > -#define DIFF_FORMAT_SHORTSTAT        0x0020\n> > -#define DIFF_FORMAT_DIRSTAT  0x0040\n> > +#define DIFF_FORMAT_RAW              (1U<<0)\n> > +#define DIFF_FORMAT_DIFFSTAT (1U<<1)\n> > +#define DIFF_FORMAT_NUMSTAT  (1U<<2)\n> > +#define DIFF_FORMAT_SUMMARY  (1U<<3)\n> > +#define DIFF_FORMAT_PATCH    (1U<<4)\n> > +#define DIFF_FORMAT_SHORTSTAT        (1U<<5)\n> > +#define DIFF_FORMAT_DIRSTAT  (1U<<6)\n> >\n> >  /* These override all above */\n> > -#define DIFF_FORMAT_NAME     0x0100\n> > -#define DIFF_FORMAT_NAME_STATUS      0x0200\n> > -#define DIFF_FORMAT_CHECKDIFF        0x0400\n> > +#define DIFF_FORMAT_NAME     (1U<<8)\n> > +#define DIFF_FORMAT_NAME_STATUS      (1U<<9)\n> > +#define DIFF_FORMAT_CHECKDIFF        (1U<<10)\n>\n> Do we need these changes for the new feature to work?\nIt has no effect on this new feature. I should put this modification\nin an additional commit, right?\n>\n> I also find this \"skip and discard ones earlier than the given path\"\n> makes the utility of the feature artificially narrower than needed,\n> when I imagine how else this feature, or a variant of it, would be\n> useful in other situations.  For example, consider that there are 5\n> paths, and you've seen 3 of them so far before you went off, so you\n> are restarting from 4th file.  But wouldn't it be more useful, after\n> showing the 4th and 5th file, if the tool wraps around to show 1st,\n> 2nd, and 3rd file if the user kept going?\n>\nWell, it would be better if you can see 1st, 2nd, 3rd again.\n> I suspect that this feature fits the overall system much better if\n> it is implemented as a new step in the diffcore transformation.  The\n> way our diff subsystem works is roughly:\n>\n>  * The front-end \"diff\", \"diff-files\", \"diff-index\" and \"diff-trees\"\n>    are given two \"tree like things\" to compare, and feeds bunch of\n>    <old, new> tuples to the diff internal machinery.  Each of these\n>    tuples are called \"filepairs\", and a \"pair\" has two \"filespecs\",\n>    one describing the contents, the mode, and the path in the \"old\"\n>    side of the \"tree like things\", the other describing the\n>    contents, the mode, and the path in the \"new\" side of the \"tree\n>    like things\".\n>\n>  * The series of filepairs are given to the \"diffcore\" machinery,\n>    where they may be broken (i.e. a filepair that says that the old\n>    contents of \"hello.txt\" was X, and the new contents of\n>    \"hello.txt\" is Y, may become two filepairs, one that says that\n>    the \"hello.txt\" file with contents X used to be in the old tree\n>    but there is nothing corresponding to it in the new tree, and the\n>    other says that a new \"hello.txt\" with contents Y appeared\n>    without corresponding thing in the old tree), matched (i.e. there\n>    may originally be two filepairs, one that says path A.txt appears\n>    on the old side but disappeared on the new side, and the other\n>    that says path sub/A.txt did not exist on the old side but\n>    appears on the new side---these two filepairs may be merged to\n>    express \"A.txt on the old side got renamed to sub/A.txt on the\n>    new side\"), etc.\n>\nThank you for these interpretations on \"git diff\" for me, it will help me\nunderstand its principle.\n>  * The set of filepairs processed in the \"diffcore\" machinery is\n>    given to the backend and each filepair will be shown in the\n>    output, as a series of patches, a diffstat, etc.\n>\n> There is a step in the \"diffcore\" machinery called \"diffcore-order\".\n> The front-ends all feed the filepairs alphabetically to the\n> \"diffcore\" machinery, but \"diffcore-order\" can reorder them, so that\n> the original order that the \"git diff HEAD --\" frontend found the\n> changes may be to \"hello.c\" and then \"hello.h\" (because .c sorts\n> before .h), but the users can specify with the \"-O<orderfile>\"\n> option that they want to view the header files before the source\n> files.  When the set of filepairs exits the diffcore machinery, the\n> original \"hello.c\" then \"hello.h\" order may get modified to show\n> \"hello.h\" then \"hello.c\".  It probably is the simplest to model this\n> new feature after how \"diffcore-order\" does it.\n>\nVery magical \"diffcore-order\", when I was looking for a solution yesterday,\nI noticed that these functions similar to \"diffcore_apply_filter\" in\n\"diffcore_std\",\n> So, perhaps we can introduce a new \"diffcore-rotate\" step, where the\n> filepairs before the specified location are rotated out to the end\n> of the filepairs?\n>\nAwesome idea. In this way, `difftool --rotate-to=<file>` can call\n`diff --rotate-to=<file>` , user can choose the starting file, and they can\nalso see previous files.\n> The following is just a quick draft that is only lightly tested by\n> running itself with the \"--rotate-to=diffcore-rotate.c\" option, but\n> it should be sufficient to get you started.  You should add a way to\n> diagnose that the name given to the \"--start-file\" option actually\n> exists in the diff on the \"difftool\" side, because a path that does\n> not exist in the patch with the following code is simply ignored\n> (and that is very much deliberate, because we do not want it to die\n> while running \"git log -p --rotate-to=X\" where some changes may or\n> may not touch X).\nYes, I want to know why being so cautious in git log?If the file name is\nwrong, why can't I make it exit? :)\n>\n>\n> $ ./git diff --stat -p --rotate-to=diffcore-rotate.c HEAD\n>\n>  diffcore-rotate.c | 35 +++++++++++++++++++++++++++++++++++\n>  diffcore.h        |  1 +\n>  Makefile          |  1 +\n>  diff.c            |  4 ++++\n>  diff.h            |  1 +\n>  5 files changed, 42 insertions(+)\n>\n> diff --git c/diffcore-rotate.c w/diffcore-rotate.c\n> new file mode 100644\n> index 0000000000..0d17901616\n> --- /dev/null\n> +++ w/diffcore-rotate.c\n> @@ -0,0 +1,35 @@\n> +/*\n> + * Copyright (C) 2021, Google LLC.\n> + * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano\n> + */\n> +#include \"cache.h\"\n> +#include \"diff.h\"\n> +#include \"diffcore.h\"\n> +\n> +void diffcore_rotate(const char *rotate_to_filename)\n> +{\n> +       struct diff_queue_struct *q = &diff_queued_diff;\n> +       struct diff_queue_struct outq;\n> +       int rotate_to, i;\n> +\n> +       if (!q->nr)\n> +               return;\n> +\n> +       for (i = 0; i < q->nr; i++)\n> +               if (!strcmp(rotate_to_filename, q->queue[i]->two->path))\n> +                       break;\n> +       /* we did not find the specified path */\n> +       if (q->nr <= i)\n> +               return;\n> +\n> +       DIFF_QUEUE_CLEAR(&outq);\n> +       rotate_to = i;\n> +\n> +       for (i = rotate_to; i < q->nr; i++)\n> +               diff_q(&outq, q->queue[i]);\n> +       for (i = 0; i < rotate_to; i++)\n> +               diff_q(&outq, q->queue[i]);\n> +\n> +       free(q->queue);\n> +       *q = outq;\n> +}\n> diff --git c/diffcore.h w/diffcore.h\n> index d2a63c5c71..bd5959375b 100644\n> --- c/diffcore.h\n> +++ w/diffcore.h\n> @@ -164,6 +164,7 @@ void diffcore_rename(struct diff_options *);\n>  void diffcore_merge_broken(void);\n>  void diffcore_pickaxe(struct diff_options *);\n>  void diffcore_order(const char *orderfile);\n> +void diffcore_rotate(const char *rotate_to_filename);\n>\n>  /* low-level interface to diffcore_order */\n>  struct obj_order {\n> diff --git c/Makefile w/Makefile\n> index b797033c58..031b0b88e6 100644\n> --- c/Makefile\n> +++ w/Makefile\n> @@ -869,6 +869,7 @@ LIB_OBJS += diffcore-delta.o\n>  LIB_OBJS += diffcore-order.o\n>  LIB_OBJS += diffcore-pickaxe.o\n>  LIB_OBJS += diffcore-rename.o\n> +LIB_OBJS += diffcore-rotate.o\n>  LIB_OBJS += dir-iterator.o\n>  LIB_OBJS += dir.o\n>  LIB_OBJS += editor.o\n> diff --git c/diff.c w/diff.c\n> index 69e3bc00ed..90a8d5abd0 100644\n> --- c/diff.c\n> +++ w/diff.c\n> @@ -5599,6 +5599,8 @@ static void prep_parse_options(struct diff_options *options)\n>                           DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n>                 OPT_FILENAME('O', NULL, &options->orderfile,\n>                              N_(\"control the order in which files appear in the output\")),\n> +               OPT_STRING(0, \"rotate-to\", &options->rotate_to, N_(\"<path>\"),\n> +                          N_(\"show the change in the specified path first\")),\n>                 OPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n>                                N_(\"look for differences that change the number of occurrences of the specified object\"),\n>                                PARSE_OPT_NONEG, diff_opt_find_object),\n> @@ -6669,6 +6671,8 @@ void diffcore_std(struct diff_options *options)\n>                 diffcore_pickaxe(options);\n>         if (options->orderfile)\n>                 diffcore_order(options->orderfile);\n> +       if (options->rotate_to)\n> +               diffcore_rotate(options->rotate_to);\n>         if (!options->found_follow)\n>                 /* See try_to_follow_renames() in tree-diff.c */\n>                 diff_resolve_rename_copy();\n> diff --git c/diff.h w/diff.h\n> index 2ff2b1c7f2..0801469e63 100644\n> --- c/diff.h\n> +++ w/diff.h\n> @@ -226,6 +226,7 @@ enum diff_submodule_format {\n>   */\n>  struct diff_options {\n>         const char *orderfile;\n> +       const char *rotate_to;\n>\n>         /**\n>          * A constant string (can and typically does contain newlines to look for\n\nThanks for the patch!\nAfter that, there was too little work I could do,do i just need to add\nthe following\n code in `diff_flush_patch_all_file_pairs`?\nif (o->rotate_to && q->nr && strcmp(q->queue[0]->one->path, o->rotate_to) &&\nstrcmp(q->queue[0]->one->path, o->rotate_to)) {\n    error(_(\"could not find start file name '%s'\"), o->rotate_to);\n        return;\n}\nIn addition, Do I need to do the documentation and tests related to\nyour `diff --rotate-to`?\nIf so, I will finish it, otherwise, I will only do corresponding tests\nfor `git difftool --rotate-to`.\n\nThank you Junio, you are like a tireless teacher,\n\nHappy Chinese New Year!\n\n--\nZheNing Hu\n"},{"id":"416649","messageId":"xmqqk0rf3i07.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"CAOLTT8QbutZ2pHZ7Zg7vEJAy=d66YKP12rVW=RSJV+8fH6RRMw@mail.gmail.com","subject":"Re: [PATCH v3] difftool.c: learn a new way start from specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-10T18:16:56Z","receivedAt":"2021-02-10T18:20:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"胡哲宁 <adlternative@gmail.com> writes:\n\n> It has no effect on this new feature. I should put this modification\n> in an additional commit, right?\n\nOr you can just drop it.  It certainly is a distracting change to be\npart of this topic.\n\n> Yes, I want to know why being so cautious in git log?If the file name is\n> wrong, why can't I make it exit? :)\n\nImagine a history where file1 and file2 are in the initial commit.\nThe second commit adds file3 and modifies file2, and then the third\ncommit modifies file1.  What would happen when you did this?\n\n  $ git log -p --rotate-to=file2\n\nFor the commit at HEAD, the set of paths shown would be file1 and\nnothing else (because it is the only path that gets modified).  You\ncannot start showing from \"file2\".  Dying (and not showing HEAD~1\nand HEAD~2) is the last thing you want to do in this situation.  We\ndo not even want to give a warning or an error, as it is totally\nexpected that some commits do not touch a given path---it would be\nvery unusual if a path is touched by every commit ;-).\n\nFor \"difftool --start-at=file2\", the equation is different.  It does\nnot traverse history where each commit may or may not modify file2,\nand when the user says s/he wants to resume from file2, file2 must\nbe in the set of paths that have changes, or something is wrong (i.e.\nthe user meant file3 but mistyped it as file2).\n\n> Awesome idea. In this way, `difftool --rotate-to=<file>` can call\n> `diff --rotate-to=<file>` , user can choose the starting file, and they can\n> also see previous files.\n\nSo \"difftool --start-at=<file>\" can of use \"diff --rotate-to=<file>\"\nto implement the feature (after all, that is why I wrote it), but\nthe error condition between the two are quite different.  And ...\n\n> After that, there was too little work I could do,do i just need to add\n> the following\n>  code in `diff_flush_patch_all_file_pairs`?\n\n\n> if (o->rotate_to && q->nr && strcmp(q->queue[0]->one->path, o->rotate_to) &&\n> strcmp(q->queue[0]->one->path, o->rotate_to)) {\n>     error(_(\"could not find start file name '%s'\"), o->rotate_to);\n>         return;\n> }\n\n... that is why an unconditional change like this in diff.c is not\nacceptable, as it would break other codepaths like \"git log -p\".  If\nwe were to add an error there, it has to be very limited to exclude\nat least \"log -p\"---there may be other features that share the code\nthat should not trigger an error for similar reasons.\n\nIf diffcore-rotate chooses \"missing rotate-to file makes it a no-op\"\nas its semantics, and if \"difftool --start-at\" does not want to see\na misspelt filename making it a no-op, then the latter needs to\nensure that the name it got from the user is indeed in the set of\npaths that have changes before running \"diff --rotate-to\" to\nimplement its \"difftool --start-at\" feature.\n\nThe \"missing rotate-to file in the diff_queue MUST NOT cause\ndiffcore-rotate to error out\" rule is probably unnegitiable, but\nthere are other ways to make it easier to use, though.\n\nFor example, we could change the rotate-to logic to mean \"start at\nthe given path, or if such a path does not exist in the diff_queue,\nthen the first path that sorts after the given path is where we\nstart\".  That way, if the diff_queue has paths A B C D E and\nrotate-to gives C, then we rotate the output to C D E A B.  And if\nthe diff_queue has A B D E and rotate-to gives C, then the output\nwould become D E A B (instead of becoming a no-op).  Then, a mistyped\nfilename may not do what the user wanted to do (after all, that is\nthe definition of MIStyping), but it would do something noticeable\nby the user, which may be useful enough and at least would let the\nuser notice the mistake.\n\n> In addition, Do I need to do the documentation and tests related to\n> your `diff --rotate-to`?\n\nOnce we know how we want \"diff --rotate-to\" to behave exactly, I can\nhelp that part further, if you want.  And then you can build on top.\n\nBut we need to design exactly what the desired semantics would be\nbefore any of that.\n\nThanks.\n\n"},{"id":"416652","messageId":"xmqqft233cz8.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"xmqqk0rf3i07.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] difftool.c: learn a new way start from specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-10T20:05:31Z","receivedAt":"2021-02-10T20:06:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> So \"difftool --start-at=<file>\" can of use \"diff --rotate-to=<file>\"\n\nsorry; s/can of/can of course/.\n"},{"id":"416858","messageId":"CAOLTT8QmHvfQeOTbw0xwDY=z_GyF2g5bc-C3Do1ONoQ+CqiqgA@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqk0rf3i07.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3] difftool.c: learn a new way start from specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-14T07:53:44Z","receivedAt":"2021-02-14T07:52:48Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月11日周四 上午2:17写道：\n>\n> 胡哲宁 <adlternative@gmail.com> writes:\n>\n> > It has no effect on this new feature. I should put this modification\n> > in an additional commit, right?\n>\n> Or you can just drop it.  It certainly is a distracting change to be\n> part of this topic.\n>\nOK.\n> > Yes, I want to know why being so cautious in git log?If the file name is\n> > wrong, why can't I make it exit? :)\n>\n> Imagine a history where file1 and file2 are in the initial commit.\n> The second commit adds file3 and modifies file2, and then the third\n> commit modifies file1.  What would happen when you did this?\n>\n>   $ git log -p --rotate-to=file2\n>\n> For the commit at HEAD, the set of paths shown would be file1 and\n> nothing else (because it is the only path that gets modified).  You\n> cannot start showing from \"file2\".  Dying (and not showing HEAD~1\n> and HEAD~2) is the last thing you want to do in this situation.  We\n> do not even want to give a warning or an error, as it is totally\n> expected that some commits do not touch a given path---it would be\n> very unusual if a path is touched by every commit ;-).\n>\nNow I understand that in `log -p`, some commit do not have the file, and some\ncommit have the file. The best way to display the commit without the file\nis to keep it as it is, and rotate the commit with the file. And in `difftool`\nneed to ensure the exist the specified file or give the more matching file\nas the beginning(As you mentioned later).\n> For \"difftool --start-at=file2\", the equation is different.  It does\n> not traverse history where each commit may or may not modify file2,\n> and when the user says s/he wants to resume from file2, file2 must\n> be in the set of paths that have changes, or something is wrong (i.e.\n> the user meant file3 but mistyped it as file2).\n>\n> > Awesome idea. In this way, `difftool --rotate-to=<file>` can call\n> > `diff --rotate-to=<file>` , user can choose the starting file, and they can\n> > also see previous files.\n>\n> So \"difftool --start-at=<file>\" can of use \"diff --rotate-to=<file>\"\n> to implement the feature (after all, that is why I wrote it), but\n> the error condition between the two are quite different.  And ...\n\n> > After that, there was too little work I could do,do i just need to add\n> > the following\n> >  code in `diff_flush_patch_all_file_pairs`?\n>\n>\n> > if (o->rotate_to && q->nr && strcmp(q->queue[0]->one->path, o->rotate_to) &&\n> > strcmp(q->queue[0]->one->path, o->rotate_to)) {\n> >     error(_(\"could not find start file name '%s'\"), o->rotate_to);\n> >         return;\n> > }\n>\n> ... that is why an unconditional change like this in diff.c is not\n> acceptable, as it would break other codepaths like \"git log -p\".  If\n> we were to add an error there, it has to be very limited to exclude\n> at least \"log -p\"---there may be other features that share the code\n> that should not trigger an error for similar reasons.\n>\n> If diffcore-rotate chooses \"missing rotate-to file makes it a no-op\"\n> as its semantics, and if \"difftool --start-at\" does not want to see\n> a misspelt filename making it a no-op, then the latter needs to\n> ensure that the name it got from the user is indeed in the set of\n> paths that have changes before running \"diff --rotate-to\" to\n> implement its \"difftool --start-at\" feature.\n>\n> The \"missing rotate-to file in the diff_queue MUST NOT cause\n> diffcore-rotate to error out\" rule is probably unnegitiable, but\n> there are other ways to make it easier to use, though.\n>\n> For example, we could change the rotate-to logic to mean \"start at\n> the given path, or if such a path does not exist in the diff_queue,\n> then the first path that sorts after the given path is where we\n> start\".  That way, if the diff_queue has paths A B C D E and\n> rotate-to gives C, then we rotate the output to C D E A B.  And if\n> the diff_queue has A B D E and rotate-to gives C, then the output\n> would become D E A B (instead of becoming a no-op).  Then, a mistyped\n> filename may not do what the user wanted to do (after all, that is\n> the definition of MIStyping), but it would do something noticeable\n> by the user, which may be useful enough and at least would let the\n> user notice the mistake.\n>\nIn doing this, I think the processing methods of difftool and other diffs\nare unified.I think this kind of processing is actually very easy,\njust need to change\n> if (!strcmp(rotate_to_filename, q->queue[i]->two->path))\nto\n> if (strcmp(rotate_to_filename, q->queue[i]->two->path) <= 0)\nOf course, it might be better if there is an algorithm that can achieve\nthe highest degree of file name matching.\n\nNow that `difftool --start-at` and `diff --rotate-to` are unified effects,\nis \"start-at\" just an alias for \"rotate-to\"?\nOr do I need to write like this?\n> OPT_STRING(0, \"start-with\", &options->rotate-to, N_(\"<path>\"),\n>   N_(\"pass from difftool to diff, has the same effort as `rotate-to`\")),\n\n> > In addition, Do I need to do the documentation and tests related to\n> > your `diff --rotate-to`?\n>\n> Once we know how we want \"diff --rotate-to\" to behave exactly, I can\n> help that part further, if you want.  And then you can build on top.\n>\n> But we need to design exactly what the desired semantics would be\n> before any of that.\n>\n> Thanks.\n>\nThanks.\n"},{"id":"416884","messageId":"pull.870.v4.git.1613308167239.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v3.git.1612884654078.gitgitgadget@gmail.com","subject":"[PATCH v4] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-14T13:09:26Z","receivedAt":"2021-02-14T13:10:14Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view in turn.\nIf there is a commit with many files and we exit in the search,\nWe will have to traverse list again to get the file diff which\nwe want to see. Therefore, here is a new method: user can use\n`git difftool --start-from=<filename>` to start viewing from\nthe specified file, This will improve the user experience.\n\n`difftool --start-from=<file>` will pass the file name to\n`diffcore-rotate`, it will traverse all files in diff_queue,\nif it finds a matching file, it will rearrange the order of\ndiff_filepair of diff_queue, Rotate the file specified by the\nuser to the first one. If the file name specified by the user\ndoes not match any item in the diff queue, Git will also rotate\nthe queue, it will find the the first file name larger than the\nspecified file name as the first element of the new diff_queue.\nThis will help users find their mistakes.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    difftool.c: learn a new way start at specified file\n    \n    git user may should travel the diff list to choice file diff to view, if\n    they exit in midway,they must travel it again. By starting from the\n    specified file method, provides a possibility for this user-friendly\n    solution.\n    \n    this patch's origin discuss is here:\n    https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n    \n    Maybe this patch is more like skip to in Junio's original thread than\n    the previous versions.\n    \n    Thanks!\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\nRange-diff vs v3:\n\n 1:  29fc6b4bc08f ! 1:  3accfb942301 difftool.c: learn a new way start from specified file\n     @@ Metadata\n      Author: ZheNing Hu <adlternative@gmail.com>\n      \n       ## Commit message ##\n     -    difftool.c: learn a new way start from specified file\n     +    difftool.c: learn a new way start at specified file\n      \n          `git difftool` only allow us to select file to view in turn.\n          If there is a commit with many files and we exit in the search,\n          We will have to traverse list again to get the file diff which\n          we want to see. Therefore, here is a new method: user can use\n          `git difftool --start-from=<filename>` to start viewing from\n     -    the specified file. This will improve the user experience.\n     -    At the same time, turn bit field constants into bit shift format\n     -    in `diff.h`.\n     +    the specified file, This will improve the user experience.\n     +\n     +    `difftool --start-from=<file>` will pass the file name to\n     +    `diffcore-rotate`, it will traverse all files in diff_queue,\n     +    if it finds a matching file, it will rearrange the order of\n     +    diff_filepair of diff_queue, Rotate the file specified by the\n     +    user to the first one. If the file name specified by the user\n     +    does not match any item in the diff queue, Git will also rotate\n     +    the queue, it will find the the first file name larger than the\n     +    specified file name as the first element of the new diff_queue.\n     +    This will help users find their mistakes.\n      \n          Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n      \n     @@ Documentation/git-difftool.txt: OPTIONS\n       \tThis is the default behaviour; the option is provided to\n       \toverride any configuration settings.\n       \n     -+--start-from::\n     ++--start-from=<file>::\n      +\tStart viewing diff from the specified file.\n      +\n       -t <tool>::\n       --tool=<tool>::\n       \tUse the diff tool specified by <tool>.  Valid values include\n      \n     - ## builtin/difftool.c ##\n     -@@ builtin/difftool.c: int cmd_difftool(int argc, const char **argv, const char *prefix)\n     - {\n     - \tint use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0,\n     - \t    tool_help = 0, no_index = 0;\n     --\tstatic char *difftool_cmd = NULL, *extcmd = NULL;\n     -+\tstatic char *difftool_cmd = NULL, *extcmd = NULL, *start_file = NULL;\n     - \tstruct option builtin_difftool_options[] = {\n     - \t\tOPT_BOOL('g', \"gui\", &use_gui_tool,\n     - \t\t\t N_(\"use `diff.guitool` instead of `diff.tool`\")),\n     -@@ builtin/difftool.c: int cmd_difftool(int argc, const char **argv, const char *prefix)\n     - \t\tOPT_STRING('x', \"extcmd\", &extcmd, N_(\"command\"),\n     - \t\t\t   N_(\"specify a custom command for viewing diffs\")),\n     - \t\tOPT_ARGUMENT(\"no-index\", &no_index, N_(\"passed to `diff`\")),\n     -+\t\tOPT_STRING(0, \"start-from\", &start_file, N_(\"start-from\"),\n     -+\t\t\t   N_(\"start viewing diff from the specified file\")),\n     - \t\tOPT_END()\n     - \t};\n     - \n     -@@ builtin/difftool.c: int cmd_difftool(int argc, const char **argv, const char *prefix)\n     - \t\t\t     builtin_difftool_usage, PARSE_OPT_KEEP_UNKNOWN |\n     - \t\t\t     PARSE_OPT_KEEP_DASHDASH);\n     - \n     -+\tif (start_file)\n     -+\t\tsetenv(\"START_FILE\", start_file, 1);\n     -+\n     - \tif (tool_help)\n     - \t\treturn print_tool_help();\n     - \n     + ## Makefile ##\n     +@@ Makefile: LIB_OBJS += diffcore-delta.o\n     + LIB_OBJS += diffcore-order.o\n     + LIB_OBJS += diffcore-pickaxe.o\n     + LIB_OBJS += diffcore-rename.o\n     ++LIB_OBJS += diffcore-rotate.o\n     + LIB_OBJS += dir-iterator.o\n     + LIB_OBJS += dir.o\n     + LIB_OBJS += editor.o\n      \n       ## diff.c ##\n     -@@ diff.c: static void run_external_diff(const char *pgm,\n     - \t\t\t      const char *xfrm_msg,\n     - \t\t\t      struct diff_options *o)\n     - {\n     -+\tconst char *start_file = NULL;\n     - \tstruct strvec argv = STRVEC_INIT;\n     - \tstruct strvec env = STRVEC_INIT;\n     - \tstruct diff_queue_struct *q = &diff_queued_diff;\n     -@@ diff.c: static void run_external_diff(const char *pgm,\n     - \n     - \tdiff_free_filespec_data(one);\n     - \tdiff_free_filespec_data(two);\n     -+\n     -+\tstart_file = xstrdup_or_null(getenv(\"START_FILE\"));\n     -+\tif (start_file) {\n     -+\t\tif (strcmp(start_file, name))\n     -+\t\t\tgoto finish;\n     -+\t\tunsetenv(\"START_FILE\");\n     -+\t}\n     - \tif (run_command_v_opt_cd_env(argv.v, RUN_USING_SHELL, NULL, env.v))\n     - \t\tdie(_(\"external diff died, stopping at %s\"), name);\n     - \n     -+finish:\n     - \tremove_tempfile();\n     - \tstrvec_clear(&argv);\n     - \tstrvec_clear(&env);\n     +@@ diff.c: static void prep_parse_options(struct diff_options *options)\n     + \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n     + \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n     + \t\t\t     N_(\"control the order in which files appear in the output\")),\n     ++\t\tOPT_STRING(0, \"rotate-to\", &options->rotate_to, N_(\"<path>\"),\n     ++\t\t\t   N_(\"show the change in the specified path first\")),\n     ++\t\tOPT_STRING(0, \"start-from\", &options->rotate_to, N_(\"<path>\"),\n     ++\t\t\t   N_(\"pass from difftool to diff, has the same effort as `rotate-to`\")),\n     + \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n     + \t\t\t       N_(\"look for differences that change the number of occurrences of the specified object\"),\n     + \t\t\t       PARSE_OPT_NONEG, diff_opt_find_object),\n     +@@ diff.c: void diffcore_std(struct diff_options *options)\n     + \t\tdiffcore_pickaxe(options);\n     + \tif (options->orderfile)\n     + \t\tdiffcore_order(options->orderfile);\n     ++\tif (options->rotate_to)\n     ++\t\tdiffcore_rotate(options->rotate_to);\n     + \tif (!options->found_follow)\n     + \t\t/* See try_to_follow_renames() in tree-diff.c */\n     + \t\tdiff_resolve_rename_copy();\n      \n       ## diff.h ##\n     -@@ diff.h: typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,\n     - \n     - typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data);\n     - \n     --#define DIFF_FORMAT_RAW\t\t0x0001\n     --#define DIFF_FORMAT_DIFFSTAT\t0x0002\n     --#define DIFF_FORMAT_NUMSTAT\t0x0004\n     --#define DIFF_FORMAT_SUMMARY\t0x0008\n     --#define DIFF_FORMAT_PATCH\t0x0010\n     --#define DIFF_FORMAT_SHORTSTAT\t0x0020\n     --#define DIFF_FORMAT_DIRSTAT\t0x0040\n     -+#define DIFF_FORMAT_RAW\t\t(1U<<0)\n     -+#define DIFF_FORMAT_DIFFSTAT\t(1U<<1)\n     -+#define DIFF_FORMAT_NUMSTAT\t(1U<<2)\n     -+#define DIFF_FORMAT_SUMMARY\t(1U<<3)\n     -+#define DIFF_FORMAT_PATCH\t(1U<<4)\n     -+#define DIFF_FORMAT_SHORTSTAT\t(1U<<5)\n     -+#define DIFF_FORMAT_DIRSTAT\t(1U<<6)\n     +@@ diff.h: enum diff_submodule_format {\n     +  */\n     + struct diff_options {\n     + \tconst char *orderfile;\n     ++\tconst char *rotate_to;\n       \n     - /* These override all above */\n     --#define DIFF_FORMAT_NAME\t0x0100\n     --#define DIFF_FORMAT_NAME_STATUS\t0x0200\n     --#define DIFF_FORMAT_CHECKDIFF\t0x0400\n     -+#define DIFF_FORMAT_NAME\t(1U<<8)\n     -+#define DIFF_FORMAT_NAME_STATUS\t(1U<<9)\n     -+#define DIFF_FORMAT_CHECKDIFF\t(1U<<10)\n     + \t/**\n     + \t * A constant string (can and typically does contain newlines to look for\n     +\n     + ## diffcore-rotate.c (new) ##\n     +@@\n     ++/*\n     ++ * Copyright (C) 2021, Google LLC.\n     ++ * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano\n     ++ */\n     ++#include \"cache.h\"\n     ++#include \"diff.h\"\n     ++#include \"diffcore.h\"\n     ++\n     ++void diffcore_rotate(const char *rotate_to_filename)\n     ++{\n     ++\tstruct diff_queue_struct *q = &diff_queued_diff;\n     ++\tstruct diff_queue_struct outq;\n     ++\tint rotate_to, i;\n     ++\n     ++\tif (!q->nr)\n     ++\t\treturn;\n     ++\n     ++\tfor (i = 0; i < q->nr; i++)\n     ++\t\tif (strcmp(rotate_to_filename, q->queue[i]->two->path) <= 0)\n     ++\t\t\tbreak;\n     ++\t/* we did not find the specified path */\n     ++\tif (q->nr <= i)\n     ++\t\treturn;\n     ++\n     ++\tDIFF_QUEUE_CLEAR(&outq);\n     ++\trotate_to = i;\n     ++\n     ++\tfor (i = rotate_to; i < q->nr; i++)\n     ++\t\tdiff_q(&outq, q->queue[i]);\n     ++\tfor (i = 0; i < rotate_to; i++)\n     ++\t\tdiff_q(&outq, q->queue[i]);\n     ++\n     ++\tfree(q->queue);\n     ++\t*q = outq;\n     ++}\n     +\n     + ## diffcore.h ##\n     +@@ diffcore.h: void diffcore_rename(struct diff_options *);\n     + void diffcore_merge_broken(void);\n     + void diffcore_pickaxe(struct diff_options *);\n     + void diffcore_order(const char *orderfile);\n     ++void diffcore_rotate(const char *rotate_to_filename);\n       \n     - /* Same as output_format = 0 but we know that -s flag was given\n     -  * and we should not give default value to output_format.\n     + /* low-level interface to diffcore_order */\n     + struct obj_order {\n      \n       ## t/t7800-difftool.sh ##\n      @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n     @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd ar\n      +\ttest_when_finished git reset --hard &&\n      +\techo 1 >1 &&\n      +\techo 2 >2 &&\n     -+\techo 3 >3 &&\n     -+\tgit add 1 2 3 &&\n     -+\tgit commit -a -m \"123\" &&\n     -+\tgit difftool --start-from=\"2\" HEAD^ 2>&1 >output &&\n     -+\ttest_line_count = 4 output\n     ++\techo 4 >4 &&\n     ++\tgit add 1 2 4 &&\n     ++\tgit commit -a -m \"124\" &&\n     ++\tgit difftool --no-prompt --extcmd=cat --start-from=\"2\" HEAD^  >output &&\n     ++\tcat >expect <<-\\EOF &&\n     ++\t2\n     ++\t4\n     ++\t1\n     ++\tEOF\n     ++\ttest_cmp output expect &&\n     ++\tgit difftool --no-prompt --extcmd=cat --start-from=\"3\" HEAD^  >output &&\n     ++\tcat >expect <<-\\EOF &&\n     ++\t4\n     ++\t1\n     ++\t2\n     ++\tEOF\n     ++\ttest_cmp output expect\n      +'\n     -+\n       test_done\n\n\n Documentation/git-difftool.txt |  3 +++\n Makefile                       |  1 +\n diff.c                         |  6 ++++++\n diff.h                         |  1 +\n diffcore-rotate.c              | 35 ++++++++++++++++++++++++++++++++++\n diffcore.h                     |  1 +\n t/t7800-difftool.sh            | 23 ++++++++++++++++++++++\n 7 files changed, 70 insertions(+)\n create mode 100644 diffcore-rotate.c\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 484c485fd06c..b2bb4e00f683 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -34,6 +34,9 @@ OPTIONS\n \tThis is the default behaviour; the option is provided to\n \toverride any configuration settings.\n \n+--start-from=<file>::\n+\tStart viewing diff from the specified file.\n+\n -t <tool>::\n --tool=<tool>::\n \tUse the diff tool specified by <tool>.  Valid values include\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e009d..6b1f6b72629a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -882,6 +882,7 @@ LIB_OBJS += diffcore-delta.o\n LIB_OBJS += diffcore-order.o\n LIB_OBJS += diffcore-pickaxe.o\n LIB_OBJS += diffcore-rename.o\n+LIB_OBJS += diffcore-rotate.o\n LIB_OBJS += dir-iterator.o\n LIB_OBJS += dir.o\n LIB_OBJS += editor.o\ndiff --git a/diff.c b/diff.c\nindex 69e3bc00ed8f..c41bdcabb791 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5599,6 +5599,10 @@ static void prep_parse_options(struct diff_options *options)\n \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t     N_(\"control the order in which files appear in the output\")),\n+\t\tOPT_STRING(0, \"rotate-to\", &options->rotate_to, N_(\"<path>\"),\n+\t\t\t   N_(\"show the change in the specified path first\")),\n+\t\tOPT_STRING(0, \"start-from\", &options->rotate_to, N_(\"<path>\"),\n+\t\t\t   N_(\"pass from difftool to diff, has the same effort as `rotate-to`\")),\n \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n \t\t\t       N_(\"look for differences that change the number of occurrences of the specified object\"),\n \t\t\t       PARSE_OPT_NONEG, diff_opt_find_object),\n@@ -6669,6 +6673,8 @@ void diffcore_std(struct diff_options *options)\n \t\tdiffcore_pickaxe(options);\n \tif (options->orderfile)\n \t\tdiffcore_order(options->orderfile);\n+\tif (options->rotate_to)\n+\t\tdiffcore_rotate(options->rotate_to);\n \tif (!options->found_follow)\n \t\t/* See try_to_follow_renames() in tree-diff.c */\n \t\tdiff_resolve_rename_copy();\ndiff --git a/diff.h b/diff.h\nindex 2ff2b1c7f2ca..0801469e6390 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -226,6 +226,7 @@ enum diff_submodule_format {\n  */\n struct diff_options {\n \tconst char *orderfile;\n+\tconst char *rotate_to;\n \n \t/**\n \t * A constant string (can and typically does contain newlines to look for\ndiff --git a/diffcore-rotate.c b/diffcore-rotate.c\nnew file mode 100644\nindex 000000000000..1bcfd935cf50\n--- /dev/null\n+++ b/diffcore-rotate.c\n@@ -0,0 +1,35 @@\n+/*\n+ * Copyright (C) 2021, Google LLC.\n+ * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano\n+ */\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+\n+void diffcore_rotate(const char *rotate_to_filename)\n+{\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct diff_queue_struct outq;\n+\tint rotate_to, i;\n+\n+\tif (!q->nr)\n+\t\treturn;\n+\n+\tfor (i = 0; i < q->nr; i++)\n+\t\tif (strcmp(rotate_to_filename, q->queue[i]->two->path) <= 0)\n+\t\t\tbreak;\n+\t/* we did not find the specified path */\n+\tif (q->nr <= i)\n+\t\treturn;\n+\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\trotate_to = i;\n+\n+\tfor (i = rotate_to; i < q->nr; i++)\n+\t\tdiff_q(&outq, q->queue[i]);\n+\tfor (i = 0; i < rotate_to; i++)\n+\t\tdiff_q(&outq, q->queue[i]);\n+\n+\tfree(q->queue);\n+\t*q = outq;\n+}\ndiff --git a/diffcore.h b/diffcore.h\nindex d2a63c5c71f4..bd5959375bf4 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -164,6 +164,7 @@ void diffcore_rename(struct diff_options *);\n void diffcore_merge_broken(void);\n void diffcore_pickaxe(struct diff_options *);\n void diffcore_order(const char *orderfile);\n+void diffcore_rotate(const char *rotate_to_filename);\n \n /* low-level interface to diffcore_order */\n struct obj_order {\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 9662abc1e784..949d07d70fe1 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -762,4 +762,27 @@ test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n \ttest_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n '\n \n+test_expect_success 'difftool --start-from' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\techo 1 >1 &&\n+\techo 2 >2 &&\n+\techo 4 >4 &&\n+\tgit add 1 2 4 &&\n+\tgit commit -a -m \"124\" &&\n+\tgit difftool --no-prompt --extcmd=cat --start-from=\"2\" HEAD^  >output &&\n+\tcat >expect <<-\\EOF &&\n+\t2\n+\t4\n+\t1\n+\tEOF\n+\ttest_cmp output expect &&\n+\tgit difftool --no-prompt --extcmd=cat --start-from=\"3\" HEAD^  >output &&\n+\tcat >expect <<-\\EOF &&\n+\t4\n+\t1\n+\t2\n+\tEOF\n+\ttest_cmp output expect\n+'\n test_done\n\nbase-commit: e6362826a0409539642a5738db61827e5978e2e4\n-- \ngitgitgadget\n"},{"id":"416997","messageId":"xmqqy2foss1j.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.v4.git.1613308167239.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-16T01:46:48Z","receivedAt":"2021-02-16T01:48:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> `git difftool` only allow us to select file to view in turn.\n> If there is a commit with many files and we exit in the search,\n> We will have to traverse list again to get the file diff which\n> we want to see. Therefore, here is a new method: user can use\n> `git difftool --start-from=<filename>` to start viewing from\n> the specified file, This will improve the user experience.\n\nIt appears that you seem to have based this patch on my \"if we were\nto make 'git diff' help you by adding to diffcore pipeline, it would\nlook like this\" illustration patch.\n\nBut I later sent a real version of \"diff --rotate-to/--start-at\"\npatch (and Cc'ed you, IIRC).\n\n  https://lore.kernel.org/git/xmqqo8gqwasu.fsf@gitster.c.googlers.com/\n\nPlease build on top of that patch instead.  E.g.\n\n    $ git fetch\n    $ git checkout -b zh/difftool-start-at fb4bfd0f8b\n\nthen work on that topic branch, perhaps.  You do not have to (and\nyou do not want to) include my changes to your patches.\n\nThanks.\n"},{"id":"417046","messageId":"pull.870.v5.git.1613480198.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v4.git.1613308167239.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-16T12:56:35Z","receivedAt":"2021-02-16T12:57:25Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"git user may should travel the diff list to choice file diff to view, if\nthey exit in midway,they must travel it again. By starting from the\nspecified file method, provides a possibility for this user-friendly\nsolution.\n\nthis patch's origin discuss is here:\nhttps://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/\n\nThanks!\n\nJunio C Hamano (1):\n  diff: --{rotate,skip}-to=<path>\n\nZheNing Hu (1):\n  difftool.c: learn a new way start at specified file\n\n Documentation/diff-options.txt |  8 ++++\n Documentation/git-difftool.txt | 10 +++++\n Documentation/gitdiffcore.txt  | 21 ++++++++++\n Makefile                       |  1 +\n builtin/diff-files.c           |  1 +\n builtin/diff-index.c           |  2 +\n builtin/diff-tree.c            |  3 ++\n builtin/diff.c                 |  1 +\n diff.c                         | 21 ++++++++++\n diff.h                         | 21 ++++++++++\n diffcore-rotate.c              | 46 ++++++++++++++++++++++\n diffcore.h                     |  1 +\n t/t4056-diff-order.sh          | 72 +++++++++++++++++++++++++++++++++-\n t/t7800-difftool.sh            | 30 ++++++++++++++\n 14 files changed, 237 insertions(+), 1 deletion(-)\n create mode 100644 diffcore-rotate.c\n\n\nbase-commit: c6102b758572c7515f606b2423dfe38934fe6764\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\nRange-diff vs v4:\n\n 1:  3accfb942301 ! 1:  fb4bfd0f8b16 difftool.c: learn a new way start at specified file\n     @@\n       ## Metadata ##\n     -Author: ZheNing Hu <adlternative@gmail.com>\n     +Author: Junio C Hamano <gitster@pobox.com>\n      \n       ## Commit message ##\n     -    difftool.c: learn a new way start at specified file\n     -\n     -    `git difftool` only allow us to select file to view in turn.\n     -    If there is a commit with many files and we exit in the search,\n     -    We will have to traverse list again to get the file diff which\n     -    we want to see. Therefore, here is a new method: user can use\n     -    `git difftool --start-from=<filename>` to start viewing from\n     -    the specified file, This will improve the user experience.\n     -\n     -    `difftool --start-from=<file>` will pass the file name to\n     -    `diffcore-rotate`, it will traverse all files in diff_queue,\n     -    if it finds a matching file, it will rearrange the order of\n     -    diff_filepair of diff_queue, Rotate the file specified by the\n     -    user to the first one. If the file name specified by the user\n     -    does not match any item in the diff queue, Git will also rotate\n     -    the queue, it will find the the first file name larger than the\n     -    specified file name as the first element of the new diff_queue.\n     -    This will help users find their mistakes.\n     -\n     -    Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n     -\n     - ## Documentation/git-difftool.txt ##\n     -@@ Documentation/git-difftool.txt: OPTIONS\n     - \tThis is the default behaviour; the option is provided to\n     - \toverride any configuration settings.\n     - \n     -+--start-from=<file>::\n     -+\tStart viewing diff from the specified file.\n     -+\n     - -t <tool>::\n     - --tool=<tool>::\n     - \tUse the diff tool specified by <tool>.  Valid values include\n     +    diff: --{rotate,skip}-to=<path>\n     +\n     +    In the implementation of \"git difftool\", there is a case where the\n     +    user wants to start viewing the diffs at a specific path and\n     +    continue on to the rest, optionally wrapping around to the\n     +    beginning.  Since it is somewhat cumbersome to implement such a\n     +    feature as a post-processing step of \"git diff\" output, let's\n     +    support it internally with two new options.\n     +\n     +     - \"git diff --rotate-to=C\", when the resulting patch would show\n     +       paths A B C D E without the option, would \"rotate\" the paths to\n     +       shows patch to C D E A B instead.  It is an error when there is\n     +       no patch for C is shown.\n     +\n     +     - \"git diff --skip-to=C\" would instead \"skip\" the paths before C,\n     +       and shows patch to C D E.  Again, it is an error when there is no\n     +       patch for C is shown.\n     +\n     +     - \"git log [-p]\" also accepts these two options, but it is not an\n     +       error if there is no change to the specified path.  Instead, the\n     +       set of output paths are rotated or skipped to the specified path\n     +       or the first path that sorts after the specified path.\n     +\n     +    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     +\n     + ## Documentation/diff-options.txt ##\n     +@@ Documentation/diff-options.txt: matches a pattern if removing any number of the final pathname\n     + components matches the pattern.  For example, the pattern \"`foo*bar`\"\n     + matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n     + \n     ++--skip-to=<file>::\n     ++--rotate-to=<file::\n     ++\tDiscard the files before the named <file> from the output\n     ++\t(i.e. 'skip to'), or move them to the end of the output\n     ++\t(i.e. 'rotate to').  These were invented primarily for use\n     ++\tof the `git difftool` command, and may not be very useful\n     ++\totherwise.\n     ++\n     + ifndef::git-format-patch[]\n     + -R::\n     + \tSwap two inputs; that is, show differences from index or\n     +\n     + ## Documentation/gitdiffcore.txt ##\n     +@@ Documentation/gitdiffcore.txt: into another list.  There are currently 5 such transformations:\n     + - diffcore-merge-broken\n     + - diffcore-pickaxe\n     + - diffcore-order\n     ++- diffcore-rotate\n     + \n     + These are applied in sequence.  The set of filepairs 'git diff-{asterisk}'\n     + commands find are used as the input to diffcore-break, and\n     +@@ Documentation/gitdiffcore.txt: Documentation\n     + t\n     + ------------------------------------------------\n     + \n     ++diffcore-rotate: For Changing At Which Path Output Starts\n     ++---------------------------------------------------------\n     ++\n     ++This transformation takes one pathname, and rotates the set of\n     ++filepairs so that the filepair for the given pathname comes first,\n     ++optionally discarding the paths that come before it.  This is used\n     ++to implement the `--skip-to` and the `--rotate-to` options.  It is\n     ++an error when the specified pathname is not in the set of filepairs,\n     ++but it is not useful to error out when used with \"git log\" family of\n     ++commands, because it is unreasonable to expect that a given path\n     ++would be modified by each and every commit shown by the \"git log\"\n     ++command.  For this reason, when used with \"git log\", the filepair\n     ++that sorts the same as, or the first one that sorts after, the given\n     ++pathname is where the output starts.\n     ++\n     ++Use of this transformation combined with diffcore-order will produce\n     ++unexpected results, as the input to this transformation is likely\n     ++not sorted when diffcore-order is in effect.\n     ++\n     ++\n     + SEE ALSO\n     + --------\n     + linkgit:git-diff[1],\n      \n       ## Makefile ##\n      @@ Makefile: LIB_OBJS += diffcore-delta.o\n     @@ Makefile: LIB_OBJS += diffcore-delta.o\n       LIB_OBJS += dir.o\n       LIB_OBJS += editor.o\n      \n     + ## builtin/diff-files.c ##\n     +@@ builtin/diff-files.c: int cmd_diff_files(int argc, const char **argv, const char *prefix)\n     + \t}\n     + \tif (!rev.diffopt.output_format)\n     + \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n     ++\trev.diffopt.rotate_to_strict = 1;\n     + \n     + \t/*\n     + \t * Make sure there are NO revision (i.e. pending object) parameter,\n     +\n     + ## builtin/diff-index.c ##\n     +@@ builtin/diff-index.c: int cmd_diff_index(int argc, const char **argv, const char *prefix)\n     + \tif (!rev.diffopt.output_format)\n     + \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n     + \n     ++\trev.diffopt.rotate_to_strict = 1;\n     ++\n     + \t/*\n     + \t * Make sure there is one revision (i.e. pending object),\n     + \t * and there is no revision filtering parameters.\n     +\n     + ## builtin/diff-tree.c ##\n     +@@ builtin/diff-tree.c: int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n     + \tif (merge_base && opt->pending.nr != 2)\n     + \t\tdie(_(\"--merge-base only works with two commits\"));\n     + \n     ++\topt->diffopt.rotate_to_strict = 1;\n     ++\n     + \t/*\n     + \t * NOTE!  We expect \"a..b\" to expand to \"^a b\" but it is\n     + \t * perfectly valid for revision range parser to yield \"b ^a\",\n     +@@ builtin/diff-tree.c: int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n     + \t\tint saved_nrl = 0;\n     + \t\tint saved_dcctc = 0;\n     + \n     ++\t\topt->diffopt.rotate_to_strict = 0;\n     + \t\tif (opt->diffopt.detect_rename) {\n     + \t\t\tif (!the_index.cache)\n     + \t\t\t\trepo_read_index(the_repository);\n     +\n     + ## builtin/diff.c ##\n     +@@ builtin/diff.c: int cmd_diff(int argc, const char **argv, const char *prefix)\n     + \t}\n     + \n     + \trev.diffopt.flags.recursive = 1;\n     ++\trev.diffopt.rotate_to_strict = 1;\n     + \n     + \tsetup_diff_pager(&rev.diffopt);\n     + \n     +\n       ## diff.c ##\n     +@@ diff.c: static int diff_opt_word_diff_regex(const struct option *opt,\n     + \treturn 0;\n     + }\n     + \n     ++static int diff_opt_rotate_to(const struct option *opt, const char *arg, int unset)\n     ++{\n     ++\tstruct diff_options *options = opt->value;\n     ++\n     ++\tBUG_ON_OPT_NEG(unset);\n     ++\tif (!strcmp(opt->long_name, \"skip-to\"))\n     ++\t\toptions->skip_instead_of_rotate = 1;\n     ++\telse\n     ++\t\toptions->skip_instead_of_rotate = 0;\n     ++\toptions->rotate_to = arg;\n     ++\treturn 0;\n     ++}\n     ++\n     + static void prep_parse_options(struct diff_options *options)\n     + {\n     + \tstruct option parseopts[] = {\n      @@ diff.c: static void prep_parse_options(struct diff_options *options)\n       \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n       \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n       \t\t\t     N_(\"control the order in which files appear in the output\")),\n     -+\t\tOPT_STRING(0, \"rotate-to\", &options->rotate_to, N_(\"<path>\"),\n     -+\t\t\t   N_(\"show the change in the specified path first\")),\n     -+\t\tOPT_STRING(0, \"start-from\", &options->rotate_to, N_(\"<path>\"),\n     -+\t\t\t   N_(\"pass from difftool to diff, has the same effort as `rotate-to`\")),\n     ++\t\tOPT_CALLBACK_F(0, \"rotate-to\", options, N_(\"<path>\"),\n     ++\t\t\t       N_(\"show the change in the specified path first\"),\n     ++\t\t\t       PARSE_OPT_NONEG, diff_opt_rotate_to),\n     ++\t\tOPT_CALLBACK_F(0, \"skip-to\", options, N_(\"<path>\"),\n     ++\t\t\t       N_(\"skip the output to the specified path\"),\n     ++\t\t\t       PARSE_OPT_NONEG, diff_opt_rotate_to),\n       \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n       \t\t\t       N_(\"look for differences that change the number of occurrences of the specified object\"),\n       \t\t\t       PARSE_OPT_NONEG, diff_opt_find_object),\n     @@ diff.c: void diffcore_std(struct diff_options *options)\n       \tif (options->orderfile)\n       \t\tdiffcore_order(options->orderfile);\n      +\tif (options->rotate_to)\n     -+\t\tdiffcore_rotate(options->rotate_to);\n     ++\t\tdiffcore_rotate(options);\n       \tif (!options->found_follow)\n       \t\t/* See try_to_follow_renames() in tree-diff.c */\n       \t\tdiff_resolve_rename_copy();\n      \n       ## diff.h ##\n      @@ diff.h: enum diff_submodule_format {\n     -  */\n       struct diff_options {\n       \tconst char *orderfile;\n     -+\tconst char *rotate_to;\n       \n     ++\t/*\n     ++\t * \"--rotate-to=<file>\" would start showing at <file> and when\n     ++\t * the output reaches the end, wrap around by default.\n     ++\t * Setting skip_instead_of_rotate to true stops the output at the\n     ++\t * end, effectively discarding the earlier part of the output\n     ++\t * before <file>'s diff (this is used to implement the\n     ++\t * \"--skip-to=<file>\" option).\n     ++\t *\n     ++\t * When rotate_to_strict is set, it is an error if there is no\n     ++\t * <file> in the diff.  Otherwise, the output starts at the\n     ++\t * path that is the same as, or first path that sorts after,\n     ++\t * <file>.  Because it is unreasonable to require the exact\n     ++\t * match for \"git log -p --rotate-to=<file>\" (i.e. not all\n     ++\t * commit would touch that single <file>), \"git log\" sets it\n     ++\t * to false.  \"git diff\" sets it to true to detect an error\n     ++\t * in the command line option.\n     ++\t */\n     ++\tconst char *rotate_to;\n     ++\tint skip_instead_of_rotate;\n     ++\tint rotate_to_strict;\n     ++\n       \t/**\n       \t * A constant string (can and typically does contain newlines to look for\n     + \t * a block of text, not just a single line) to filter out the filepairs\n      \n       ## diffcore-rotate.c (new) ##\n      @@\n     @@ diffcore-rotate.c (new)\n      +#include \"diff.h\"\n      +#include \"diffcore.h\"\n      +\n     -+void diffcore_rotate(const char *rotate_to_filename)\n     ++void diffcore_rotate(struct diff_options *opt)\n      +{\n      +\tstruct diff_queue_struct *q = &diff_queued_diff;\n      +\tstruct diff_queue_struct outq;\n     @@ diffcore-rotate.c (new)\n      +\tif (!q->nr)\n      +\t\treturn;\n      +\n     -+\tfor (i = 0; i < q->nr; i++)\n     -+\t\tif (strcmp(rotate_to_filename, q->queue[i]->two->path) <= 0)\n     -+\t\t\tbreak;\n     -+\t/* we did not find the specified path */\n     -+\tif (q->nr <= i)\n     ++\tfor (i = 0; i < q->nr; i++) {\n     ++\t\tint cmp = strcmp(opt->rotate_to, q->queue[i]->two->path);\n     ++\t\tif (!cmp)\n     ++\t\t\tbreak; /* exact match */\n     ++\t\tif (!opt->rotate_to_strict && cmp < 0)\n     ++\t\t\tbreak; /* q->queue[i] is now past the target pathname */\n     ++\t}\n     ++\n     ++\tif (q->nr <= i) {\n     ++\t\t/* we did not find the specified path */\n     ++\t\tif (opt->rotate_to_strict)\n     ++\t\t\tdie(_(\"No such path '%s' in the diff\"), opt->rotate_to);\n      +\t\treturn;\n     ++\t}\n      +\n      +\tDIFF_QUEUE_CLEAR(&outq);\n      +\trotate_to = i;\n      +\n      +\tfor (i = rotate_to; i < q->nr; i++)\n      +\t\tdiff_q(&outq, q->queue[i]);\n     -+\tfor (i = 0; i < rotate_to; i++)\n     -+\t\tdiff_q(&outq, q->queue[i]);\n     -+\n     ++\tfor (i = 0; i < rotate_to; i++) {\n     ++\t\tif (opt->skip_instead_of_rotate)\n     ++\t\t\tdiff_free_filepair(q->queue[i]);\n     ++\t\telse\n     ++\t\t\tdiff_q(&outq, q->queue[i]);\n     ++\t}\n      +\tfree(q->queue);\n      +\t*q = outq;\n      +}\n     @@ diffcore.h: void diffcore_rename(struct diff_options *);\n       void diffcore_merge_broken(void);\n       void diffcore_pickaxe(struct diff_options *);\n       void diffcore_order(const char *orderfile);\n     -+void diffcore_rotate(const char *rotate_to_filename);\n     ++void diffcore_rotate(struct diff_options *);\n       \n       /* low-level interface to diffcore_order */\n       struct obj_order {\n      \n     - ## t/t7800-difftool.sh ##\n     -@@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n     - \ttest_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n     - '\n     - \n     -+test_expect_success 'difftool --start-from' '\n     -+\tdifftool_test_setup &&\n     -+\ttest_when_finished git reset --hard &&\n     -+\techo 1 >1 &&\n     -+\techo 2 >2 &&\n     -+\techo 4 >4 &&\n     -+\tgit add 1 2 4 &&\n     -+\tgit commit -a -m \"124\" &&\n     -+\tgit difftool --no-prompt --extcmd=cat --start-from=\"2\" HEAD^  >output &&\n     + ## t/t4056-diff-order.sh ##\n     +@@\n     + #!/bin/sh\n     + \n     +-test_description='diff order'\n     ++test_description='diff order & rotate'\n     + \n     + GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n     + export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n     +@@ t/t4056-diff-order.sh: do\n     + \t'\n     + done\n     + \n     ++### rotate and skip\n     ++\n     ++test_expect_success 'rotate and skip setup' '\n     ++\t>sample1.t &&\n     ++\t>sample2.t &&\n     ++\t>sample3.t &&\n     ++\t>sample4.t &&\n     ++\tgit add sample[1234].t &&\n     ++\tgit commit -m \"added\" sample[1234].t &&\n     ++\techo modified >>sample1.t &&\n     ++\techo modified >>sample2.t &&\n     ++\techo modified >>sample4.t &&\n     ++\tgit commit -m \"updated\" sample[1234].t\n     ++'\n     ++\n     ++test_expect_success 'diff --rotate-to' '\n     ++\tgit diff --rotate-to=sample2.t --name-only HEAD^ >actual &&\n     ++\ttest_write_lines sample2.t sample4.t sample1.t >expect &&\n     ++\ttest_cmp expect actual\n     ++'\n     ++\n     ++test_expect_success 'diff --skip-to' '\n     ++\tgit diff --skip-to=sample2.t --name-only HEAD^ >actual &&\n     ++\ttest_write_lines sample2.t sample4.t >expect &&\n     ++\ttest_cmp expect actual\n     ++'\n     ++\n     ++test_expect_success 'diff --rotate/skip-to error condition' '\n     ++\ttest_must_fail git diff --rotate-to=sample3.t HEAD^ &&\n     ++\ttest_must_fail git diff --skip-to=sample3.t HEAD^\n     ++'\n     ++\n     ++test_expect_success 'log --rotate-to' '\n     ++\tgit log --rotate-to=sample3.t --raw HEAD~2.. >raw &&\n     ++\t# just distill the commit header and paths\n     ++\tsed -n -e \"s/^commit.*/commit/p\" \\\n     ++\t       -e \"/^:/s/^.*\t//p\" raw >actual &&\n     ++\n      +\tcat >expect <<-\\EOF &&\n     -+\t2\n     -+\t4\n     -+\t1\n     ++\tcommit\n     ++\tsample4.t\n     ++\tsample1.t\n     ++\tsample2.t\n     ++\tcommit\n     ++\tsample3.t\n     ++\tsample4.t\n     ++\tsample1.t\n     ++\tsample2.t\n      +\tEOF\n     -+\ttest_cmp output expect &&\n     -+\tgit difftool --no-prompt --extcmd=cat --start-from=\"3\" HEAD^  >output &&\n     ++\n     ++\ttest_cmp expect actual\n     ++'\n     ++\n     ++test_expect_success 'log --skip-to' '\n     ++\tgit log --skip-to=sample3.t --raw HEAD~2.. >raw &&\n     ++\t# just distill the commit header and paths\n     ++\tsed -n -e \"s/^commit.*/commit/p\" \\\n     ++\t       -e \"/^:/s/^.*\t//p\" raw >actual &&\n     ++\n      +\tcat >expect <<-\\EOF &&\n     -+\t4\n     -+\t1\n     -+\t2\n     ++\tcommit\n     ++\tsample4.t\n     ++\tcommit\n     ++\tsample3.t\n     ++\tsample4.t\n      +\tEOF\n     -+\ttest_cmp output expect\n     ++\n     ++\ttest_cmp expect actual\n      +'\n     ++\n       test_done\n -:  ------------ > 2:  98e2707ee2fa difftool.c: learn a new way start at specified file\n\n-- \ngitgitgadget\n"},{"id":"417047","messageId":"fb4bfd0f8b162e51e71711fe5503ca684f980d58.1613480198.git.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v5.git.1613480198.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] diff: --{rotate,skip}-to=<path>","fromName":"Junio C Hamano via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-16T12:56:36Z","receivedAt":"2021-02-16T12:57:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nIn the implementation of \"git difftool\", there is a case where the\nuser wants to start viewing the diffs at a specific path and\ncontinue on to the rest, optionally wrapping around to the\nbeginning.  Since it is somewhat cumbersome to implement such a\nfeature as a post-processing step of \"git diff\" output, let's\nsupport it internally with two new options.\n\n - \"git diff --rotate-to=C\", when the resulting patch would show\n   paths A B C D E without the option, would \"rotate\" the paths to\n   shows patch to C D E A B instead.  It is an error when there is\n   no patch for C is shown.\n\n - \"git diff --skip-to=C\" would instead \"skip\" the paths before C,\n   and shows patch to C D E.  Again, it is an error when there is no\n   patch for C is shown.\n\n - \"git log [-p]\" also accepts these two options, but it is not an\n   error if there is no change to the specified path.  Instead, the\n   set of output paths are rotated or skipped to the specified path\n   or the first path that sorts after the specified path.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/diff-options.txt |  8 ++++\n Documentation/gitdiffcore.txt  | 21 ++++++++++\n Makefile                       |  1 +\n builtin/diff-files.c           |  1 +\n builtin/diff-index.c           |  2 +\n builtin/diff-tree.c            |  3 ++\n builtin/diff.c                 |  1 +\n diff.c                         | 21 ++++++++++\n diff.h                         | 21 ++++++++++\n diffcore-rotate.c              | 46 ++++++++++++++++++++++\n diffcore.h                     |  1 +\n t/t4056-diff-order.sh          | 72 +++++++++++++++++++++++++++++++++-\n 12 files changed, 197 insertions(+), 1 deletion(-)\n create mode 100644 diffcore-rotate.c\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex e5733ccb2d1a..7c5b3cf42bcc 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -700,6 +700,14 @@ matches a pattern if removing any number of the final pathname\n components matches the pattern.  For example, the pattern \"`foo*bar`\"\n matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n \n+--skip-to=<file>::\n+--rotate-to=<file::\n+\tDiscard the files before the named <file> from the output\n+\t(i.e. 'skip to'), or move them to the end of the output\n+\t(i.e. 'rotate to').  These were invented primarily for use\n+\tof the `git difftool` command, and may not be very useful\n+\totherwise.\n+\n ifndef::git-format-patch[]\n -R::\n \tSwap two inputs; that is, show differences from index or\ndiff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt\nindex c970d9fe438a..2bd1220477e5 100644\n--- a/Documentation/gitdiffcore.txt\n+++ b/Documentation/gitdiffcore.txt\n@@ -74,6 +74,7 @@ into another list.  There are currently 5 such transformations:\n - diffcore-merge-broken\n - diffcore-pickaxe\n - diffcore-order\n+- diffcore-rotate\n \n These are applied in sequence.  The set of filepairs 'git diff-{asterisk}'\n commands find are used as the input to diffcore-break, and\n@@ -276,6 +277,26 @@ Documentation\n t\n ------------------------------------------------\n \n+diffcore-rotate: For Changing At Which Path Output Starts\n+---------------------------------------------------------\n+\n+This transformation takes one pathname, and rotates the set of\n+filepairs so that the filepair for the given pathname comes first,\n+optionally discarding the paths that come before it.  This is used\n+to implement the `--skip-to` and the `--rotate-to` options.  It is\n+an error when the specified pathname is not in the set of filepairs,\n+but it is not useful to error out when used with \"git log\" family of\n+commands, because it is unreasonable to expect that a given path\n+would be modified by each and every commit shown by the \"git log\"\n+command.  For this reason, when used with \"git log\", the filepair\n+that sorts the same as, or the first one that sorts after, the given\n+pathname is where the output starts.\n+\n+Use of this transformation combined with diffcore-order will produce\n+unexpected results, as the input to this transformation is likely\n+not sorted when diffcore-order is in effect.\n+\n+\n SEE ALSO\n --------\n linkgit:git-diff[1],\ndiff --git a/Makefile b/Makefile\nindex 5a239cac20e3..9b1bde2e0e64 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -863,6 +863,7 @@ LIB_OBJS += diffcore-delta.o\n LIB_OBJS += diffcore-order.o\n LIB_OBJS += diffcore-pickaxe.o\n LIB_OBJS += diffcore-rename.o\n+LIB_OBJS += diffcore-rotate.o\n LIB_OBJS += dir-iterator.o\n LIB_OBJS += dir.o\n LIB_OBJS += editor.o\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex 4742a4559b21..e037efb07eff 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -54,6 +54,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \t}\n \tif (!rev.diffopt.output_format)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n+\trev.diffopt.rotate_to_strict = 1;\n \n \t/*\n \t * Make sure there are NO revision (i.e. pending object) parameter,\ndiff --git a/builtin/diff-index.c b/builtin/diff-index.c\nindex 7f5281c46168..06635e8fb26f 100644\n--- a/builtin/diff-index.c\n+++ b/builtin/diff-index.c\n@@ -41,6 +41,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)\n \tif (!rev.diffopt.output_format)\n \t\trev.diffopt.output_format = DIFF_FORMAT_RAW;\n \n+\trev.diffopt.rotate_to_strict = 1;\n+\n \t/*\n \t * Make sure there is one revision (i.e. pending object),\n \t * and there is no revision filtering parameters.\ndiff --git a/builtin/diff-tree.c b/builtin/diff-tree.c\nindex 9fc95e959f0e..b6a9a9328e88 100644\n--- a/builtin/diff-tree.c\n+++ b/builtin/diff-tree.c\n@@ -156,6 +156,8 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \tif (merge_base && opt->pending.nr != 2)\n \t\tdie(_(\"--merge-base only works with two commits\"));\n \n+\topt->diffopt.rotate_to_strict = 1;\n+\n \t/*\n \t * NOTE!  We expect \"a..b\" to expand to \"^a b\" but it is\n \t * perfectly valid for revision range parser to yield \"b ^a\",\n@@ -192,6 +194,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)\n \t\tint saved_nrl = 0;\n \t\tint saved_dcctc = 0;\n \n+\t\topt->diffopt.rotate_to_strict = 0;\n \t\tif (opt->diffopt.detect_rename) {\n \t\t\tif (!the_index.cache)\n \t\t\t\trepo_read_index(the_repository);\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 5cfe1717e8de..f1b88c7389eb 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -491,6 +491,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t}\n \n \trev.diffopt.flags.recursive = 1;\n+\trev.diffopt.rotate_to_strict = 1;\n \n \tsetup_diff_pager(&rev.diffopt);\n \ndiff --git a/diff.c b/diff.c\nindex 69e3bc00ed8f..71e473854842 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -5348,6 +5348,19 @@ static int diff_opt_word_diff_regex(const struct option *opt,\n \treturn 0;\n }\n \n+static int diff_opt_rotate_to(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct diff_options *options = opt->value;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\tif (!strcmp(opt->long_name, \"skip-to\"))\n+\t\toptions->skip_instead_of_rotate = 1;\n+\telse\n+\t\toptions->skip_instead_of_rotate = 0;\n+\toptions->rotate_to = arg;\n+\treturn 0;\n+}\n+\n static void prep_parse_options(struct diff_options *options)\n {\n \tstruct option parseopts[] = {\n@@ -5599,6 +5612,12 @@ static void prep_parse_options(struct diff_options *options)\n \t\t\t  DIFF_PICKAXE_REGEX, PARSE_OPT_NONEG),\n \t\tOPT_FILENAME('O', NULL, &options->orderfile,\n \t\t\t     N_(\"control the order in which files appear in the output\")),\n+\t\tOPT_CALLBACK_F(0, \"rotate-to\", options, N_(\"<path>\"),\n+\t\t\t       N_(\"show the change in the specified path first\"),\n+\t\t\t       PARSE_OPT_NONEG, diff_opt_rotate_to),\n+\t\tOPT_CALLBACK_F(0, \"skip-to\", options, N_(\"<path>\"),\n+\t\t\t       N_(\"skip the output to the specified path\"),\n+\t\t\t       PARSE_OPT_NONEG, diff_opt_rotate_to),\n \t\tOPT_CALLBACK_F(0, \"find-object\", options, N_(\"<object-id>\"),\n \t\t\t       N_(\"look for differences that change the number of occurrences of the specified object\"),\n \t\t\t       PARSE_OPT_NONEG, diff_opt_find_object),\n@@ -6669,6 +6688,8 @@ void diffcore_std(struct diff_options *options)\n \t\tdiffcore_pickaxe(options);\n \tif (options->orderfile)\n \t\tdiffcore_order(options->orderfile);\n+\tif (options->rotate_to)\n+\t\tdiffcore_rotate(options);\n \tif (!options->found_follow)\n \t\t/* See try_to_follow_renames() in tree-diff.c */\n \t\tdiff_resolve_rename_copy();\ndiff --git a/diff.h b/diff.h\nindex 2ff2b1c7f2ca..45300e3597f2 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -227,6 +227,27 @@ enum diff_submodule_format {\n struct diff_options {\n \tconst char *orderfile;\n \n+\t/*\n+\t * \"--rotate-to=<file>\" would start showing at <file> and when\n+\t * the output reaches the end, wrap around by default.\n+\t * Setting skip_instead_of_rotate to true stops the output at the\n+\t * end, effectively discarding the earlier part of the output\n+\t * before <file>'s diff (this is used to implement the\n+\t * \"--skip-to=<file>\" option).\n+\t *\n+\t * When rotate_to_strict is set, it is an error if there is no\n+\t * <file> in the diff.  Otherwise, the output starts at the\n+\t * path that is the same as, or first path that sorts after,\n+\t * <file>.  Because it is unreasonable to require the exact\n+\t * match for \"git log -p --rotate-to=<file>\" (i.e. not all\n+\t * commit would touch that single <file>), \"git log\" sets it\n+\t * to false.  \"git diff\" sets it to true to detect an error\n+\t * in the command line option.\n+\t */\n+\tconst char *rotate_to;\n+\tint skip_instead_of_rotate;\n+\tint rotate_to_strict;\n+\n \t/**\n \t * A constant string (can and typically does contain newlines to look for\n \t * a block of text, not just a single line) to filter out the filepairs\ndiff --git a/diffcore-rotate.c b/diffcore-rotate.c\nnew file mode 100644\nindex 000000000000..445f060ab001\n--- /dev/null\n+++ b/diffcore-rotate.c\n@@ -0,0 +1,46 @@\n+/*\n+ * Copyright (C) 2021, Google LLC.\n+ * Based on diffcore-order.c, which is Copyright (C) 2005, Junio C Hamano\n+ */\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+\n+void diffcore_rotate(struct diff_options *opt)\n+{\n+\tstruct diff_queue_struct *q = &diff_queued_diff;\n+\tstruct diff_queue_struct outq;\n+\tint rotate_to, i;\n+\n+\tif (!q->nr)\n+\t\treturn;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tint cmp = strcmp(opt->rotate_to, q->queue[i]->two->path);\n+\t\tif (!cmp)\n+\t\t\tbreak; /* exact match */\n+\t\tif (!opt->rotate_to_strict && cmp < 0)\n+\t\t\tbreak; /* q->queue[i] is now past the target pathname */\n+\t}\n+\n+\tif (q->nr <= i) {\n+\t\t/* we did not find the specified path */\n+\t\tif (opt->rotate_to_strict)\n+\t\t\tdie(_(\"No such path '%s' in the diff\"), opt->rotate_to);\n+\t\treturn;\n+\t}\n+\n+\tDIFF_QUEUE_CLEAR(&outq);\n+\trotate_to = i;\n+\n+\tfor (i = rotate_to; i < q->nr; i++)\n+\t\tdiff_q(&outq, q->queue[i]);\n+\tfor (i = 0; i < rotate_to; i++) {\n+\t\tif (opt->skip_instead_of_rotate)\n+\t\t\tdiff_free_filepair(q->queue[i]);\n+\t\telse\n+\t\t\tdiff_q(&outq, q->queue[i]);\n+\t}\n+\tfree(q->queue);\n+\t*q = outq;\n+}\ndiff --git a/diffcore.h b/diffcore.h\nindex d2a63c5c71f4..c1592bcd0135 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -164,6 +164,7 @@ void diffcore_rename(struct diff_options *);\n void diffcore_merge_broken(void);\n void diffcore_pickaxe(struct diff_options *);\n void diffcore_order(const char *orderfile);\n+void diffcore_rotate(struct diff_options *);\n \n /* low-level interface to diffcore_order */\n struct obj_order {\ndiff --git a/t/t4056-diff-order.sh b/t/t4056-diff-order.sh\nindex 63ea7144bb49..aec1d9d1b42f 100755\n--- a/t/t4056-diff-order.sh\n+++ b/t/t4056-diff-order.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='diff order'\n+test_description='diff order & rotate'\n \n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n@@ -127,4 +127,74 @@ do\n \t'\n done\n \n+### rotate and skip\n+\n+test_expect_success 'rotate and skip setup' '\n+\t>sample1.t &&\n+\t>sample2.t &&\n+\t>sample3.t &&\n+\t>sample4.t &&\n+\tgit add sample[1234].t &&\n+\tgit commit -m \"added\" sample[1234].t &&\n+\techo modified >>sample1.t &&\n+\techo modified >>sample2.t &&\n+\techo modified >>sample4.t &&\n+\tgit commit -m \"updated\" sample[1234].t\n+'\n+\n+test_expect_success 'diff --rotate-to' '\n+\tgit diff --rotate-to=sample2.t --name-only HEAD^ >actual &&\n+\ttest_write_lines sample2.t sample4.t sample1.t >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --skip-to' '\n+\tgit diff --skip-to=sample2.t --name-only HEAD^ >actual &&\n+\ttest_write_lines sample2.t sample4.t >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --rotate/skip-to error condition' '\n+\ttest_must_fail git diff --rotate-to=sample3.t HEAD^ &&\n+\ttest_must_fail git diff --skip-to=sample3.t HEAD^\n+'\n+\n+test_expect_success 'log --rotate-to' '\n+\tgit log --rotate-to=sample3.t --raw HEAD~2.. >raw &&\n+\t# just distill the commit header and paths\n+\tsed -n -e \"s/^commit.*/commit/p\" \\\n+\t       -e \"/^:/s/^.*\t//p\" raw >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tcommit\n+\tsample4.t\n+\tsample1.t\n+\tsample2.t\n+\tcommit\n+\tsample3.t\n+\tsample4.t\n+\tsample1.t\n+\tsample2.t\n+\tEOF\n+\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'log --skip-to' '\n+\tgit log --skip-to=sample3.t --raw HEAD~2.. >raw &&\n+\t# just distill the commit header and paths\n+\tsed -n -e \"s/^commit.*/commit/p\" \\\n+\t       -e \"/^:/s/^.*\t//p\" raw >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tcommit\n+\tsample4.t\n+\tcommit\n+\tsample3.t\n+\tsample4.t\n+\tEOF\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"417048","messageId":"98e2707ee2faf653e972b0706311ddd099765ce5.1613480198.git.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v5.git.1613480198.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-16T12:56:37Z","receivedAt":"2021-02-16T12:57:38Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view in turn.\nIf there is a commit with many files and we exit in the search,\nWe will have to traverse list again to get the file diff which\nwe want to see. Therefore, here is a new method: user can use\n`git difftool --rotate-to=<filename>` or `git difftool --skip-to=<filename>`\nto start viewing from the specified file, This will improve the\nuser experience.\n\n`git difftool --rotate-to=<file>` or `git difftool --skip-to=<filename>`\nwill pass the path to `diffcore-rotate`, and diff-core will\nadjust the order of files, make the specified file sorted to\nthe first.`git difftool --rotate-to=<file>` will move files before\nthe  specified path to the last output, and\n`git difftool --skip-to=<filename>`  will ignore these files output.\nIt is an error when there is no patch for specified file is shown.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n Documentation/diff-options.txt |  2 +-\n Documentation/git-difftool.txt | 10 ++++++++++\n t/t7800-difftool.sh            | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 7c5b3cf42bcc..aa2b5c11f20b 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -701,7 +701,7 @@ components matches the pattern.  For example, the pattern \"`foo*bar`\"\n matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n \n --skip-to=<file>::\n---rotate-to=<file::\n+--rotate-to=<file>::\n \tDiscard the files before the named <file> from the output\n \t(i.e. 'skip to'), or move them to the end of the output\n \t(i.e. 'rotate to').  These were invented primarily for use\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 484c485fd06c..c64dff69c976 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -34,6 +34,16 @@ OPTIONS\n \tThis is the default behaviour; the option is provided to\n \toverride any configuration settings.\n \n+--rotate-to=<file>::\n+\tInternally call `git diff --rotate-to=<file>`,\n+\tshow the change in the specified path first.\n+\tFiles before the specified path will be moved to the last output.\n+\n+--skip-to=<file>::\n+\tInternally call `git diff --skip-to=<file>`,\n+\tskip the output to the specified path.\n+\tFiles before the specified path will not output.\n+\n -t <tool>::\n --tool=<tool>::\n \tUse the diff tool specified by <tool>.  Valid values include\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 9192c141ffc6..112b798b1c23 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -762,4 +762,34 @@ test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n \ttest_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n '\n \n+test_expect_success 'difftool --rotate-to' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\techo 1 >1 &&\n+\techo 2 >2 &&\n+\techo 4 >4 &&\n+\tgit add 1 2 4 &&\n+\tgit commit -a -m \"124\" &&\n+\tgit difftool --no-prompt --extcmd=cat --rotate-to=\"2\" HEAD^ >output&&\n+\tcat >expect <<-\\EOF &&\n+\t2\n+\t4\n+\t1\n+\tEOF\n+\ttest_cmp output expect &&\n+\ttest_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^\n+'\n+\n+test_expect_success 'difftool --skip-to' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\tgit difftool --no-prompt --extcmd=cat --skip-to=\"2\" HEAD^ >output &&\n+\tcat >expect <<-\\EOF &&\n+\t2\n+\t4\n+\tEOF\n+\ttest_cmp output expect &&\n+\ttest_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n+'\n+\n test_done\n-- \ngitgitgadget\n"},{"id":"417089","messageId":"xmqqblcjrgvc.fsf@gitster.c.googlers.com","threadId":"55116","inReplyTo":"pull.870.v5.git.1613480198.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-16T18:45:43Z","receivedAt":"2021-02-16T18:46:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> Junio C Hamano (1):\n>   diff: --{rotate,skip}-to=<path>\n\nThat's not part of your series (didn't I ask you not to include it)?\n\n> ZheNing Hu (1):\n>   difftool.c: learn a new way start at specified file\n\nWill see what I find, but I may not be able to get to it today.\n\nThanks.\n"},{"id":"417142","messageId":"CAOLTT8T=R-M1eK9thSuzHNOJ8wkaTX3yYsLEgpqmHiEYWgM1XA@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqblcjrgvc.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-17T04:12:10Z","receivedAt":"2021-02-17T04:15:18Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Oh, I am sorry.\nThen I only need to squash the two commit, right?\n\nJunio C Hamano <gitster@pobox.com> 于2021年2月17日周三 上午2:45写道：\n>\n> \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > Junio C Hamano (1):\n> >   diff: --{rotate,skip}-to=<path>\n>\n> That's not part of your series (didn't I ask you not to include it)?\n>\n> > ZheNing Hu (1):\n> >   difftool.c: learn a new way start at specified file\n>\n> Will see what I find, but I may not be able to get to it today.\n>\n> Thanks.\n"},{"id":"417151","messageId":"xmqq8s7nm1dv.fsf@gitster.g","threadId":"55116","inReplyTo":"98e2707ee2faf653e972b0706311ddd099765ce5.1613480198.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 2/2] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-17T10:31:24Z","receivedAt":"2021-02-17T10:32:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> `git difftool` only allow us to select file to view in turn.\n> If there is a commit with many files and we exit in the search,\n\nI am not sure what \"in the search\" refers to.  \"in the middle\" I\nwould understand, though.\n\n> We will have to traverse list again to get the file diff which\n\nLet's downcase this \"We\".\n\n> we want to see. Therefore, here is a new method: user can use\n> `git difftool --rotate-to=<filename>` or `git difftool --skip-to=<filename>`\n> to start viewing from the specified file, This will improve the\n> user experience.\n\nDo we need both?  I'd rather not to give end-user-facing commands\ntoo many knobs that would do similar things.  Too many choices to\nchoose from without clear answer to \"which one should I prefer to\nuse?\" is a bad combination for end-users.\n\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 7c5b3cf42bcc..aa2b5c11f20b 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -701,7 +701,7 @@ components matches the pattern.  For example, the pattern \"`foo*bar`\"\n>  matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n>  \n>  --skip-to=<file>::\n> ---rotate-to=<file::\n> +--rotate-to=<file>::\n>  \tDiscard the files before the named <file> from the output\n>  \t(i.e. 'skip to'), or move them to the end of the output\n>  \t(i.e. 'rotate to').  These were invented primarily for use\n\nThanks for correcting, but this change should not be a part of this\npatch.  Instead, you help the other's topic by giving a review (and\nyou could just have said \"there there is closing '>' missing\").\n\n> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\n> index 484c485fd06c..c64dff69c976 100644\n> --- a/Documentation/git-difftool.txt\n> +++ b/Documentation/git-difftool.txt\n> @@ -34,6 +34,16 @@ OPTIONS\n>  \tThis is the default behaviour; the option is provided to\n>  \toverride any configuration settings.\n>  \n> +--rotate-to=<file>::\n> +\tInternally call `git diff --rotate-to=<file>`,\n> +\tshow the change in the specified path first.\n> +\tFiles before the specified path will be moved to the last output.\n> +\n> +--skip-to=<file>::\n> +\tInternally call `git diff --skip-to=<file>`,\n> +\tskip the output to the specified path.\n> +\tFiles before the specified path will not output.\n> +\n\nThis, unlike the \"diffcore\" stuff, is end-user facing, and it is\nbetter not to force the readers even know what --skip-to option\nto the diff does (after all, difftool users are using 'git difftool'\nand they are not necessarily 'git diff' users).\n\n    --skip-to=<file>::\n            Start showing the diff for the given path, skipping all\n            the paths before it.\n\nor something, perhaps.\n\n> +test_expect_success 'difftool --skip-to' '\n> +\tdifftool_test_setup &&\n> +\ttest_when_finished git reset --hard &&\n> +\tgit difftool --no-prompt --extcmd=cat --skip-to=\"2\" HEAD^ >output &&\n> +\tcat >expect <<-\\EOF &&\n> +\t2\n> +\t4\n> +\tEOF\n> +\ttest_cmp output expect &&\n> +\ttest_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n> +'\n\nThis probably should be split into two independent tests.  One to\ncheck that the non-failing case works as expected, the other to\ncheck that a bogus command line option errors out as expected.\n\nThanks.\n"},{"id":"417153","messageId":"YCz6oDZCAODPS8sY@generichostname","threadId":"55116","inReplyTo":"CAOLTT8T=R-M1eK9thSuzHNOJ8wkaTX3yYsLEgpqmHiEYWgM1XA@mail.gmail.com","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-02-17T11:14:40Z","receivedAt":"2021-02-17T11:16:03Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi ZheNing,\n\nOn Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n> Oh, I am sorry.\n> Then I only need to squash the two commit, right?\n\nI've never used GGG before but I suspect that in your GitHub PR, you\nneed to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n\nCCing the creator of GGG, please correct me if I'm wrong.\n\n-Denton\n\n> Junio C Hamano <gitster@pobox.com> 于2021年2月17日周三 上午2:45写道：\n> >\n> > \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> >\n> > > Junio C Hamano (1):\n> > >   diff: --{rotate,skip}-to=<path>\n> >\n> > That's not part of your series (didn't I ask you not to include it)?\n> >\n> > > ZheNing Hu (1):\n> > >   difftool.c: learn a new way start at specified file\n> >\n> > Will see what I find, but I may not be able to get to it today.\n> >\n> > Thanks.\n"},{"id":"417155","messageId":"CAOLTT8Ri+XbSg_=KaLOCmNX4Nrii1ssN9_FFbnmm7ew4vYN5nA@mail.gmail.com","threadId":"55116","inReplyTo":"YCz6oDZCAODPS8sY@generichostname","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-17T11:40:23Z","receivedAt":"2021-02-17T11:43:55Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi Denton Liu,\nYou mean I should cherry-pick Junio's patch to my topic branch, right?\n\nDenton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n>\n> Hi ZheNing,\n>\n> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n> > Oh, I am sorry.\n> > Then I only need to squash the two commit, right?\n>\n> I've never used GGG before but I suspect that in your GitHub PR, you\n> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n>\n> CCing the creator of GGG, please correct me if I'm wrong.\n>\n> -Denton\n>\n> > Junio C Hamano <gitster@pobox.com> 于2021年2月17日周三 上午2:45写道：\n> > >\n> > > \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> > >\n> > > > Junio C Hamano (1):\n> > > >   diff: --{rotate,skip}-to=<path>\n> > >\n> > > That's not part of your series (didn't I ask you not to include it)?\n> > >\n> > > > ZheNing Hu (1):\n> > > >   difftool.c: learn a new way start at specified file\n> > >\n> > > Will see what I find, but I may not be able to get to it today.\n> > >\n> > > Thanks.\n"},{"id":"417160","messageId":"CAOLTT8QSuNs6=Um=E0FOBWM+xXx3sbmCx1j+GkeyomTsuwjjKw@mail.gmail.com","threadId":"55116","inReplyTo":"xmqq8s7nm1dv.fsf@gitster.g","subject":"Re: [PATCH v5 2/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-17T16:18:18Z","receivedAt":"2021-02-17T16:19:33Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月17日周三 下午6:31写道：\n>\n> \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: ZheNing Hu <adlternative@gmail.com>\n> >\n> > `git difftool` only allow us to select file to view in turn.\n> > If there is a commit with many files and we exit in the search,\n>\n> I am not sure what \"in the search\" refers to.  \"in the middle\" I\n> would understand, though.\n>\n> > We will have to traverse list again to get the file diff which\n>\n> Let's downcase this \"We\".\n>\n> > we want to see. Therefore, here is a new method: user can use\n> > `git difftool --rotate-to=<filename>` or `git difftool --skip-to=<filename>`\n> > to start viewing from the specified file, This will improve the\n> > user experience.\n>\n> Do we need both?  I'd rather not to give end-user-facing commands\n> too many knobs that would do similar things.  Too many choices to\n> choose from without clear answer to \"which one should I prefer to\n> use?\" is a bad combination for end-users.\n>\nSo users will not need to use `git difftool --skip-to`? Then I am confused\nabout the meaning of `git difftool --skip-to`.\n> > diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> > index 7c5b3cf42bcc..aa2b5c11f20b 100644\n> > --- a/Documentation/diff-options.txt\n> > +++ b/Documentation/diff-options.txt\n> > @@ -701,7 +701,7 @@ components matches the pattern.  For example, the pattern \"`foo*bar`\"\n> >  matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n> >\n> >  --skip-to=<file>::\n> > ---rotate-to=<file::\n> > +--rotate-to=<file>::\n> >       Discard the files before the named <file> from the output\n> >       (i.e. 'skip to'), or move them to the end of the output\n> >       (i.e. 'rotate to').  These were invented primarily for use\n>\n> Thanks for correcting, but this change should not be a part of this\n> patch.  Instead, you help the other's topic by giving a review (and\n> you could just have said \"there there is closing '>' missing\").\n>\nOkay, I will pay attention later.\n> > diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\n> > index 484c485fd06c..c64dff69c976 100644\n> > --- a/Documentation/git-difftool.txt\n> > +++ b/Documentation/git-difftool.txt\n> > @@ -34,6 +34,16 @@ OPTIONS\n> >       This is the default behaviour; the option is provided to\n> >       override any configuration settings.\n> >\n> > +--rotate-to=<file>::\n> > +     Internally call `git diff --rotate-to=<file>`,\n> > +     show the change in the specified path first.\n> > +     Files before the specified path will be moved to the last output.\n> > +\n> > +--skip-to=<file>::\n> > +     Internally call `git diff --skip-to=<file>`,\n> > +     skip the output to the specified path.\n> > +     Files before the specified path will not output.\n> > +\n>\n> This, unlike the \"diffcore\" stuff, is end-user facing, and it is\n> better not to force the readers even know what --skip-to option\n> to the diff does (after all, difftool users are using 'git difftool'\n> and they are not necessarily 'git diff' users).\n>\n>     --skip-to=<file>::\n>             Start showing the diff for the given path, skipping all\n>             the paths before it.\n>\n> or something, perhaps.\n>\n> > +test_expect_success 'difftool --skip-to' '\n> > +     difftool_test_setup &&\n> > +     test_when_finished git reset --hard &&\n> > +     git difftool --no-prompt --extcmd=cat --skip-to=\"2\" HEAD^ >output &&\n> > +     cat >expect <<-\\EOF &&\n> > +     2\n> > +     4\n> > +     EOF\n> > +     test_cmp output expect &&\n> > +     test_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n> > +'\n>\n> This probably should be split into two independent tests.  One to\n> check that the non-failing case works as expected, the other to\n> check that a bogus command line option errors out as expected.\n>\nI will finish it.\n> Thanks.\n\nBesides, I have some curiosity about one place in the code in your\npatch:\n> int cmd_diff_tree(...)\n> ...\n> if (read_stdin) {\n> ...\n> opt->diffopt.rotate_to_strict = 0;\n> ...\n}\nThis is the only place where rotate_to_strict is set to zero,\nSo the \"git log -p\" you mentioned earlier is here Called this code\nto avoid exiting  the program because of the wrong path, right?\n\nThanks.\n"},{"id":"417172","messageId":"xmqqo8gile02.fsf@gitster.g","threadId":"55116","inReplyTo":"CAOLTT8Ri+XbSg_=KaLOCmNX4Nrii1ssN9_FFbnmm7ew4vYN5nA@mail.gmail.com","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-17T18:56:29Z","receivedAt":"2021-02-17T18:57:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ZheNing Hu <adlternative@gmail.com> writes:\n\n> Denton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n>>\n>> Hi ZheNing,\n>>\n>> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n>> > Oh, I am sorry.\n>> > Then I only need to squash the two commit, right?\n>>\n>> I've never used GGG before but I suspect that in your GitHub PR, you\n>> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n>>\n>> CCing the creator of GGG, please correct me if I'm wrong.\n>>\n>> -Denton\n\n> Hi Denton Liu,\n> You mean I should cherry-pick Junio's patch to my topic branch, right?\n\nThanks, Denton, for helping.\n\nZheNing, the end result we want to see on the list is just a single\npatch, your 2/2 alone, that says \"this patch depends on the\ndiffcore-rotate topic\" _under_ its \"---\" three-dash lines (where\n\"meta\" comments on the patch to explain how it fits the rest of the\nworld, etc.).  As a single patch \"topic\", there won't be even 1/1\nmarking, i.e. something like:\n\n    Subject: [PATCH v6] difftool.c: learn a new way start at specified file\n    From: ZheNing Hu <adlternative@gmail.com>\n\n    `git difftool` only allow us to ...\n    ...\n    Teach the command an option '--skip-to=<path>' to allow the\n    user to say that diffs for earlier paths are not interesting\n    (because they were already seen in an earlier session) and\n    start this session with the named path.\n\n    Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n    ---\n\n     * An earlier round tried to implement the skipping all in the\n       GIT_EXTERNAL_DIFF, but this round takes advantage of the new\n       \"diff --skip-to=<path>\" feature implemented by gitster\n       (therefore, the patch depends on that topic).\n\n     Documentation/git-difftool.txt | 10 ++++++++++\n     t/t7800-difftool.sh            | 30 ++++++++++++++++++++++++++++++\n     2 files changed, 40 insertions(+)\n\n    ... patch here ...\n\n\nI do not know how to achieve that end result with GGG and I do not\nknow if GGG allows its users to do so easily, though.\n\nThanks.\n"},{"id":"417283","messageId":"CAOLTT8SkVQV+KFCAqipWmA35wcieM+Y1u45ONmc1ASchf8ub5w@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqo8gile02.fsf@gitster.g","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-18T05:20:48Z","receivedAt":"2021-02-18T05:21:58Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio, thank you for your patient explanation.\nJunio C Hamano <gitster@pobox.com> 于2021年2月18日周四 上午2:56写道：\n>\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > Denton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n> >>\n> >> Hi ZheNing,\n> >>\n> >> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n> >> > Oh, I am sorry.\n> >> > Then I only need to squash the two commit, right?\n> >>\n> >> I've never used GGG before but I suspect that in your GitHub PR, you\n> >> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n> >>\n> >> CCing the creator of GGG, please correct me if I'm wrong.\n> >>\n> >> -Denton\n>\n> > Hi Denton Liu,\n> > You mean I should cherry-pick Junio's patch to my topic branch, right?\n>\n> Thanks, Denton, for helping.\n>\n> ZheNing, the end result we want to see on the list is just a single\n> patch, your 2/2 alone, that says \"this patch depends on the\n> diffcore-rotate topic\" _under_ its \"---\" three-dash lines (where\n> \"meta\" comments on the patch to explain how it fits the rest of the\n> world, etc.).  As a single patch \"topic\", there won't be even 1/1\n> marking, i.e. something like:\n>\n>     Subject: [PATCH v6] difftool.c: learn a new way start at specified file\n>     From: ZheNing Hu <adlternative@gmail.com>\n>\n>     `git difftool` only allow us to ...\n>     ...\n>     Teach the command an option '--skip-to=<path>' to allow the\n>     user to say that diffs for earlier paths are not interesting\n>     (because they were already seen in an earlier session) and\n>     start this session with the named path.\n>\nI noticed that \"skip-to\" is more suitable for users, right?\n(I always thought \"rotate-to\" would be better)\n>     Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n>     ---\n>\n>      * An earlier round tried to implement the skipping all in the\n>        GIT_EXTERNAL_DIFF, but this round takes advantage of the new\n>        \"diff --skip-to=<path>\" feature implemented by gitster\n>        (therefore, the patch depends on that topic).\n>\n>      Documentation/git-difftool.txt | 10 ++++++++++\n>      t/t7800-difftool.sh            | 30 ++++++++++++++++++++++++++++++\n>      2 files changed, 40 insertions(+)\n>\n>     ... patch here ...\n>\n>\n> I do not know how to achieve that end result with GGG and I do not\n> know if GGG allows its users to do so easily, though.\nI understand what you mean. I think I want GGG to work normally.\n I will try to resubmit your last patch and my new patch cherry-pick to the\n new topic branch. If there are still problems with this, please point out.\n>\n> Thanks.\n"},{"id":"417328","messageId":"CAOLTT8QNbTeSJfo2O7f5vv6Q9ZVMrkGjRCikc4P7eN7M6aeZdw@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqo8gile02.fsf@gitster.g","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-18T15:04:10Z","receivedAt":"2021-02-18T16:55:52Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年2月18日周四 上午2:56写道：\n>\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > Denton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n> >>\n> >> Hi ZheNing,\n> >>\n> >> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n> >> > Oh, I am sorry.\n> >> > Then I only need to squash the two commit, right?\n> >>\n> >> I've never used GGG before but I suspect that in your GitHub PR, you\n> >> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n> >>\n> >> CCing the creator of GGG, please correct me if I'm wrong.\n> >>\n> >> -Denton\n>\n> > Hi Denton Liu,\n> > You mean I should cherry-pick Junio's patch to my topic branch, right?\n>\n> Thanks, Denton, for helping.\n>\n> ZheNing, the end result we want to see on the list is just a single\n> patch, your 2/2 alone, that says \"this patch depends on the\n> diffcore-rotate topic\" _under_ its \"---\" three-dash lines (where\n> \"meta\" comments on the patch to explain how it fits the rest of the\n> world, etc.).  As a single patch \"topic\", there won't be even 1/1\n> marking, i.e. something like:\n>\n>     Subject: [PATCH v6] difftool.c: learn a new way start at specified file\n>     From: ZheNing Hu <adlternative@gmail.com>\n>\n>     `git difftool` only allow us to ...\n>     ...\n>     Teach the command an option '--skip-to=<path>' to allow the\n>     user to say that diffs for earlier paths are not interesting\n>     (because they were already seen in an earlier session) and\n>     start this session with the named path.\n>\n>     Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n>     ---\n>\n>      * An earlier round tried to implement the skipping all in the\n>        GIT_EXTERNAL_DIFF, but this round takes advantage of the new\n>        \"diff --skip-to=<path>\" feature implemented by gitster\n>        (therefore, the patch depends on that topic).\n>\n>      Documentation/git-difftool.txt | 10 ++++++++++\n>      t/t7800-difftool.sh            | 30 ++++++++++++++++++++++++++++++\n>      2 files changed, 40 insertions(+)\n>\n>     ... patch here ...\n>\n>\n> I do not know how to achieve that end result with GGG and I do not\n> know if GGG allows its users to do so easily, though.\n>\nHi, Junio,\nI think my patch is stuck in GGG, and the current version is after I\ncherry-pick your patch and my patch on the master. Because I don’t\nknow how to based on your patch but not submit your patch. Is there\nany good way?\nThanks.\n> Thanks.\n"},{"id":"417340","messageId":"xmqqy2flfay4.fsf@gitster.g","threadId":"55116","inReplyTo":"CAOLTT8QNbTeSJfo2O7f5vv6Q9ZVMrkGjRCikc4P7eN7M6aeZdw@mail.gmail.com","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-18T19:11:15Z","receivedAt":"2021-02-18T19:33:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ZheNing Hu <adlternative@gmail.com> writes:\n\n> I think my patch is stuck in GGG, and the current version is after I\n> cherry-pick your patch and my patch on the master. Because I don’t\n> know how to based on your patch but not submit your patch. Is there\n> any good way?\n\nI am not capable of doing helpdesk for GGG but I think you can send\ntwo of them anyway with the title of the first one munged for\nreading humans to tell them not to use or even look at it ;-)\n\nGGG may send both of them out, but the ultimate objective here in\nthe exercise is to avoid the unwanted first one to waste people's\ntime, so until such a feature is implemented in GGG (or there may\nalready be one, but we do not know it), that would serve as a\nworkaround.\n\nThanks.\n"},{"id":"417393","messageId":"CAOLTT8QUsrKPnCkbbz_4G6Z-4PRfK8H_pz_4YMzXKdACNNxT2A@mail.gmail.com","threadId":"55116","inReplyTo":"xmqqy2flfay4.fsf@gitster.g","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-19T10:59:55Z","receivedAt":"2021-02-19T11:02:11Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Ok,hope these problems won't make you unpleasant.\n I will try to solve these small problems.\n\nJunio C Hamano <gitster@pobox.com> 于2021年2月19日周五 上午3:11写道：\n>\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > I think my patch is stuck in GGG, and the current version is after I\n> > cherry-pick your patch and my patch on the master. Because I don’t\n> > know how to based on your patch but not submit your patch. Is there\n> > any good way?\n>\n> I am not capable of doing helpdesk for GGG but I think you can send\n> two of them anyway with the title of the first one munged for\n> reading humans to tell them not to use or even look at it ;-)\n>\n> GGG may send both of them out, but the ultimate objective here in\n> the exercise is to avoid the unwanted first one to waste people's\n> time, so until such a feature is implemented in GGG (or there may\n> already be one, but we do not know it), that would serve as a\n> workaround.\n>\n> Thanks.\n"},{"id":"417403","messageId":"pull.870.v6.git.1613739235241.gitgitgadget@gmail.com","threadId":"55116","inReplyTo":"pull.870.v5.git.1613480198.gitgitgadget@gmail.com","subject":"[PATCH v6] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-19T12:53:54Z","receivedAt":"2021-02-19T12:55:10Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\n`git difftool` only allow us to select file to view in turn.\nIf there is a commit with many files and we exit in the middle,\nwe will have to traverse list again to get the file diff which\nwe want to see. Therefore,teach the command an option\n`--skip-to=<path>` to allow the user to say that diffs for earlier\npaths are not interesting (because they were already seen in an\nearlier session) and start this session with the named path.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n    difftool.c: learn a new way start at specified file\n    \n     * The patch of the previous version implemented the jump through\n       environment variables. The current version is based on the \"diff\n       --skip-to=\" feature implemented by gitster, which implements a\n       possible solution for the jump of difftool.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v6\nPull-Request: https://github.com/gitgitgadget/git/pull/870\n\nRange-diff vs v5:\n\n 1:  fb4bfd0f8b16 < -:  ------------ diff: --{rotate,skip}-to=<path>\n 2:  98e2707ee2fa ! 1:  4377a917ca9e difftool.c: learn a new way start at specified file\n     @@ Commit message\n          difftool.c: learn a new way start at specified file\n      \n          `git difftool` only allow us to select file to view in turn.\n     -    If there is a commit with many files and we exit in the search,\n     -    We will have to traverse list again to get the file diff which\n     -    we want to see. Therefore, here is a new method: user can use\n     -    `git difftool --rotate-to=<filename>` or `git difftool --skip-to=<filename>`\n     -    to start viewing from the specified file, This will improve the\n     -    user experience.\n     -\n     -    `git difftool --rotate-to=<file>` or `git difftool --skip-to=<filename>`\n     -    will pass the path to `diffcore-rotate`, and diff-core will\n     -    adjust the order of files, make the specified file sorted to\n     -    the first.`git difftool --rotate-to=<file>` will move files before\n     -    the  specified path to the last output, and\n     -    `git difftool --skip-to=<filename>`  will ignore these files output.\n     -    It is an error when there is no patch for specified file is shown.\n     +    If there is a commit with many files and we exit in the middle,\n     +    we will have to traverse list again to get the file diff which\n     +    we want to see. Therefore,teach the command an option\n     +    `--skip-to=<path>` to allow the user to say that diffs for earlier\n     +    paths are not interesting (because they were already seen in an\n     +    earlier session) and start this session with the named path.\n      \n          Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n      \n     - ## Documentation/diff-options.txt ##\n     -@@ Documentation/diff-options.txt: components matches the pattern.  For example, the pattern \"`foo*bar`\"\n     - matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n     - \n     - --skip-to=<file>::\n     ----rotate-to=<file::\n     -+--rotate-to=<file>::\n     - \tDiscard the files before the named <file> from the output\n     - \t(i.e. 'skip to'), or move them to the end of the output\n     - \t(i.e. 'rotate to').  These were invented primarily for use\n     -\n       ## Documentation/git-difftool.txt ##\n      @@ Documentation/git-difftool.txt: OPTIONS\n       \tThis is the default behaviour; the option is provided to\n       \toverride any configuration settings.\n       \n      +--rotate-to=<file>::\n     -+\tInternally call `git diff --rotate-to=<file>`,\n     -+\tshow the change in the specified path first.\n     -+\tFiles before the specified path will be moved to the last output.\n     ++\tStart showing the diff for the given path,\n     ++\tthe paths before it will move to end and output.\n      +\n      +--skip-to=<file>::\n     -+\tInternally call `git diff --skip-to=<file>`,\n     -+\tskip the output to the specified path.\n     -+\tFiles before the specified path will not output.\n     ++\tStart showing the diff for the given path, skipping all\n     ++\tthe paths before it.\n      +\n       -t <tool>::\n       --tool=<tool>::\n     @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd ar\n      +\t4\n      +\t1\n      +\tEOF\n     -+\ttest_cmp output expect &&\n     -+\ttest_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^\n     ++\ttest_cmp output expect\n      +'\n      +\n      +test_expect_success 'difftool --skip-to' '\n     @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd ar\n      +\t2\n      +\t4\n      +\tEOF\n     -+\ttest_cmp output expect &&\n     -+\ttest_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n     ++\ttest_cmp output expect\n      +'\n      +\n     ++test_expect_success 'difftool --rotate/skip-to error condition' '\n     ++\ttest_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^ &&\n     ++\ttest_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n     ++'\n       test_done\n\n\n Documentation/git-difftool.txt |  8 ++++++++\n t/t7800-difftool.sh            | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex 484c485fd06c..143b0c49d739 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -34,6 +34,14 @@ OPTIONS\n \tThis is the default behaviour; the option is provided to\n \toverride any configuration settings.\n \n+--rotate-to=<file>::\n+\tStart showing the diff for the given path,\n+\tthe paths before it will move to end and output.\n+\n+--skip-to=<file>::\n+\tStart showing the diff for the given path, skipping all\n+\tthe paths before it.\n+\n -t <tool>::\n --tool=<tool>::\n \tUse the diff tool specified by <tool>.  Valid values include\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 9192c141ffc6..3e041e83aede 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -762,4 +762,36 @@ test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n \ttest_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n '\n \n+test_expect_success 'difftool --rotate-to' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\techo 1 >1 &&\n+\techo 2 >2 &&\n+\techo 4 >4 &&\n+\tgit add 1 2 4 &&\n+\tgit commit -a -m \"124\" &&\n+\tgit difftool --no-prompt --extcmd=cat --rotate-to=\"2\" HEAD^ >output&&\n+\tcat >expect <<-\\EOF &&\n+\t2\n+\t4\n+\t1\n+\tEOF\n+\ttest_cmp output expect\n+'\n+\n+test_expect_success 'difftool --skip-to' '\n+\tdifftool_test_setup &&\n+\ttest_when_finished git reset --hard &&\n+\tgit difftool --no-prompt --extcmd=cat --skip-to=\"2\" HEAD^ >output &&\n+\tcat >expect <<-\\EOF &&\n+\t2\n+\t4\n+\tEOF\n+\ttest_cmp output expect\n+'\n+\n+test_expect_success 'difftool --rotate/skip-to error condition' '\n+\ttest_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^ &&\n+\ttest_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n+'\n test_done\n\nbase-commit: 1eb4136ac2a24764257567b930535fcece01719f\n-- \ngitgitgadget\n"},{"id":"417470","messageId":"CAOLTT8ST9_eazfsy+=j-t3bHuwhnsBhzXiQ4peC3RbLEK1CTxg@mail.gmail.com","threadId":"55116","inReplyTo":"pull.870.v6.git.1613739235241.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] difftool.c: learn a new way start at specified file","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-02-22T15:11:40Z","receivedAt":"2021-02-22T15:12:50Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi,Junio,\n\nZheNing Hu via GitGitGadget <gitgitgadget@gmail.com> 于2021年2月19日周五 下午8:53写道：\n>\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> `git difftool` only allow us to select file to view in turn.\n> If there is a commit with many files and we exit in the middle,\n> we will have to traverse list again to get the file diff which\n> we want to see. Therefore,teach the command an option\n> `--skip-to=<path>` to allow the user to say that diffs for earlier\n> paths are not interesting (because they were already seen in an\n> earlier session) and start this session with the named path.\n>\n> Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> ---\n>     difftool.c: learn a new way start at specified file\n>\n>      * The patch of the previous version implemented the jump through\n>        environment variables. The current version is based on the \"diff\n>        --skip-to=\" feature implemented by gitster, which implements a\n>        possible solution for the jump of difftool.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v6\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v6\n> Pull-Request: https://github.com/gitgitgadget/git/pull/870\n>\n> Range-diff vs v5:\n>\n>  1:  fb4bfd0f8b16 < -:  ------------ diff: --{rotate,skip}-to=<path>\n>  2:  98e2707ee2fa ! 1:  4377a917ca9e difftool.c: learn a new way start at specified file\n>      @@ Commit message\n>           difftool.c: learn a new way start at specified file\n>\n>           `git difftool` only allow us to select file to view in turn.\n>      -    If there is a commit with many files and we exit in the search,\n>      -    We will have to traverse list again to get the file diff which\n>      -    we want to see. Therefore, here is a new method: user can use\n>      -    `git difftool --rotate-to=<filename>` or `git difftool --skip-to=<filename>`\n>      -    to start viewing from the specified file, This will improve the\n>      -    user experience.\n>      -\n>      -    `git difftool --rotate-to=<file>` or `git difftool --skip-to=<filename>`\n>      -    will pass the path to `diffcore-rotate`, and diff-core will\n>      -    adjust the order of files, make the specified file sorted to\n>      -    the first.`git difftool --rotate-to=<file>` will move files before\n>      -    the  specified path to the last output, and\n>      -    `git difftool --skip-to=<filename>`  will ignore these files output.\n>      -    It is an error when there is no patch for specified file is shown.\n>      +    If there is a commit with many files and we exit in the middle,\n>      +    we will have to traverse list again to get the file diff which\n>      +    we want to see. Therefore,teach the command an option\n>      +    `--skip-to=<path>` to allow the user to say that diffs for earlier\n>      +    paths are not interesting (because they were already seen in an\n>      +    earlier session) and start this session with the named path.\n>\n>           Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n>\n>      - ## Documentation/diff-options.txt ##\n>      -@@ Documentation/diff-options.txt: components matches the pattern.  For example, the pattern \"`foo*bar`\"\n>      - matches \"`fooasdfbar`\" and \"`foo/bar/baz/asdf`\" but not \"`foobarx`\".\n>      -\n>      - --skip-to=<file>::\n>      ----rotate-to=<file::\n>      -+--rotate-to=<file>::\n>      -  Discard the files before the named <file> from the output\n>      -  (i.e. 'skip to'), or move them to the end of the output\n>      -  (i.e. 'rotate to').  These were invented primarily for use\n>      -\n>        ## Documentation/git-difftool.txt ##\n>       @@ Documentation/git-difftool.txt: OPTIONS\n>         This is the default behaviour; the option is provided to\n>         override any configuration settings.\n>\n>       +--rotate-to=<file>::\n>      -+ Internally call `git diff --rotate-to=<file>`,\n>      -+ show the change in the specified path first.\n>      -+ Files before the specified path will be moved to the last output.\n>      ++ Start showing the diff for the given path,\n>      ++ the paths before it will move to end and output.\n>       +\n>       +--skip-to=<file>::\n>      -+ Internally call `git diff --skip-to=<file>`,\n>      -+ skip the output to the specified path.\n>      -+ Files before the specified path will not output.\n>      ++ Start showing the diff for the given path, skipping all\n>      ++ the paths before it.\n>       +\n>        -t <tool>::\n>        --tool=<tool>::\n>      @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd ar\n>       + 4\n>       + 1\n>       + EOF\n>      -+ test_cmp output expect &&\n>      -+ test_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^\n>      ++ test_cmp output expect\n>       +'\n>       +\n>       +test_expect_success 'difftool --skip-to' '\n>      @@ t/t7800-difftool.sh: test_expect_success 'difftool --gui, --tool and --extcmd ar\n>       + 2\n>       + 4\n>       + EOF\n>      -+ test_cmp output expect &&\n>      -+ test_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n>      ++ test_cmp output expect\n>       +'\n>       +\n>      ++test_expect_success 'difftool --rotate/skip-to error condition' '\n>      ++ test_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^ &&\n>      ++ test_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n>      ++'\n>        test_done\n>\n>\n>  Documentation/git-difftool.txt |  8 ++++++++\n>  t/t7800-difftool.sh            | 32 ++++++++++++++++++++++++++++++++\n>  2 files changed, 40 insertions(+)\n>\n> diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\n> index 484c485fd06c..143b0c49d739 100644\n> --- a/Documentation/git-difftool.txt\n> +++ b/Documentation/git-difftool.txt\n> @@ -34,6 +34,14 @@ OPTIONS\n>         This is the default behaviour; the option is provided to\n>         override any configuration settings.\n>\n> +--rotate-to=<file>::\n> +       Start showing the diff for the given path,\n> +       the paths before it will move to end and output.\n> +\n> +--skip-to=<file>::\n> +       Start showing the diff for the given path, skipping all\n> +       the paths before it.\n> +\n>  -t <tool>::\n>  --tool=<tool>::\n>         Use the diff tool specified by <tool>.  Valid values include\n> diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\n> index 9192c141ffc6..3e041e83aede 100755\n> --- a/t/t7800-difftool.sh\n> +++ b/t/t7800-difftool.sh\n> @@ -762,4 +762,36 @@ test_expect_success 'difftool --gui, --tool and --extcmd are mutually exclusive'\n>         test_must_fail git difftool --gui --tool=test-tool --extcmd=cat\n>  '\n>\n> +test_expect_success 'difftool --rotate-to' '\n> +       difftool_test_setup &&\n> +       test_when_finished git reset --hard &&\n> +       echo 1 >1 &&\n> +       echo 2 >2 &&\n> +       echo 4 >4 &&\n> +       git add 1 2 4 &&\n> +       git commit -a -m \"124\" &&\n> +       git difftool --no-prompt --extcmd=cat --rotate-to=\"2\" HEAD^ >output&&\n> +       cat >expect <<-\\EOF &&\n> +       2\n> +       4\n> +       1\n> +       EOF\n> +       test_cmp output expect\n> +'\n> +\n> +test_expect_success 'difftool --skip-to' '\n> +       difftool_test_setup &&\n> +       test_when_finished git reset --hard &&\n> +       git difftool --no-prompt --extcmd=cat --skip-to=\"2\" HEAD^ >output &&\n> +       cat >expect <<-\\EOF &&\n> +       2\n> +       4\n> +       EOF\n> +       test_cmp output expect\n> +'\n> +\n> +test_expect_success 'difftool --rotate/skip-to error condition' '\n> +       test_must_fail git difftool --no-prompt --extcmd=cat --rotate-to=\"3\" HEAD^ &&\n> +       test_must_fail git difftool --no-prompt --extcmd=cat --skip-to=\"3\" HEAD^\n> +'\n>  test_done\n>\n> base-commit: 1eb4136ac2a24764257567b930535fcece01719f\n> --\n> gitgitgadget\n\nA few days ago I successfully sent this patch with the help\nof Denton Liu, I don't know if you see it?\nAny reply is appreciated.\n\n--\nZheNing Hu\n"},{"id":"417497","messageId":"xmqq4ki37pds.fsf@gitster.g","threadId":"55116","inReplyTo":"pull.870.v6.git.1613739235241.gitgitgadget@gmail.com","subject":"Re: [PATCH v6] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-22T21:40:15Z","receivedAt":"2021-02-22T21:41:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: ZheNing Hu <adlternative@gmail.com>\n>\n> `git difftool` only allow us to select file to view in turn.\n> If there is a commit with many files and we exit in the middle,\n> we will have to traverse list again to get the file diff which\n> we want to see. Therefore,teach the command an option\n> `--skip-to=<path>` to allow the user to say that diffs for earlier\n> paths are not interesting (because they were already seen in an\n> earlier session) and start this session with the named path.\n>\n> Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n> ---\n>     difftool.c: learn a new way start at specified file\n>     \n>      * The patch of the previous version implemented the jump through\n>        environment variables. The current version is based on the \"diff\n>        --skip-to=\" feature implemented by gitster, which implements a\n>        possible solution for the jump of difftool.\n\nSo there was absolutely no need to change anything in difftool,\nbecause it just passes down anything that it does not understand\ndown to the underlying \"git diff\"?\n\nVery interesting.\n\nThanks, will queue.  Let's see if we get comments from others.\n"},{"id":"417820","messageId":"nycvar.QRO.7.76.6.2102251206080.57@tvgsbejvaqbjf.bet","threadId":"55116","inReplyTo":"xmqqo8gile02.fsf@gitster.g","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-25T11:08:36Z","receivedAt":"2021-02-25T15:13:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio, ZheNing & Denton,\n\nOn Wed, 17 Feb 2021, Junio C Hamano wrote:\n\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > Denton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n> >>\n> >> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n> >> > Oh, I am sorry.\n> >> > Then I only need to squash the two commit, right?\n> >>\n> >> I've never used GGG before but I suspect that in your GitHub PR, you\n> >> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n\nYes, that is my understanding of what needed to be done.\n\n> > You mean I should cherry-pick Junio's patch to my topic branch, right?\n\nThat, too.\n\n> ZheNing, the end result we want to see on the list is just a single\n> patch, your 2/2 alone, that says \"this patch depends on the\n> diffcore-rotate topic\" _under_ its \"---\" three-dash lines (where\n> \"meta\" comments on the patch to explain how it fits the rest of the\n> world, etc.).  As a single patch \"topic\", there won't be even 1/1\n> marking, i.e. something like:\n>\n>     Subject: [PATCH v6] difftool.c: learn a new way start at specified file\n>     From: ZheNing Hu <adlternative@gmail.com>\n>\n>     `git difftool` only allow us to ...\n>     ...\n>     Teach the command an option '--skip-to=<path>' to allow the\n>     user to say that diffs for earlier paths are not interesting\n>     (because they were already seen in an earlier session) and\n>     start this session with the named path.\n>\n>     Signed-off-by: ZheNing Hu <adlternative@gmail.com>\n>     ---\n>\n>      * An earlier round tried to implement the skipping all in the\n>        GIT_EXTERNAL_DIFF, but this round takes advantage of the new\n>        \"diff --skip-to=<path>\" feature implemented by gitster\n>        (therefore, the patch depends on that topic).\n>\n>      Documentation/git-difftool.txt | 10 ++++++++++\n>      t/t7800-difftool.sh            | 30 ++++++++++++++++++++++++++++++\n>      2 files changed, 40 insertions(+)\n>\n>     ... patch here ...\n>\n>\n> I do not know how to achieve that end result with GGG and I do not\n> know if GGG allows its users to do so easily, though.\n\nFor single-patch contributions, the PR description is not turned into a\nseparate cover letter (per your request, Junio), but it is put between the\ncommit message and the diff as you illustrated.\n\nSo yes, the comment can go into the PR description (AKA the first comment\non the PR) and the next `/submit` will include it in the single mail.\n\nCiao,\nDscho\n"},{"id":"417835","messageId":"xmqqpn0oug2v.fsf@gitster.g","threadId":"55116","inReplyTo":"nycvar.QRO.7.76.6.2102251206080.57@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v5 0/2] difftool.c: learn a new way start at specified file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T19:01:44Z","receivedAt":"2021-02-25T19:02:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Junio, ZheNing & Denton,\n>\n> On Wed, 17 Feb 2021, Junio C Hamano wrote:\n>\n>> ZheNing Hu <adlternative@gmail.com> writes:\n>>\n>> > Denton Liu <liu.denton@gmail.com> 于2021年2月17日周三 下午7:14写道：\n>> >>\n>> >> On Wed, Feb 17, 2021 at 12:12:10PM +0800, ZheNing Hu wrote:\n>> >> > Oh, I am sorry.\n>> >> > Then I only need to squash the two commit, right?\n>> >>\n>> >> I've never used GGG before but I suspect that in your GitHub PR, you\n>> >> need to set the PR base to 'master' instead of 'jc/diffcore-rotate'.\n>\n> Yes, that is my understanding of what needed to be done.\n\nThanks.\n\n>> > You mean I should cherry-pick Junio's patch to my topic branch, right?\n>\n> That, too.\n\nNot quite.  The 'jc/diffcore-rotate' topic would be the 'upstream'\nbranch of their topic, so patches in jc/diffcore-rotate won't need\nto and should not be cherry-picked, I think.\n\n>> ZheNing, the end result we want to see on the list is just a single\n>> patch, your 2/2 alone, that says \"this patch depends on the\n>> diffcore-rotate topic\" _under_ its \"---\" three-dash lines (where\n>> \"meta\" comments on the patch to explain how it fits the rest of the\n>> world, etc.).  As a single patch \"topic\", there won't be even 1/1\n>> marking, i.e. something like:\n>> ...\n>> I do not know how to achieve that end result with GGG and I do not\n>> know if GGG allows its users to do so easily, though.\n>\n> For single-patch contributions, the PR description is not turned into a\n> separate cover letter (per your request, Junio), but it is put between the\n> commit message and the diff as you illustrated.\n>\n> So yes, the comment can go into the PR description (AKA the first comment\n> on the PR) and the next `/submit` will include it in the single mail.\n\nGood.  FWIW, the part I said \"I do not now how\" was not about making\nthe single-patch topic look the way we want, but about making the\nwork a single-patch topic to begin with (which was answered by the\nabove \"set the PR base\" suggestion).\n"}]}