threads / discuss / 51848

git-gui: automatically move focus to staged file before typing commit message?

Subject: git-gui: automatically move focus to staged file before typing commit message?

## tl;dr

22 messages between Sep 14, 2019 and Oct 17, 2019.

replies: 21people: 3as markdown or json

Birger Skogeng Pedersen· Sep 14, 2019, 12:24 UTC · lore
Hi everyone,
I personally prefer to have the changes I am about to commit visible
in the diff view, while I write my commit message. So usually I do
this:
1. Stage the file(s) I've been working on.
2. Select a file I just staged, so I can see the changes in the diff widget.
3. Jump to the "Commit Message" widget to type up my commit message.
Basically, I would like to be able to skip step 2. When the user
stages the last file in the "Unstaged Changes" widget, no file is
selected and the diff view becomes blank. When this is the case, I
would prefer that git-gui automatically selects one of the staged
files and shows it in the diff widget before I type up my commit
message. Naturally, this automatic selection should **only** happen
when the user chooses focus the "Commit Message" widget.

I propose: (When the user focuses the "Commit Message" widget, if no file is currently selected (i.e. diff widget shows no text)) automatically select the first file listed in the "Staged Changes" widget so the changes of that file show up in the diff widget.

Thoughts?

Best regards, Birger S Pedersen

Pratyush Yadav· Sep 14, 2019, 21:15 UTC · re: Birger Skogeng Pedersen · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On 14/09/19 02:24PM, Birger Skogeng Pedersen wrote:
Show 16 quoted lines
> Hi everyone,
> 
> 
> I personally prefer to have the changes I am about to commit visible
> in the diff view, while I write my commit message. So usually I do
> this:
> 1. Stage the file(s) I've been working on.
> 2. Select a file I just staged, so I can see the changes in the diff widget.
> 3. Jump to the "Commit Message" widget to type up my commit message.
> Basically, I would like to be able to skip step 2. When the user
> stages the last file in the "Unstaged Changes" widget, no file is
> selected and the diff view becomes blank. When this is the case, I
> would prefer that git-gui automatically selects one of the staged
> files and shows it in the diff widget before I type up my commit
> message. Naturally, this automatic selection should **only** happen
> when the user chooses focus the "Commit Message" widget.

Why should it only happen when the commit message widget is selected? What's wrong with directly switching focus when all the files are staged?

What I have in mind is once there are no more files to stage, the focus 
directly goes to the staged files section, and the first staged file 
gets selected. Then if you want you can type in the commit message. And 
conversely, when unstaging things, once all files are unstaged, the 
focus goes directly to the unstaged files section.
 
Show 5 quoted lines
> I propose:
> (When the user focuses the "Commit Message" widget, if no file is
> currently selected (i.e. diff widget shows no text))
> automatically select the first file listed in the "Staged Changes"
> widget so the changes of that file show up in the diff widget.
-- 
Regards,
Pratyush Yadav
Johannes Sixt· Sep 14, 2019, 21:26 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

Am 14.09.19 um 23:15 schrieb Pratyush Yadav:
Show 12 quoted lines
> On 14/09/19 02:24PM, Birger Skogeng Pedersen wrote:
>> When the user
>> stages the last file in the "Unstaged Changes" widget, no file is
>> selected and the diff view becomes blank. When this is the case, I
>> would prefer that git-gui automatically selects one of the staged
>> files and shows it in the diff widget before I type up my commit
>> message. Naturally, this automatic selection should **only** happen
>> when the user chooses focus the "Commit Message" widget.
> 
> Why should it only happen when the commit message widget is selected? 
> What's wrong with directly switching focus when all the files are 
> staged?
That was my reaction, too.
> What I have in mind is once there are no more files to stage, the focus 
> directly goes to the staged files section, and the first staged file 
> gets selected.

... or the last one that was staged. Typically, it is a fixup found during testing that is staged last. Then I like to have a look at the complete staged changes of that file.

 Then if you want you can type in the commit message. And
