diff mbox series

[v2] git-difftool-helper.sh: learn a new way go back to last save point

Message ID pull.870.v2.git.1612803744188.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series [v2] git-difftool-helper.sh: learn a new way go back to last save point | expand

Commit Message

ZheNing Hu Feb. 8, 2021, 5:02 p.m. UTC
From: ZheNing Hu <adlternative@gmail.com>

`git difftool` only allow us to select file to view in turn.
If there is a commit with many files and we exit in the search,
We will have to traverse list again to get the file diff which
we want to see. Therefore, here is a new method: every time before
we view the file diff, the current coordinates will be stored in
`GIT_DIR/difftool-last-position`, this file will be deleted after
successful traversing. But if an unexpected exit occurred midway or
users similar to using "ctrl+c" kill the process,and the user wants
to redo the same `git difftoool`, git will view the coordinates in
the save point, ask user if they want continue from the last position.
This will improve the user experience.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
    git-difftool-helper.sh: learn a new way skip to save point
    
    git user may should travel the diff list to choice file diff to view, if
    they exit in midway,they must travel it again. By saving current
    coordinates in GIT_DIR/difftool-last-position method, provides a
    possibility for this user-friendly solution.
    
    this patch's origin discuss is here:
    https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/
    
    Thanks!

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/870

