| Message ID | 20201103161631.89971-2-stefan@haller-berlin.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-gui: Auto-rescan on activate | expand |
On 03.11.20 17:16, Stefan Haller wrote: > Do an automatic rescan whenever the git-gui window receives focus. Most other > GUI tools do this, and it's very convenient; no more pressing F5 manually. > > People who don't like this behavior can turn it off in the Options dialog. Ping - any thoughts? I have been running with this patch for a few weeks now, and I already don't want to miss it any more. Cc:-ing a few people who were involved in the discussion on Pratyush's similar patch last summer. [0] [0] <https://lore.kernel.org/git/20190728151726.9188-1- me@yadavpratyush.com/> > > Signed-off-by: Stefan Haller <stefan@haller-berlin.de> > --- > git-gui.sh | 5 +++++ > lib/option.tcl | 1 + > 2 files changed, 6 insertions(+) > > diff --git a/git-gui.sh b/git-gui.sh > index 867b8ce..14735a3 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -906,6 +906,7 @@ set font_descs { > } > set default_config(gui.stageuntracked) ask > set default_config(gui.displayuntracked) true > +set default_config(gui.autorescan) true > > ###################################################################### > ## > @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} > bind . <Alt-Key-3> {focus $::ui_diff} > bind . <Alt-Key-4> {focus $::ui_comm} > > +if {[is_config_true gui.autorescan]} { > + bind . <FocusIn> { if {"%W" eq "."} do_rescan } > +} > + > set file_lists_last_clicked($ui_index) {} > set file_lists_last_clicked($ui_workdir) {} > > diff --git a/lib/option.tcl b/lib/option.tcl > index e43971b..9e83db7 100644 > --- a/lib/option.tcl > +++ b/lib/option.tcl > @@ -145,6 +145,7 @@ proc do_options {} { > {b merge.diffstat {mc "Show Diffstat After Merge"}} > {t merge.tool {mc "Use Merge Tool"}} > > + {b gui.autorescan {mc "Auto-Rescan On Activate"}} > {b gui.trustmtime {mc "Trust File Modification Timestamps"}} > {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} > {b gui.matchtrackingbranch {mc "Match Tracking Branches"}} > -- > 2.29.2 >
Hi Stefan, On 14/11/20 08:14PM, Stefan Haller wrote: > On 03.11.20 17:16, Stefan Haller wrote: > > Do an automatic rescan whenever the git-gui window receives focus. Most other > > GUI tools do this, and it's very convenient; no more pressing F5 manually. > > > > People who don't like this behavior can turn it off in the Options dialog. > > Ping - any thoughts? I have been running with this patch for a few weeks > now, and I already don't want to miss it any more. I have been staring at your patch for the last few days with indecision. I have finally made up my mind. I do not think it is a good idea to hurt the experience of a significant population of our users without at least telling them how they can fix it. The config option is not very visible at all. Experience has told me that people don't often go looking around in the options menu to find a fix for their problem. So we need to do a better job of educating them why they might be experiencing slowdowns and how they can avoid them. Let's take inspiration from git status. When the local branch diverges from the upstream branch, git status shows you how many commits your branch is ahead and how many commits behind upstream. This can be an expensive operation if the divergence point is far behind. In those cases, git status prints something like: It took 30.00 seconds to calculate the branch ahead/behind values. You can use '--no-ahead-behind' to avoid this. This made me aware this option existed and that I can use it to avoid slowdowns. We should do something similar for the auto rescan. Measure how long it takes to finish the rescan and if it takes longer than X seconds then tell the user that they can use this option to disable this. If they don't mind the delay they can keep on using it. I am working on a patch to add this. Will send it in a day or two. > Cc:-ing a few people who were involved in the discussion on Pratyush's > similar patch last summer. [0] > > > [0] <https://lore.kernel.org/git/20190728151726.9188-1- > me@yadavpratyush.com/> > > > > > > Signed-off-by: Stefan Haller <stefan@haller-berlin.de> > > --- > > git-gui.sh | 5 +++++ > > lib/option.tcl | 1 + > > 2 files changed, 6 insertions(+) > > > > diff --git a/git-gui.sh b/git-gui.sh > > index 867b8ce..14735a3 100755 > > --- a/git-gui.sh > > +++ b/git-gui.sh > > @@ -906,6 +906,7 @@ set font_descs { > > } > > set default_config(gui.stageuntracked) ask > > set default_config(gui.displayuntracked) true > > +set default_config(gui.autorescan) true > > > > ###################################################################### > > ## > > @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} > > bind . <Alt-Key-3> {focus $::ui_diff} > > bind . <Alt-Key-4> {focus $::ui_comm} > > > > +if {[is_config_true gui.autorescan]} { > > + bind . <FocusIn> { if {"%W" eq "."} do_rescan } > > +} > > + > > set file_lists_last_clicked($ui_index) {} > > set file_lists_last_clicked($ui_workdir) {} > > > > diff --git a/lib/option.tcl b/lib/option.tcl > > index e43971b..9e83db7 100644 > > --- a/lib/option.tcl > > +++ b/lib/option.tcl > > @@ -145,6 +145,7 @@ proc do_options {} { > > {b merge.diffstat {mc "Show Diffstat After Merge"}} > > {t merge.tool {mc "Use Merge Tool"}} > > > > + {b gui.autorescan {mc "Auto-Rescan On Activate"}} > > {b gui.trustmtime {mc "Trust File Modification Timestamps"}} > > {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} > > {b gui.matchtrackingbranch {mc "Match Tracking Branches"}} > > -- > > 2.29.2 > >
On 17.11.20 8:36, Pratyush Yadav wrote: > Hi Stefan, > > On 14/11/20 08:14PM, Stefan Haller wrote: >> On 03.11.20 17:16, Stefan Haller wrote: >>> Do an automatic rescan whenever the git-gui window receives focus. Most other >>> GUI tools do this, and it's very convenient; no more pressing F5 manually. >>> >>> People who don't like this behavior can turn it off in the Options dialog. >> >> Ping - any thoughts? I have been running with this patch for a few weeks >> now, and I already don't want to miss it any more. > > I have been staring at your patch for the last few days with indecision. > I have finally made up my mind. I do not think it is a good idea to hurt > the experience of a significant population of our users without at least > telling them how they can fix it. > > The config option is not very visible at all. Experience has told me > that people don't often go looking around in the options menu to find a > fix for their problem. So we need to do a better job of educating them > why they might be experiencing slowdowns and how they can avoid them. > > Let's take inspiration from git status. When the local branch diverges > from the upstream branch, git status shows you how many commits your > branch is ahead and how many commits behind upstream. This can be an > expensive operation if the divergence point is far behind. In those > cases, git status prints something like: > > It took 30.00 seconds to calculate the branch ahead/behind values. > You can use '--no-ahead-behind' to avoid this. > > This made me aware this option existed and that I can use it to avoid > slowdowns. > > We should do something similar for the auto rescan. Measure how long it > takes to finish the rescan and if it takes longer than X seconds then > tell the user that they can use this option to disable this. If they > don't mind the delay they can keep on using it. > > I am working on a patch to add this. Will send it in a day or two. Sounds good to me. While I personally don't think such a check is necessary in this case, I also have nothing against it if you find it important. It just needs to be possible to disable that check itself, too. I certainly wouldn't want to be bothered by it, even if the rescan should take longer than whatever threshold you decide on. So if you put up a dialog to inform the user, the dialog should ideally have a "Don't show again" option. >> Cc:-ing a few people who were involved in the discussion on Pratyush's >> similar patch last summer. [0] >> >> >> [0] <https://lore.kernel.org/git/20190728151726.9188-1- >> me@yadavpratyush.com/> >> >> >>> >>> Signed-off-by: Stefan Haller <stefan@haller-berlin.de> >>> --- >>> git-gui.sh | 5 +++++ >>> lib/option.tcl | 1 + >>> 2 files changed, 6 insertions(+) >>> >>> diff --git a/git-gui.sh b/git-gui.sh >>> index 867b8ce..14735a3 100755 >>> --- a/git-gui.sh >>> +++ b/git-gui.sh >>> @@ -906,6 +906,7 @@ set font_descs { >>> } >>> set default_config(gui.stageuntracked) ask >>> set default_config(gui.displayuntracked) true >>> +set default_config(gui.autorescan) true >>> >>> ###################################################################### >>> ## >>> @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} >>> bind . <Alt-Key-3> {focus $::ui_diff} >>> bind . <Alt-Key-4> {focus $::ui_comm} >>> >>> +if {[is_config_true gui.autorescan]} { >>> + bind . <FocusIn> { if {"%W" eq "."} do_rescan } >>> +} >>> + >>> set file_lists_last_clicked($ui_index) {} >>> set file_lists_last_clicked($ui_workdir) {} >>> >>> diff --git a/lib/option.tcl b/lib/option.tcl >>> index e43971b..9e83db7 100644 >>> --- a/lib/option.tcl >>> +++ b/lib/option.tcl >>> @@ -145,6 +145,7 @@ proc do_options {} { >>> {b merge.diffstat {mc "Show Diffstat After Merge"}} >>> {t merge.tool {mc "Use Merge Tool"}} >>> >>> + {b gui.autorescan {mc "Auto-Rescan On Activate"}} >>> {b gui.trustmtime {mc "Trust File Modification Timestamps"}} >>> {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} >>> {b gui.matchtrackingbranch {mc "Match Tracking Branches"}} >>> -- >>> 2.29.2 >>> >
On 17/11/20 12:13PM, Stefan Haller wrote: > On 17.11.20 8:36, Pratyush Yadav wrote: > > Hi Stefan, > > > > On 14/11/20 08:14PM, Stefan Haller wrote: > >> On 03.11.20 17:16, Stefan Haller wrote: > >>> Do an automatic rescan whenever the git-gui window receives focus. Most other > >>> GUI tools do this, and it's very convenient; no more pressing F5 manually. > >>> > >>> People who don't like this behavior can turn it off in the Options dialog. > >> > >> Ping - any thoughts? I have been running with this patch for a few weeks > >> now, and I already don't want to miss it any more. > > > > I have been staring at your patch for the last few days with indecision. > > I have finally made up my mind. I do not think it is a good idea to hurt > > the experience of a significant population of our users without at least > > telling them how they can fix it. > > > > The config option is not very visible at all. Experience has told me > > that people don't often go looking around in the options menu to find a > > fix for their problem. So we need to do a better job of educating them > > why they might be experiencing slowdowns and how they can avoid them. > > > > Let's take inspiration from git status. When the local branch diverges > > from the upstream branch, git status shows you how many commits your > > branch is ahead and how many commits behind upstream. This can be an > > expensive operation if the divergence point is far behind. In those > > cases, git status prints something like: > > > > It took 30.00 seconds to calculate the branch ahead/behind values. > > You can use '--no-ahead-behind' to avoid this. > > > > This made me aware this option existed and that I can use it to avoid > > slowdowns. > > > > We should do something similar for the auto rescan. Measure how long it > > takes to finish the rescan and if it takes longer than X seconds then > > tell the user that they can use this option to disable this. If they > > don't mind the delay they can keep on using it. > > > > I am working on a patch to add this. Will send it in a day or two. > > Sounds good to me. While I personally don't think such a check is > necessary in this case, I also have nothing against it if you find it > important. > > It just needs to be possible to disable that check itself, too. I > certainly wouldn't want to be bothered by it, even if the rescan should > take longer than whatever threshold you decide on. So if you put up a > dialog to inform the user, the dialog should ideally have a "Don't show > again" option. That's the plan. It will be a yes/no/cancel prompt. Saying yes or no sets auto rescan to on/off and the message won't pop up again. Saying cancel does nothing and you will see the popup again on the next long rescan. > > >> Cc:-ing a few people who were involved in the discussion on Pratyush's > >> similar patch last summer. [0] > >> > >> > >> [0] <https://lore.kernel.org/git/20190728151726.9188-1- > >> me@yadavpratyush.com/> > >> > >> > >>> > >>> Signed-off-by: Stefan Haller <stefan@haller-berlin.de> > >>> --- > >>> git-gui.sh | 5 +++++ > >>> lib/option.tcl | 1 + > >>> 2 files changed, 6 insertions(+) > >>> > >>> diff --git a/git-gui.sh b/git-gui.sh > >>> index 867b8ce..14735a3 100755 > >>> --- a/git-gui.sh > >>> +++ b/git-gui.sh > >>> @@ -906,6 +906,7 @@ set font_descs { > >>> } > >>> set default_config(gui.stageuntracked) ask > >>> set default_config(gui.displayuntracked) true > >>> +set default_config(gui.autorescan) true > >>> > >>> ###################################################################### > >>> ## > >>> @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} > >>> bind . <Alt-Key-3> {focus $::ui_diff} > >>> bind . <Alt-Key-4> {focus $::ui_comm} > >>> > >>> +if {[is_config_true gui.autorescan]} { > >>> + bind . <FocusIn> { if {"%W" eq "."} do_rescan } > >>> +} > >>> + > >>> set file_lists_last_clicked($ui_index) {} > >>> set file_lists_last_clicked($ui_workdir) {} > >>> > >>> diff --git a/lib/option.tcl b/lib/option.tcl > >>> index e43971b..9e83db7 100644 > >>> --- a/lib/option.tcl > >>> +++ b/lib/option.tcl > >>> @@ -145,6 +145,7 @@ proc do_options {} { > >>> {b merge.diffstat {mc "Show Diffstat After Merge"}} > >>> {t merge.tool {mc "Use Merge Tool"}} > >>> > >>> + {b gui.autorescan {mc "Auto-Rescan On Activate"}} > >>> {b gui.trustmtime {mc "Trust File Modification Timestamps"}} > >>> {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} > >>> {b gui.matchtrackingbranch {mc "Match Tracking Branches"}} > >>> -- > >>> 2.29.2 > >>> > >
On 17.11.20 13:05, Pratyush Yadav wrote: > On 17/11/20 12:13PM, Stefan Haller wrote: >> On 17.11.20 8:36, Pratyush Yadav wrote: >>> Hi Stefan, >>> >>> On 14/11/20 08:14PM, Stefan Haller wrote: >>>> On 03.11.20 17:16, Stefan Haller wrote: >>>>> Do an automatic rescan whenever the git-gui window receives focus. Most other >>>>> GUI tools do this, and it's very convenient; no more pressing F5 manually. >>>>> >>>>> People who don't like this behavior can turn it off in the Options dialog. >>>> >>>> Ping - any thoughts? I have been running with this patch for a few weeks >>>> now, and I already don't want to miss it any more. >>> >>> I have been staring at your patch for the last few days with indecision. >>> I have finally made up my mind. I do not think it is a good idea to hurt >>> the experience of a significant population of our users without at least >>> telling them how they can fix it. >>> >>> The config option is not very visible at all. Experience has told me >>> that people don't often go looking around in the options menu to find a >>> fix for their problem. So we need to do a better job of educating them >>> why they might be experiencing slowdowns and how they can avoid them. >>> >>> Let's take inspiration from git status. When the local branch diverges >>> from the upstream branch, git status shows you how many commits your >>> branch is ahead and how many commits behind upstream. This can be an >>> expensive operation if the divergence point is far behind. In those >>> cases, git status prints something like: >>> >>> It took 30.00 seconds to calculate the branch ahead/behind values. >>> You can use '--no-ahead-behind' to avoid this. >>> >>> This made me aware this option existed and that I can use it to avoid >>> slowdowns. >>> >>> We should do something similar for the auto rescan. Measure how long it >>> takes to finish the rescan and if it takes longer than X seconds then >>> tell the user that they can use this option to disable this. If they >>> don't mind the delay they can keep on using it. >>> >>> I am working on a patch to add this. Will send it in a day or two. >> >> Sounds good to me. While I personally don't think such a check is >> necessary in this case, I also have nothing against it if you find it >> important. >> >> It just needs to be possible to disable that check itself, too. I >> certainly wouldn't want to be bothered by it, even if the rescan should >> take longer than whatever threshold you decide on. So if you put up a >> dialog to inform the user, the dialog should ideally have a "Don't show >> again" option. > > That's the plan. It will be a yes/no/cancel prompt. Saying yes or no > sets auto rescan to on/off and the message won't pop up again. Saying > cancel does nothing and you will see the popup again on the next long > rescan. Interesting. After thinking about this for a while, I'm not convinced that a Yes/No/Cancel dialog is the best user experience for this, for the following reasons: - It isn't obvious whether clicking No will turn auto-rescan off only for this repo, or globally (unless you provide two different buttons for this). - It doesn't teach users how they can turn it back on if they clicked No too hastily (e.g. because they didn't immediately understand the difference between No and Cancel). - It isn't really intuitively obvious what "Cancel" means. What is the operation that is being cancelled here? The rescan itself has already happened. (Yes, I know that the operation being cancelled is the process of deciding whether the option should be turned off, but as I said, I don't find this intuitive.) I think it might be a better user experience to have a dialog like this: The automatic rescan on activating the application has taken more than X seconds. If this bothers you, you can turn it off in the Preferences dialog. [x] Don't show again [OK] The only downside is that it's more work to implement, as you can't use tk_messageBox. All of this is just my humble opinion; if you decide to stick to your plan, that's ok with me too. >>>> Cc:-ing a few people who were involved in the discussion on Pratyush's >>>> similar patch last summer. [0] >>>> >>>> >>>> [0] <https://lore.kernel.org/git/20190728151726.9188-1- >>>> me@yadavpratyush.com/> >>>> >>>> >>>>> >>>>> Signed-off-by: Stefan Haller <stefan@haller-berlin.de> >>>>> --- >>>>> git-gui.sh | 5 +++++ >>>>> lib/option.tcl | 1 + >>>>> 2 files changed, 6 insertions(+) >>>>> >>>>> diff --git a/git-gui.sh b/git-gui.sh >>>>> index 867b8ce..14735a3 100755 >>>>> --- a/git-gui.sh >>>>> +++ b/git-gui.sh >>>>> @@ -906,6 +906,7 @@ set font_descs { >>>>> } >>>>> set default_config(gui.stageuntracked) ask >>>>> set default_config(gui.displayuntracked) true >>>>> +set default_config(gui.autorescan) true >>>>> >>>>> ###################################################################### >>>>> ## >>>>> @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} >>>>> bind . <Alt-Key-3> {focus $::ui_diff} >>>>> bind . <Alt-Key-4> {focus $::ui_comm} >>>>> >>>>> +if {[is_config_true gui.autorescan]} { >>>>> + bind . <FocusIn> { if {"%W" eq "."} do_rescan } >>>>> +} >>>>> + >>>>> set file_lists_last_clicked($ui_index) {} >>>>> set file_lists_last_clicked($ui_workdir) {} >>>>> >>>>> diff --git a/lib/option.tcl b/lib/option.tcl >>>>> index e43971b..9e83db7 100644 >>>>> --- a/lib/option.tcl >>>>> +++ b/lib/option.tcl >>>>> @@ -145,6 +145,7 @@ proc do_options {} { >>>>> {b merge.diffstat {mc "Show Diffstat After Merge"}} >>>>> {t merge.tool {mc "Use Merge Tool"}} >>>>> >>>>> + {b gui.autorescan {mc "Auto-Rescan On Activate"}} >>>>> {b gui.trustmtime {mc "Trust File Modification Timestamps"}} >>>>> {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} >>>>> {b gui.matchtrackingbranch {mc "Match Tracking Branches"}} >>>>> -- >>>>> 2.29.2 >>>>> >>> >
diff --git a/git-gui.sh b/git-gui.sh index 867b8ce..14735a3 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -906,6 +906,7 @@ set font_descs { } set default_config(gui.stageuntracked) ask set default_config(gui.displayuntracked) true +set default_config(gui.autorescan) true ###################################################################### ## @@ -4007,6 +4008,10 @@ bind . <Alt-Key-2> {focus_widget $::ui_index} bind . <Alt-Key-3> {focus $::ui_diff} bind . <Alt-Key-4> {focus $::ui_comm} +if {[is_config_true gui.autorescan]} { + bind . <FocusIn> { if {"%W" eq "."} do_rescan } +} + set file_lists_last_clicked($ui_index) {} set file_lists_last_clicked($ui_workdir) {} diff --git a/lib/option.tcl b/lib/option.tcl index e43971b..9e83db7 100644 --- a/lib/option.tcl +++ b/lib/option.tcl @@ -145,6 +145,7 @@ proc do_options {} { {b merge.diffstat {mc "Show Diffstat After Merge"}} {t merge.tool {mc "Use Merge Tool"}} + {b gui.autorescan {mc "Auto-Rescan On Activate"}} {b gui.trustmtime {mc "Trust File Modification Timestamps"}} {b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}} {b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
Do an automatic rescan whenever the git-gui window receives focus. Most other GUI tools do this, and it's very convenient; no more pressing F5 manually. People who don't like this behavior can turn it off in the Options dialog. Signed-off-by: Stefan Haller <stefan@haller-berlin.de> --- git-gui.sh | 5 +++++ lib/option.tcl | 1 + 2 files changed, 6 insertions(+) -- 2.29.2