> conversely, when unstaging things, once all files are unstaged, the 
> focus goes directly to the unstaged files section.
Same here.
-- Hannes
Pratyush Yadav· Sep 14, 2019, 21:27 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On 15/09/19 02:45AM, Pratyush Yadav wrote:
Show 21 quoted lines
> On 14/09/19 02:24PM, Birger Skogeng Pedersen wrote:
> > Hi everyone,
> > 
> > 
> > I personally prefer to have the changes I am about to commit visible
> > in the diff view, while I write my commit message. So usually I do
> > this:
> > 1. Stage the file(s) I've been working on.
> > 2. Select a file I just staged, so I can see the changes in the diff widget.
> > 3. Jump to the "Commit Message" widget to type up my commit message.
> > Basically, I would like to be able to skip step 2. When the user
> > stages the last file in the "Unstaged Changes" widget, no file is
> > selected and the diff view becomes blank. When this is the case, I
> > would prefer that git-gui automatically selects one of the staged
> > files and shows it in the diff widget before I type up my commit
> > message. Naturally, this automatic selection should **only** happen
> > when the user chooses focus the "Commit Message" widget.
> 
> Why should it only happen when the commit message widget is selected? 
> What's wrong with directly switching focus when all the files are 
> staged?

Re-reading this email, this might have come across as a bit too harsh. That was not my intention. I just want to know why you think it should only happen when focussing on the commit message, and what in your mind is the problem with directly switching when no more files are left.

Show 16 quoted lines
> 
> What I have in mind is once there are no more files to stage, the focus 
> directly goes to the staged files section, and the first staged file 
> gets selected. Then if you want you can type in the commit message. And 
> conversely, when unstaging things, once all files are unstaged, the 
> focus goes directly to the unstaged files section.
>  
> > I propose:
> > (When the user focuses the "Commit Message" widget, if no file is
> > currently selected (i.e. diff widget shows no text))
> > automatically select the first file listed in the "Staged Changes"
> > widget so the changes of that file show up in the diff widget.
> 
> -- 
> Regards,
> Pratyush Yadav
-- 
Regards,
Pratyush Yadav
Birger Skogeng Pedersen· Sep 15, 2019, 07:55 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

Hi Pratyush,
On Sat, Sep 14, 2019 at 11:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
Show 9 quoted lines
> Why should it only happen when the commit message widget is selected?
> What's wrong with directly switching focus when all the files are
> staged?
>
> What I have in mind is once there are no more files to stage, the focus
> directly goes to the staged files section, and the first staged file
> gets selected. Then if you want you can type in the commit message. And
> conversely, when unstaging things, once all files are unstaged, the
> focus goes directly to the unstaged files section.

Your questions are fair. My reasoning: I imagine it could be a bit frustrating that the focus automatically goes away from the "Unstaged Changes" widget, when the user actually isn't done doing changes.

For instance (as a user);
- Do some changes
- Stage the changes (no more unstaged changes in the repo)
- Realize that you forgot something, jump back to the IDE and make
some more changes
- Jump back again to git-gui, hit refresh
In this scenario, I imagine the user would want to have focus kept on
the "Unstaged Changes" widget. Even if it became empty with files
before.

When the user focus the "Commit Message" widget, the user is kinda stating "I'm done staging stuff for now". And when that happens, it really doesn't make sense to show a blank diff any more.

I hope that made sense.
Birger
Pratyush Yadav· Sep 16, 2019, 18:01 UTC · re: Birger Skogeng Pedersen · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On 15/09/19 09:55AM, Birger Skogeng Pedersen wrote:
Show 16 quoted lines
> Hi Pratyush,
> 
> On Sat, Sep 14, 2019 at 11:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > Why should it only happen when the commit message widget is selected?
> > What's wrong with directly switching focus when all the files are
> > staged?
> >
> > What I have in mind is once there are no more files to stage, the focus
> > directly goes to the staged files section, and the first staged file
> > gets selected. Then if you want you can type in the commit message. And
> > conversely, when unstaging things, once all files are unstaged, the
> > focus goes directly to the unstaged files section.
> 
> Your questions are fair. My reasoning: I imagine it could be a bit
> frustrating that the focus automatically goes away from the "Unstaged
> Changes" widget, when the user actually isn't done doing changes.