Range-diff vs v1:

 1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point
     @@ Metadata
      Author: ZheNing Hu <adlternative@gmail.com>
      
       ## Commit message ##
     -    git-difftool-helper.sh: learn a new way skip to save point
     +    git-difftool-helper.sh: learn a new way go back to last save point
      
     -    `git difftool` only allow us to select file to view In turn.
     -    If there is a commit with many files and we exit in search,
     +    `git difftool` only allow us to select file to view in turn.
     +    If there is a commit with many files and we exit in the search,
          We will have to traverse list again to get the file diff which
     -    we want to see.Therefore,here is a new method:every time before
     -    we view the file diff,the current coordinates will be stored in
     -    `GIT_DIR/difftool_skip_to`,this file will be deleted after
     -    successful traversing.But if an unexpected exit occurred midway,
     -    git will view the coordinates in the save point,ask user if they
     -    want continue from the last saved point.This will improve the
     -    user experience.
     +    we want to see. Therefore, here is a new method: every time before
     +    we view the file diff, the current coordinates will be stored in
     +    `GIT_DIR/difftool-last-position`, this file will be deleted after
     +    successful traversing. But if an unexpected exit occurred midway or
     +    users similar to using "ctrl+c" kill the process,and the user wants
     +    to redo the same `git difftoool`, git will view the coordinates in
     +    the save point, ask user if they want continue from the last position.
     +    This will improve the user experience.
      
          Signed-off-by: ZheNing Hu <adlternative@gmail.com>
      
     @@ git-difftool--helper.sh
       # Copyright (c) 2009, 2010 David Aguilar
       
       TOOL_MODE=diff
     -+GIT_DIFFTOOL_SKIP_TO_FILE="$GIT_DIR/difftool-skip-to"
     ++GIT_DIFFTOOL_LAST_POSITION="$GIT_DIR/difftool-last-position"
     ++DIFFTOOL_FIRST_NUM="1"
       . git-mergetool--lib
       
       # difftool.prompt controls the default prompt/no-prompt behavior
     @@ git-difftool--helper.sh: launch_merge_tool () {
       	# the user with the real $MERGED name before launching $merge_tool.
       	if should_prompt
       	then
     -+		if test -f "$GIT_DIFFTOOL_SKIP_TO_FILE"
     ++		if test -f "$GIT_DIFFTOOL_LAST_POSITION"
      +		then
     -+			SAVE_POINT_NUM=$(cat "$GIT_DIFFTOOL_SKIP_TO_FILE")
     -+			if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&
     -+				test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER
     ++			if SAVE_POINT_NUM=$(cat 2>/dev/null "$GIT_DIFFTOOL_LAST_POSITION") &&
     ++				test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
     ++					test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
      +			then
     -+				# choice skip or not skip when check first file.
     -+				if test $GIT_DIFF_PATH_COUNTER -eq "1"
     ++				if test "$GIT_DIFF_PATH_COUNTER" -eq "$DIFFTOOL_FIRST_NUM"
      +				then
     -+					printf "do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?"
     ++					printf "Do you want to start from the possible last file you were viewing? [Y/n]?"
      +					read skip_ans || return
      +					if test "$skip_ans" = y
      +					then
     @@ git-difftool--helper.sh: launch_merge_tool () {
      +				fi
      +			fi
      +		fi
     -+		# write the current coordinates to .git/difftool-skip-to
     -+		if test !$SAVE_POINT_NUM || $SAVE_POINT_NUM -ne $GIT_DIFF_PATH_COUNTER
     ++		if test -z "$SAVE_POINT_NUM" ||
     ++			test "$SAVE_POINT_NUM" -ne "$GIT_DIFF_PATH_COUNTER"
      +		then
     -+			echo $GIT_DIFF_PATH_COUNTER > $GIT_DIFFTOOL_SKIP_TO_FILE
     ++			echo "$GIT_DIFF_PATH_COUNTER" >"$GIT_DIFFTOOL_LAST_POSITION"
      +		fi
       		printf "\nViewing (%s/%s): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \
       			"$GIT_DIFF_PATH_TOTAL" "$MERGED"
     @@ git-difftool--helper.sh: else
       	done
       fi
       
     -+if test -f $GIT_DIFFTOOL_SKIP_TO_FILE &&
     -+	test $GIT_DIFF_PATH_COUNTER -eq $GIT_DIFF_PATH_TOTAL
     ++if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL"
      +then
     -+	rm $GIT_DIFFTOOL_SKIP_TO_FILE
     ++	rm -f "$GIT_DIFFTOOL_LAST_POSITION"
      +
      +fi
       exit 0


 git-difftool--helper.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)


base-commit: e6362826a0409539642a5738db61827e5978e2e4

Comments

Junio C Hamano Feb. 8, 2021, 8:13 p.m. UTC | #1
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
> index 46af3e60b718..a01aa7c9d551 100755
> --- a/git-difftool--helper.sh
> +++ b/git-difftool--helper.sh
> @@ -6,6 +6,8 @@
>  # Copyright (c) 2009, 2010 David Aguilar
>  
>  TOOL_MODE=diff
> +GIT_DIFFTOOL_LAST_POSITION="$GIT_DIR/difftool-last-position"
> +DIFFTOOL_FIRST_NUM="1"

Do we need this constant?  I do not think it makes the resulting
code easier to follow.

>  . git-mergetool--lib
>  
>  # difftool.prompt controls the default prompt/no-prompt behavior
> @@ -40,6 +42,30 @@ launch_merge_tool () {
>  	# the user with the real $MERGED name before launching $merge_tool.
>  	if should_prompt
>  	then
> +		if test -f "$GIT_DIFFTOOL_LAST_POSITION"
> +		then
> +			if SAVE_POINT_NUM=$(cat 2>/dev/null "$GIT_DIFFTOOL_LAST_POSITION") &&
> +				test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
> +					test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
> +			then

No need to push the subsequent lines that far to the right,
especially when your variable names are already overly long.  It
just makes things harder to read.

			if test -r "$GIT_DIFFTOOL_LAST_POSITION" &&
			   SAVE_POINT_NUM=$(cat "$GIT_DIFFTOOL_LAST_POSITION") &&
			   test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
			   test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
			then

> +				if test "$GIT_DIFF_PATH_COUNTER" -eq "$DIFFTOOL_FIRST_NUM"
> +				then
> +					printf "Do you want to start from the possible last file you were viewing? [Y/n]?"

Where does that "possible" come from?  If the reason is "We might
have miscomputed or misrecorded an incorrect last position", we
probably should work harder to make sure we don't ;-)

At this point in the code, do we have the _name_ of the file we are
going to skip to readily available, or do we actually need to seek
to that position before we can find it out?

	You were looking at 'hello-world.txt' the last time.
	Do you want to restart from there [Y/n]?

would be far easier to answer for an end-user than an unspecified
"possible last file".

> +		fi
> +		if test -z "$SAVE_POINT_NUM" ||
> +			test "$SAVE_POINT_NUM" -ne "$GIT_DIFF_PATH_COUNTER"

Ditto about indentation.

Is this behaviour something we can write a test for in
t/t7800-difftool.sh, by the way?

Other than these, i.e. (cosmetic) indentation and overly long lines
(ui) giving filename is easier to work with for the users and (dev)
lack of tests, looking quite good.

Thanks.
David Aguilar Feb. 8, 2021, 10:15 p.m. UTC | #2
(cc'd Ryan since the thread involving him was mentioned in the commit message)

On Mon, Feb 8, 2021 at 9:02 AM ZheNing Hu via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: ZheNing Hu <adlternative@gmail.com>
>
> `git difftool` only allow us to select file to view in turn.
> If there is a commit with many files and we exit in the search,
> We will have to traverse list again to get the file diff which
> we want to see. Therefore, here is a new method: every time before
> we view the file diff, the current coordinates will be stored in
> `GIT_DIR/difftool-last-position`, this file will be deleted after
> successful traversing. But if an unexpected exit occurred midway or
> users similar to using "ctrl+c" kill the process,and the user wants
> to redo the same `git difftoool`, git will view the coordinates in
> the save point, ask user if they want continue from the last position.
> This will improve the user experience.
>
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> ---
>     git-difftool-helper.sh: learn a new way skip to save point
>
>     git user may should travel the diff list to choice file diff to view, if
>     they exit in midway,they must travel it again. By saving current
>     coordinates in GIT_DIR/difftool-last-position method, provides a
>     possibility for this user-friendly solution.
>
>     this patch's origin discuss is here:
>     https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/
>
>     Thanks!
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/870
>
> Range-diff vs v1:
>
>  1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point
>      @@ Metadata
>       Author: ZheNing Hu <adlternative@gmail.com>
>
>        ## Commit message ##
>      -    git-difftool-helper.sh: learn a new way skip to save point
>      +    git-difftool-helper.sh: learn a new way go back to last save point
>
>      -    `git difftool` only allow us to select file to view In turn.
>      -    If there is a commit with many files and we exit in search,
>      +    `git difftool` only allow us to select file to view in turn.
>      +    If there is a commit with many files and we exit in the search,
>           We will have to traverse list again to get the file diff which
>      -    we want to see.Therefore,here is a new method:every time before
>      -    we view the file diff,the current coordinates will be stored in
>      -    `GIT_DIR/difftool_skip_to`,this file will be deleted after
>      -    successful traversing.But if an unexpected exit occurred midway,
>      -    git will view the coordinates in the save point,ask user if they
>      -    want continue from the last saved point.This will improve the
>      -    user experience.
>      +    we want to see. Therefore, here is a new method: every time before
>      +    we view the file diff, the current coordinates will be stored in
>      +    `GIT_DIR/difftool-last-position`, this file will be deleted after
>      +    successful traversing. But if an unexpected exit occurred midway or
>      +    users similar to using "ctrl+c" kill the process,and the user wants
>      +    to redo the same `git difftoool`, git will view the coordinates in
>      +    the save point, ask user if they want continue from the last position.
>      +    This will improve the user experience.
>
>           Signed-off-by: ZheNing Hu <adlternative@gmail.com>
>
>      @@ git-difftool--helper.sh
>        # Copyright (c) 2009, 2010 David Aguilar
>
>        TOOL_MODE=diff
>      -+GIT_DIFFTOOL_SKIP_TO_FILE="$GIT_DIR/difftool-skip-to"
>      ++GIT_DIFFTOOL_LAST_POSITION="$GIT_DIR/difftool-last-position"
>      ++DIFFTOOL_FIRST_NUM="1"
>        . git-mergetool--lib
>
>        # difftool.prompt controls the default prompt/no-prompt behavior
>      @@ git-difftool--helper.sh: launch_merge_tool () {
>         # the user with the real $MERGED name before launching $merge_tool.
>         if should_prompt
>         then
>      -+         if test -f "$GIT_DIFFTOOL_SKIP_TO_FILE"
>      ++         if test -f "$GIT_DIFFTOOL_LAST_POSITION"
>       +         then
>      -+                 SAVE_POINT_NUM=$(cat "$GIT_DIFFTOOL_SKIP_TO_FILE")
>      -+                 if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&
>      -+                         test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER
>      ++                 if SAVE_POINT_NUM=$(cat 2>/dev/null "$GIT_DIFFTOOL_LAST_POSITION") &&
>      ++                         test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
>      ++                                 test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
>       +                 then
>      -+                         # choice skip or not skip when check first file.
>      -+                         if test $GIT_DIFF_PATH_COUNTER -eq "1"
>      ++                         if test "$GIT_DIFF_PATH_COUNTER" -eq "$DIFFTOOL_FIRST_NUM"
>       +                         then
>      -+                                 printf "do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?"
>      ++                                 printf "Do you want to start from the possible last file you were viewing? [Y/n]?"
>       +                                 read skip_ans || return
>       +                                 if test "$skip_ans" = y
>       +                                 then
>      @@ git-difftool--helper.sh: launch_merge_tool () {
>       +                         fi
>       +                 fi
>       +         fi


Similar to Junio's question about, "where does this possible come
from?", I wasn't able to make out the behavior in the following
situation.

What about when the user switches branches or specifies a pathspec on
the command-line or some other avenue that ends up with the number of
files to diff being very different than the last difftool invocation?

Will difftool, for example, skip over a smaller set of files on
invocation 2 if invocation 1 involved many files and we exited out
with a counter number that is very high?

One thing that's not too good about having state files in .git/ is
that they're global data and we also have to think about, "what if the
user has multiple difftools running?" and those kind of complexities.

I don't want this to seem like I'm trying to be dismissive of this
feature which does seem like a useful thing in general, so I'll try to
come up with an alternative interface that is slightly more general
but a admittedly a little bit more cumbersome because it's not as
automatic.

What if instead of global state, maybe the user could specify a path
that difftool could skip forward to?   For example, we could teach
difftool to resume by telling it where we last left off:

   git difftool --resume-from=foo/bar099.txt

Then we don't need the global counter state file?


Finally, I'm going to plug what I believe to be the right tool for the
job here.  Have you tried git cola?[1]  Difftool is tightly
integrated, and the UI is such that you can trivially choose any of
the modified/staged files and difftool them by using the Ctrl-d
hotkey.

https://github.com/git-cola/git-cola/

Cola is purpose-built for driving difftool, and for interactive
staging, so not mentioning it in the context of wanting a better UI
for difftool would be a disservice to difftool users.
Junio C Hamano Feb. 8, 2021, 11:34 p.m. UTC | #3
David Aguilar <davvid@gmail.com> writes:

> What if instead of global state, maybe the user could specify a path
> that difftool could skip forward to?   For example, we could teach
> difftool to resume by telling it where we last left off:
>
>    git difftool --resume-from=foo/bar099.txt
>
> Then we don't need the global counter state file?

Does it have to be the second and subsequent invocation to pass the
new "resume-from" option?  As we do not have "global" state, I would
presume that we do not even know if it is the first invocation, so
perhaps a better name would be "--start-from=$pathname"?

> Finally, I'm going to plug what I believe to be the right tool for the
> job here.  Have you tried git cola?[1]  Difftool is tightly
> integrated, and the UI is such that you can trivially choose any of
> the modified/staged files and difftool them by using the Ctrl-d
> hotkey.
>
> https://github.com/git-cola/git-cola/
>
> Cola is purpose-built for driving difftool, and for interactive
> staging, so not mentioning it in the context of wanting a better UI
> for difftool would be a disservice to difftool users.

;-)
ZheNing Hu Feb. 9, 2021, 6:04 a.m. UTC | #4
David Aguilar <davvid@gmail.com> 于2021年2月9日周二 上午6:16写道:
>
> (cc'd Ryan since the thread involving him was mentioned in the commit message)
>
> On Mon, Feb 8, 2021 at 9:02 AM ZheNing Hu via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > `git difftool` only allow us to select file to view in turn.
> > If there is a commit with many files and we exit in the search,
> > We will have to traverse list again to get the file diff which
> > we want to see. Therefore, here is a new method: every time before
> > we view the file diff, the current coordinates will be stored in
> > `GIT_DIR/difftool-last-position`, this file will be deleted after
> > successful traversing. But if an unexpected exit occurred midway or
> > users similar to using "ctrl+c" kill the process,and the user wants
> > to redo the same `git difftoool`, git will view the coordinates in
> > the save point, ask user if they want continue from the last position.
> > This will improve the user experience.
> >
> > Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> > ---
> >     git-difftool-helper.sh: learn a new way skip to save point
> >
> >     git user may should travel the diff list to choice file diff to view, if
> >     they exit in midway,they must travel it again. By saving current
> >     coordinates in GIT_DIR/difftool-last-position method, provides a
> >     possibility for this user-friendly solution.
> >
> >     this patch's origin discuss is here:
> >     https://lore.kernel.org/git/gOXOaoqn-E9A2ob7ykWEcDc7ZxmSwAjcP5CCFKfr5ejCOWZQ1lfAUZcbgYT9AyQCcDgJvCrnrtziXiels-Hxol3xlkGTVHk24SvAdaSUtKQ=@rtzoeller.com/
> >
> >     Thanks!
> >
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-870%2Fadlternative%2Fdifftool_save_point-v2
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-870/adlternative/difftool_save_point-v2
> > Pull-Request: https://github.com/gitgitgadget/git/pull/870
> >
> > Range-diff vs v1:
> >
> >  1:  e77c3e33ba85 ! 1:  2468eaff322b git-difftool-helper.sh: learn a new way skip to save point
> >      @@ Metadata
> >       Author: ZheNing Hu <adlternative@gmail.com>
> >
> >        ## Commit message ##
> >      -    git-difftool-helper.sh: learn a new way skip to save point
> >      +    git-difftool-helper.sh: learn a new way go back to last save point
> >
> >      -    `git difftool` only allow us to select file to view In turn.
> >      -    If there is a commit with many files and we exit in search,
> >      +    `git difftool` only allow us to select file to view in turn.
> >      +    If there is a commit with many files and we exit in the search,
> >           We will have to traverse list again to get the file diff which
> >      -    we want to see.Therefore,here is a new method:every time before
> >      -    we view the file diff,the current coordinates will be stored in
> >      -    `GIT_DIR/difftool_skip_to`,this file will be deleted after
> >      -    successful traversing.But if an unexpected exit occurred midway,
> >      -    git will view the coordinates in the save point,ask user if they
> >      -    want continue from the last saved point.This will improve the
> >      -    user experience.
> >      +    we want to see. Therefore, here is a new method: every time before
> >      +    we view the file diff, the current coordinates will be stored in
> >      +    `GIT_DIR/difftool-last-position`, this file will be deleted after
> >      +    successful traversing. But if an unexpected exit occurred midway or
> >      +    users similar to using "ctrl+c" kill the process,and the user wants
> >      +    to redo the same `git difftoool`, git will view the coordinates in
> >      +    the save point, ask user if they want continue from the last position.
> >      +    This will improve the user experience.
> >
> >           Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> >
> >      @@ git-difftool--helper.sh
> >        # Copyright (c) 2009, 2010 David Aguilar
> >
> >        TOOL_MODE=diff
> >      -+GIT_DIFFTOOL_SKIP_TO_FILE="$GIT_DIR/difftool-skip-to"
> >      ++GIT_DIFFTOOL_LAST_POSITION="$GIT_DIR/difftool-last-position"
> >      ++DIFFTOOL_FIRST_NUM="1"
> >        . git-mergetool--lib
> >
> >        # difftool.prompt controls the default prompt/no-prompt behavior
> >      @@ git-difftool--helper.sh: launch_merge_tool () {
> >         # the user with the real $MERGED name before launching $merge_tool.
> >         if should_prompt
> >         then
> >      -+         if test -f "$GIT_DIFFTOOL_SKIP_TO_FILE"
> >      ++         if test -f "$GIT_DIFFTOOL_LAST_POSITION"
> >       +         then
> >      -+                 SAVE_POINT_NUM=$(cat "$GIT_DIFFTOOL_SKIP_TO_FILE")
> >      -+                 if test $SAVE_POINT_NUM -le $GIT_DIFF_PATH_TOTAL &&
> >      -+                         test $SAVE_POINT_NUM -gt $GIT_DIFF_PATH_COUNTER
> >      ++                 if SAVE_POINT_NUM=$(cat 2>/dev/null "$GIT_DIFFTOOL_LAST_POSITION") &&
> >      ++                         test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
> >      ++                                 test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
> >       +                 then
> >      -+                         # choice skip or not skip when check first file.
> >      -+                         if test $GIT_DIFF_PATH_COUNTER -eq "1"
> >      ++                         if test "$GIT_DIFF_PATH_COUNTER" -eq "$DIFFTOOL_FIRST_NUM"
> >       +                         then
> >      -+                                 printf "do you want to skip to last time difftool save point($SAVE_POINT_NUM) [Y/n]?"
> >      ++                                 printf "Do you want to start from the possible last file you were viewing? [Y/n]?"
> >       +                                 read skip_ans || return
> >       +                                 if test "$skip_ans" = y
> >       +                                 then
> >      @@ git-difftool--helper.sh: launch_merge_tool () {
> >       +                         fi
> >       +                 fi
> >       +         fi
>
>
> Similar to Junio's question about, "where does this possible come
> from?", I wasn't able to make out the behavior in the following
> situation.
>
> What about when the user switches branches or specifies a pathspec on
> the command-line or some other avenue that ends up with the number of
> files to diff being very different than the last difftool invocation?
>
> Will difftool, for example, skip over a smaller set of files on
> invocation 2 if invocation 1 involved many files and we exited out
> with a counter number that is very high?
>
This is what I worry about.
> One thing that's not too good about having state files in .git/ is
> that they're global data and we also have to think about, "what if the
> user has multiple difftools running?" and those kind of complexities.
>
I admit that I did not consider the situation where multiple `git difftool`
processes are going on at the same time.
> I don't want this to seem like I'm trying to be dismissive of this
> feature which does seem like a useful thing in general, so I'll try to
> come up with an alternative interface that is slightly more general
> but a admittedly a little bit more cumbersome because it's not as
> automatic.
>
> What if instead of global state, maybe the user could specify a path
> that difftool could skip forward to?   For example, we could teach
> difftool to resume by telling it where we last left off:
>
>    git difftool --resume-from=foo/bar099.txt
>
> Then we don't need the global counter state file?
>
Wonderful idea.But as Junio said, there may be no global state support,
`start-from` will be more applicable.
>
> Finally, I'm going to plug what I believe to be the right tool for the
> job here.  Have you tried git cola?[1]  Difftool is tightly
> integrated, and the UI is such that you can trivially choose any of
> the modified/staged files and difftool them by using the Ctrl-d
> hotkey.
>
> https://github.com/git-cola/git-cola/
>
> Cola is purpose-built for driving difftool, and for interactive
> staging, so not mentioning it in the context of wanting a better UI
> for difftool would be a disservice to difftool users.
I saw the difftool UI of git-cola, and it is great to view the differences
by selecting files.I have been using vscode's git plugin before, and it
works well tool.
> --
> David
Thanks for help!
--
ZheNing Hu
ZheNing Hu Feb. 9, 2021, 6:19 a.m. UTC | #5
Junio C Hamano <gitster@pobox.com> 于2021年2月9日周二 上午7:34写道:
>
> David Aguilar <davvid@gmail.com> writes:
>
> > What if instead of global state, maybe the user could specify a path
> > that difftool could skip forward to?   For example, we could teach
> > difftool to resume by telling it where we last left off:
> >
> >    git difftool --resume-from=foo/bar099.txt
> >
> > Then we don't need the global counter state file?
>
> Does it have to be the second and subsequent invocation to pass the
> new "resume-from" option?  As we do not have "global" state, I would
> presume that we do not even know if it is the first invocation, so
> perhaps a better name would be "--start-from=$pathname"?
>
Thank you for your thinking, I agree with your point of view.
As you said before: an accurate file name may be more suitable
for users than `possible last file`.
However, without the support of the global `difftool-save-point`,
it may not be possible to know the last exit point of the user.
Even if the global state is allowed, it may need to do more work
to avoid the competition for the global state under multiple
processes.
So "--start-from=$pathname" is more suitable to provide users
with a way to quickly index to specified files.
Then I shift the front and start thinking about how to realize it! :)
> > Finally, I'm going to plug what I believe to be the right tool for the
> > job here.  Have you tried git cola?[1]  Difftool is tightly
> > integrated, and the UI is such that you can trivially choose any of
> > the modified/staged files and difftool them by using the Ctrl-d
> > hotkey.
> >
> > https://github.com/git-cola/git-cola/
> >
> > Cola is purpose-built for driving difftool, and for interactive
> > staging, so not mentioning it in the context of wanting a better UI
> > for difftool would be a disservice to difftool users.
>
> ;-)
Thanks again!
diff mbox series

Patch

diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index 46af3e60b718..a01aa7c9d551 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -6,6 +6,8 @@ 
 # Copyright (c) 2009, 2010 David Aguilar
 
 TOOL_MODE=diff
+GIT_DIFFTOOL_LAST_POSITION="$GIT_DIR/difftool-last-position"
+DIFFTOOL_FIRST_NUM="1"
 . git-mergetool--lib
 
 # difftool.prompt controls the default prompt/no-prompt behavior
@@ -40,6 +42,30 @@  launch_merge_tool () {
 	# the user with the real $MERGED name before launching $merge_tool.
 	if should_prompt
 	then
+		if test -f "$GIT_DIFFTOOL_LAST_POSITION"
+		then
+			if SAVE_POINT_NUM=$(cat 2>/dev/null "$GIT_DIFFTOOL_LAST_POSITION") &&
+				test "$SAVE_POINT_NUM" -le "$GIT_DIFF_PATH_TOTAL" &&
+					test "$SAVE_POINT_NUM" -gt "$GIT_DIFF_PATH_COUNTER"
+			then
+				if test "$GIT_DIFF_PATH_COUNTER" -eq "$DIFFTOOL_FIRST_NUM"
+				then
+					printf "Do you want to start from the possible last file you were viewing? [Y/n]?"
+					read skip_ans || return
+					if test "$skip_ans" = y
+					then
+						return
+					fi
+				else
+					return
+				fi
+			fi
+		fi
+		if test -z "$SAVE_POINT_NUM" ||
+			test "$SAVE_POINT_NUM" -ne "$GIT_DIFF_PATH_COUNTER"
+		then
+			echo "$GIT_DIFF_PATH_COUNTER" >"$GIT_DIFFTOOL_LAST_POSITION"
+		fi
 		printf "\nViewing (%s/%s): '%s'\n" "$GIT_DIFF_PATH_COUNTER" \
 			"$GIT_DIFF_PATH_TOTAL" "$MERGED"
 		if use_ext_cmd
@@ -102,4 +128,9 @@  else
 	done
 fi
 
+if test "$GIT_DIFF_PATH_COUNTER" -eq "$GIT_DIFF_PATH_TOTAL"
+then
+	rm -f "$GIT_DIFFTOOL_LAST_POSITION"
+
+fi
 exit 0