I suppose a similar argument can be made against your suggestion though. When a user clicks on the commit message buffer, they did one thing: click on the buffer. They did not click on any diff. So, wouldn't it be disorienting for them if their action of clicking the commit message buffer also switches the diff view?

I'm not arguing in favour or against your suggestion, I just want to 
consider all angles/viewpoints before going forward.
 
Show 13 quoted lines
> For instance (as a user);
> - Do some changes
> - Stage the changes (no more unstaged changes in the repo)
> - Realize that you forgot something, jump back to the IDE and make
> some more changes
> - Jump back again to git-gui, hit refresh
> In this scenario, I imagine the user would want to have focus kept on
> the "Unstaged Changes" widget. Even if it became empty with files
> before.
> 
> When the user focus the "Commit Message" widget, the user is kinda
> stating "I'm done staging stuff for now". And when that happens, it
> really doesn't make sense to show a blank diff any more.

Makes sense. But I'm not sure if this would be beneficial to other git-gui users. I'd like to hear about what other people think about this change.

-- 
Regards,
Pratyush Yadav
Birger Skogeng Pedersen· Sep 26, 2019, 18:33 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

Honestly I'll need some help to get this one implemented. The only
implementation I've got working currently, is to change Alt+4 key bind
to do the following:
- Focus the "Staged Changes" widget (which will select a path in the
list, if it isn't focused already), then
- Focus the "Commit Message" widget

¯\_(ツ)_/¯ Birger

Pratyush Yadav· Sep 26, 2019, 19:30 UTC · re: Birger Skogeng Pedersen · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On 26/09/19 08:33PM, Birger Skogeng Pedersen wrote:
Show 6 quoted lines
> Honestly I'll need some help to get this one implemented. The only
> implementation I've got working currently, is to change Alt+4 key bind
> to do the following:
> - Focus the "Staged Changes" widget (which will select a path in the
> list, if it isn't focused already), then
> - Focus the "Commit Message" widget

Why are you changing the Alt+4 binding? This means your feature won't work for people who use the mouse to move around in the UI (which I suppose would be a majority).

Did you try binding a script to the FocusIn event of the commit message 
buffer? You can do this like:
  
  bind $ui_comm <FocusIn> {your_script}

I'm not sure if $ui_comm is the correct widget, but you can experiment a bit by printing something in your_script to find out for sure.

-- 
Regards,
Pratyush Yadav
Birger Skogeng Pedersen· Sep 26, 2019, 21:17 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On Thu, Sep 26, 2019 at 9:30 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> Why are you changing the Alt+4 binding?
I couldn't really find an easier way before.
> This means your feature won't
> work for people who use the mouse to move around in the UI (which I
> suppose would be a majority).

True. I would much prefer that the staged file is selected on commit widget focus, regardless of how the focus was changed (hotkey or mouse).

Show 7 quoted lines
> Did you try binding a script to the FocusIn event of the commit message
> buffer? You can do this like:
>
>   bind $ui_comm <FocusIn> {your_script}
>
> I'm not sure if $ui_comm is the correct widget, but you can experiment a
> bit by printing something in your_script to find out for sure.
Ah, that's pretty neat! I'll play around with that. Thanks, Pratyush.
Birger
Birger Skogeng Pedersen· Oct 7, 2019, 16:52 UTC · re: Birger Skogeng Pedersen · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

So I kinda got this working, but only when focusing the commit message widget.

I did not manage to get it working when invoking "do_add_all", (e.g. when pressing CTRL/CMD+i). I added this:

bind $ui_comm <$M1B-Key-i> {do_add_all;select_staged_file;break} bind $ui_comm <$M1B-Key-I> {do_add_all;select_staged_file;break}

But it seems that the "select_staged_file" procedure is invoked _before_ "do_add_all" finishes. So that's not working. All changes gets staged, but no staged change is selected.

And I'm quite stuck. Do I send the unfinished patch, so maybe I can get some advice? Or is it better to just wait until I have the perfect patch ready?

Birger
Birger Skogeng Pedersen· Oct 7, 2019, 17:11 UTC · re: Birger Skogeng Pedersen · lore

[PATCH 1/2] git-gui: implement proc select_path_in_widget

Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
---
 git-gui.sh | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/git-gui.sh b/git-gui.sh
index fd476b6..b7f4d1e 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2669,25 +2669,31 @@ proc show_less_context {} {
 }
 
 proc focus_widget {widget} {
-	global file_lists last_clicked selected_paths
-	global file_lists_last_clicked
+	global file_lists
 
 	if {[llength $file_lists($widget)] > 0} {
-		set path $file_lists_last_clicked($widget)
-		set index [lsearch -sorted -exact $file_lists($widget) $path]
-		if {$index < 0} {
-			set index 0
-			set path [lindex $file_lists($widget) $index]
-		}
-
+		select_path_in_widget $widget
 		focus $widget
-		set last_clicked [list $widget [expr $index + 1]]
-		array unset selected_paths
-		set selected_paths($path) 1
-		show_diff $path $widget
 	}
 }
 
+proc select_path_in_widget {widget} {
+	global file_lists last_clicked selected_paths
+	global file_lists_last_clicked
+
+	set path $file_lists_last_clicked($widget)
+	set index [lsearch -sorted -exact $file_lists($widget) $path]
+	if {$index < 0} {
+		set index 0
+		set path [lindex $file_lists($widget) $index]
+	}
+
+	set last_clicked [list $widget [expr $index + 1]]
+	array unset selected_paths
+	set selected_paths($path) 1
+	show_diff $path $widget
+}
+
 proc toggle_commit_type {} {
 	global commit_type_is_amend
 	set commit_type_is_amend [expr !$commit_type_is_amend]
-- 
2.23.0.windows.1
Birger Skogeng Pedersen· Oct 7, 2019, 17:11 UTC · re: Birger Skogeng Pedersen · lore

[PATCH 2/2] git-gui: select staged on ui_comm focus

When the user focuses the Commit Message widget (to write a message), the diff view may be blank.

With this patch a staged file is automatically selected when the Commit Message widget is focused, if no other file is selected (i.e. diff view is blank).

Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
---
 git-gui.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)
diff --git a/git-gui.sh b/git-gui.sh
index b7f4d1e..70b846a 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2700,6 +2700,15 @@ proc toggle_commit_type {} {
 	do_select_commit_type
 }
 
+proc check_diff_selected {} {
+	global current_diff_path file_lists
+	# If no diff path selected, select a staged file
+	if {$current_diff_path eq {}
+		&& [llength $file_lists($::ui_index)] > 0} {
+		select_path_in_widget $::ui_index
+	}
+}
+
 ######################################################################
 ##
 ## ui construction
@@ -3437,6 +3446,8 @@ pack .vpane.lower.commarea.buffer.header -side top -fill x
 pack .vpane.lower.commarea.buffer.frame -side left -fill y
 pack .vpane.lower.commarea.buffer -side left -fill y
 
+bind $ui_comm <FocusIn> {check_diff_selected}
+
 # -- Commit Message Buffer Context Menu
 #
 set ctxm .vpane.lower.commarea.buffer.ctxm
-- 
2.23.0.windows.1
Pratyush Yadav· Oct 8, 2019, 17:59 UTC · re: Birger Skogeng Pedersen · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

On 07/10/19 06:52PM, Birger Skogeng Pedersen wrote:
> So I kinda got this working, but only when focusing the commit message widget.
Isn't this the point of your feature? You change the view when focusing 
the commit message widget. I remember you were explicitly against doing 
it as soon as all unstaged files were staged. Did you change your point 
of view on that?
 
Show 9 quoted lines
> I did not manage to get it working when invoking "do_add_all", (e.g.
> when pressing CTRL/CMD+i). I added this:
> 
> bind $ui_comm <$M1B-Key-i> {do_add_all;select_staged_file;break}
> bind $ui_comm <$M1B-Key-I> {do_add_all;select_staged_file;break}
> 
> But it seems that the "select_staged_file" procedure is invoked
> _before_ "do_add_all" finishes. So that's not working. All changes
> gets staged, but no staged change is selected.
Hmm, that shouldn't happen. select_staged_file should be executed 
_after_ do_add_all, not before. I haven't looked into your patches yet 
though.
 
> And I'm quite stuck. Do I send the unfinished patch, so maybe I can
> get some advice? Or is it better to just wait until I have the perfect
> patch ready?

If you are stuck on something, and want to share the WIP feature to get help/comments, you should mark your patches as "RFC" (Request For Comments). This can be done by passing the option '-rfc' to `git-format-patch`. This will make your subject prefix to "RFC PATCH" instead of just "PATCH".

And yes, it is perfectly OK to send incomplete changes as long as you mark them as such, and specify what you need help with.

But I see that you have already sent those patches. I'm not sure when I can find time to tinker around with them, so it might take me a couple of days to get to them.

-- 
Regards,
Pratyush Yadav
Birger Skogeng Pedersen· Oct 8, 2019, 19:46 UTC · re: Pratyush Yadav · lore

Re: git-gui: automatically move focus to staged file before typing commit message?

Hi Pratyush,
On Tue, Oct 8, 2019 at 7:59 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
Show 7 quoted lines
> On 07/10/19 06:52PM, Birger Skogeng Pedersen wrote:
> > So I kinda got this working, but only when focusing the commit message widget.
>
> Isn't this the point of your feature? You change the view when focusing
> the commit message widget. I remember you were explicitly against doing
> it as soon as all unstaged files were staged. Did you change your point
> of view on that?

Yes, kindof. Sorry for not being more clear about the intent. I would prefer that the automatic selection of a staged file happens also when adding files while the Commit Message widget is in focused. For instance, if the user focuses the Commit Message widget and then hits CTRL/CMD+i a staged file should be selected (if it isn't already).

Show 13 quoted lines
> > I did not manage to get it working when invoking "do_add_all", (e.g.
> > when pressing CTRL/CMD+i). I added this:
> >
> > bind $ui_comm <$M1B-Key-i> {do_add_all;select_staged_file;break}
> > bind $ui_comm <$M1B-Key-I> {do_add_all;select_staged_file;break}
> >
> > But it seems that the "select_staged_file" procedure is invoked
> > _before_ "do_add_all" finishes. So that's not working. All changes
> > gets staged, but no staged change is selected.
>
> Hmm, that shouldn't happen. select_staged_file should be executed
> _after_ do_add_all, not before. I haven't looked into your patches yet
> though.
I don't understand why it doesn't work. I'll play around some more with it.
Show 9 quoted lines
> > And I'm quite stuck. Do I send the unfinished patch, so maybe I can
> > get some advice? Or is it better to just wait until I have the perfect
> > patch ready?
>
> If you are stuck on something, and want to share the WIP feature to get
> help/comments, you should mark your patches as "RFC" (Request For
> Comments). This can be done by passing the option '-rfc' to
> `git-format-patch`. This will make your subject prefix to "RFC PATCH"
> instead of just "PATCH".
Sorry for not doing that, I'll take not and do that next time.

Thanks! Birger

Pratyush Yadav· Oct 13, 2019, 20:21 UTC · re: Birger Skogeng Pedersen · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

Hi Birger,

Your subject is a bit redundant. A reader of this commit can easily see the diff and know that you implemented "proc select_path_in_widget". What's more important is why you implemented it. That is what should go in the commit message. So for example in this patch, you can say something like:

  git-gui: move last clicked path selection logic to a separate function
  This same logic will be used elsewhere in a follow-up commit, so make 
  it re-useable.

This is what I came up with at first thought. Maybe something even better and concise can say the same thing.

On 07/10/19 07:11PM, Birger Skogeng Pedersen wrote:
Show 31 quoted lines
> Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> ---
>  git-gui.sh | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index fd476b6..b7f4d1e 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2669,25 +2669,31 @@ proc show_less_context {} {
>  }
>  
>  proc focus_widget {widget} {
> -	global file_lists last_clicked selected_paths
> -	global file_lists_last_clicked
> +	global file_lists
>  
>  	if {[llength $file_lists($widget)] > 0} {
> -		set path $file_lists_last_clicked($widget)
> -		set index [lsearch -sorted -exact $file_lists($widget) $path]
> -		if {$index < 0} {
> -			set index 0
> -			set path [lindex $file_lists($widget) $index]
> -		}
> -
> +		select_path_in_widget $widget
>  		focus $widget
> -		set last_clicked [list $widget [expr $index + 1]]
> -		array unset selected_paths
> -		set selected_paths($path) 1
> -		show_diff $path $widget

There is a change in the order of events here. Earlier, we first focussed the widget, and then ran `show_diff`. Now we first run `show_diff` (via `select_path_in_widget`), and then focus the widget. This won't cause any problems, right?

Show 23 quoted lines
>  	}
>  }
>  
> +proc select_path_in_widget {widget} {
> +	global file_lists last_clicked selected_paths
> +	global file_lists_last_clicked
> +
> +	set path $file_lists_last_clicked($widget)
> +	set index [lsearch -sorted -exact $file_lists($widget) $path]
> +	if {$index < 0} {
> +		set index 0
> +		set path [lindex $file_lists($widget) $index]
> +	}
> +
> +	set last_clicked [list $widget [expr $index + 1]]
> +	array unset selected_paths
> +	set selected_paths($path) 1
> +	show_diff $path $widget
> +}
> +
>  proc toggle_commit_type {} {
>  	global commit_type_is_amend
>  	set commit_type_is_amend [expr !$commit_type_is_amend]

Other than that, looks good. There isn't much changed here. Just some code moved around.

-- 
Regards,
Pratyush Yadav
Pratyush Yadav· Oct 16, 2019, 19:25 UTC · re: Birger Skogeng Pedersen · lore

Re: [PATCH 2/2] git-gui: select staged on ui_comm focus

On 07/10/19 07:11PM, Birger Skogeng Pedersen wrote:
Show 25 quoted lines
> When the user focuses the Commit Message widget (to write a message), the
> diff view may be blank.
> 
> With this patch a staged file is automatically selected when the Commit
> Message widget is focused, if no other file is selected (i.e. diff view
> is blank).
> 
> Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com>
> ---
>  git-gui.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/git-gui.sh b/git-gui.sh
> index b7f4d1e..70b846a 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2700,6 +2700,15 @@ proc toggle_commit_type {} {
>  	do_select_commit_type
>  }
>  
> +proc check_diff_selected {} {
> +	global current_diff_path file_lists
> +	# If no diff path selected, select a staged file
> +	if {$current_diff_path eq {}
> +		&& [llength $file_lists($::ui_index)] > 0} {
Nitpick: Please declare ui_index to be global. That way we can just use 
$ui_index instead of the more tedious $::ui_index.
Show 13 quoted lines
> +		select_path_in_widget $::ui_index
> +	}
> +}
> +
>  ######################################################################
>  ##
>  ## ui construction
> @@ -3437,6 +3446,8 @@ pack .vpane.lower.commarea.buffer.header -side top -fill x
>  pack .vpane.lower.commarea.buffer.frame -side left -fill y
>  pack .vpane.lower.commarea.buffer -side left -fill y
>  
> +bind $ui_comm <FocusIn> {check_diff_selected}
> +

This would mean the diff shows _only_ when you switch focus to the commit message buffer. If the buffer is already in focus, and you stage all files via Ctrl-I, the staged diff would not show.

IIRC you were having some trouble with this. A quick suggestion without looking too much into the problem is to try putting the logic inside `do_add_all` instead of inside the bind event handler.

Show 6 quoted lines
>  # -- Commit Message Buffer Context Menu
>  #
>  set ctxm .vpane.lower.commarea.buffer.ctxm
> -- 
> 2.23.0.windows.1
> 
-- 
Regards,
Pratyush Yadav
Pratyush Yadav· Oct 16, 2019, 19:28 UTC · re: Birger Skogeng Pedersen · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

On 15/10/19 12:51PM, Birger Skogeng Pedersen wrote:
> Hi Pratyush,
> 
> Thanks for reviewing. How does this work, do I send a re-roll of the patch(es)?
Yes, please do.

I mentioned this earlier, and I'll mention this again: I'm not sure whether this feature would be a good thing for the larger population. So this _might_ not end up being accepted depending on how people react to the proposal. I thought I'd let you know to avoid any nasty surprises later.

-- 
Regards,
Pratyush Yadav
Birger Skogeng Pedersen· Oct 17, 2019, 05:08 UTC · re: Pratyush Yadav · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

Hi Pratyush,
On Wed, Oct 16, 2019 at 9:28 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
Show 5 quoted lines
> I mentioned this earlier, and I'll mention this again: I'm not sure
> whether this feature would be a good thing for the larger population. So
> this _might_ not end up being accepted depending on how people react to
> the proposal. I thought I'd let you know to avoid any nasty surprises
> later.

Could you please elaborate on why you think the feature might be undesired? Why would users not want a staged file to be selected automatically?

FWIW I've also got 2 comments on this in GH[1].
[1] https://github.com/git-for-windows/git/issues/2341

Best regards, Birger

Johannes Sixt· Oct 17, 2019, 05:33 UTC · re: Birger Skogeng Pedersen · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

Am 17.10.19 um 07:08 schrieb Birger Skogeng Pedersen:
Show 12 quoted lines
> Hi Pratyush,
> 
> On Wed, Oct 16, 2019 at 9:28 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
>> I mentioned this earlier, and I'll mention this again: I'm not sure
>> whether this feature would be a good thing for the larger population. So
>> this _might_ not end up being accepted depending on how people react to
>> the proposal. I thought I'd let you know to avoid any nasty surprises
>> later.
> 
> Could you please elaborate on why you think the feature might be
> undesired? Why would users not want a staged file to be selected
> automatically?

FWIW, I would prefer to experiment with the feature for a few weeks before it (or a configuration that enables it by default) is baked in.

-- Hannes
Birger Skogeng Pedersen· Oct 17, 2019, 06:54 UTC · re: Johannes Sixt · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

Hi Johannes,
On Thu, Oct 17, 2019 at 7:33 AM Johannes Sixt <j6t@kdbg.org> wrote:
> FWIW, I would prefer to experiment with the feature for a few weeks
> before it (or a configuration that enables it by default) is baked in.

Yes please do. Obviously I'm glad someone other than me will be actually testing it. (As I mentioned earlier) I'm all for it when it comes to using a config setting, rather than having this as default behaviour. I propose "gui.autoFocusStaged" as a variable name for the setting. I'll be doing a re-roll once I get some more spare time.

Birger
Pratyush Yadav· Oct 17, 2019, 18:28 UTC · re: Birger Skogeng Pedersen · lore

Re: [PATCH 1/2] git-gui: implement proc select_path_in_widget

On 17/10/19 07:08AM, Birger Skogeng Pedersen wrote:
Show 12 quoted lines
> Hi Pratyush,
> 
> On Wed, Oct 16, 2019 at 9:28 PM Pratyush Yadav <me@yadavpratyush.com> wrote:
> > I mentioned this earlier, and I'll mention this again: I'm not sure
> > whether this feature would be a good thing for the larger population. So
> > this _might_ not end up being accepted depending on how people react to
> > the proposal. I thought I'd let you know to avoid any nasty surprises
> > later.
> 
> Could you please elaborate on why you think the feature might be
> undesired? Why would users not want a staged file to be selected
> automatically?

I have similar arguments to what Philip said in the GitHub link you sent. I think software should do what the user tells it to do, and should not try to "second guess" them. This second guessing can get annoying.

So if I select the commit message buffer, I told the software to select it by clicking it. I did not tell it to switch my diff view. This switch of view can prove to be disorienting/confusing to a user because they did not select any diff. They selected a text box.

At the very least, I think we should have a config option, and it should 
be turned off by default. That said, I'd certainly like to hear what 
other people think on this topic.
 
> FWIW I've also got 2 comments on this in GH[1].
> 
> [1] https://github.com/git-for-windows/git/issues/2341
-- 
Regards,
Pratyush Yadav

← back to recent threads