[PATCH 1/2] git-gui: fix deleting item from all_remotes variable

Subsystems: library code, the rest

STALE3733d

11 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 1/2] git-gui: fix deleting item from all_remotes variable

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

lsearch and lreplace both take the variable content as argument and not
just their name.

Signed-off-by: Heiko Voigt <redacted>
---
 lib/remote.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index b92b429..1383e97 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -264,8 +264,8 @@ proc remove_remote {name} {
 		unset repo_config(remote.$name.push)
 	}
 
-	set i [lsearch -exact all_remotes $name]
-	lreplace all_remotes $i $i
+	set i [lsearch -exact $all_remotes $name]
+	set all_remotes [lreplace $all_remotes $i $i]
 
 	set remote_m .mbar.remote
 	delete_from_menu $remote_m.fetch $name
-- 
1.7.4.34.gd2cb1

Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable

From: Pat Thoyts <hidden>
Date: 2016-06-15 22:50:33

On 12 February 2011 16:43, Heiko Voigt [off-list ref] wrote:
quoted hunk
lsearch and lreplace both take the variable content as argument and not
just their name.

Signed-off-by: Heiko Voigt <redacted>
---
 lib/remote.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index b92b429..1383e97 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -264,8 +264,8 @@ proc remove_remote {name} {
               unset repo_config(remote.$name.push)
       }

-       set i [lsearch -exact all_remotes $name]
-       lreplace all_remotes $i $i
+       set i [lsearch -exact $all_remotes $name]
+       set all_remotes [lreplace $all_remotes $i $i]

       set remote_m .mbar.remote
       delete_from_menu $remote_m.fetch $name
--
1.7.4.34.gd2cb1
This fix is good and clearly resolves a bug in the tcl code --
however, what does it actually fix in the application? It looks like
removing a remote works anyway even though this variable is not being
updated.
Pat Thoyts

Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

Hi Pat,

On Sun, Feb 13, 2011 at 01:20:14PM +0000, Pat Thoyts wrote:
This fix is good and clearly resolves a bug in the tcl code --
however, what does it actually fix in the application? It looks like
removing a remote works anyway even though this variable is not being
updated.
I do not know the other implications but I needed this fix for a patch I
wrote. I did not send it because it is quite long and I wanted to wait
until my other patches are ok so you do not have to review too much.
But since you asked I will reply with the two patches to this email.

Cheers Heiko

[PATCH 1/2] git-gui: refactor remote submenu creation into subroutine

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

Signed-off-by: Heiko Voigt <redacted>
---
 lib/remote.tcl |   40 ++++++++++++++++++++++++----------------
 1 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index b92b429..d9eab78 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -157,22 +157,7 @@ proc add_fetch_entry {r} {
 	}
 
 	if {$enable} {
-		if {![winfo exists $fetch_m]} {
-			menu $remove_m
-			$remote_m insert 0 cascade \
-				-label [mc "Remove Remote"] \
-				-menu $remove_m
-
-			menu $prune_m
-			$remote_m insert 0 cascade \
-				-label [mc "Prune from"] \
-				-menu $prune_m
-
-			menu $fetch_m
-			$remote_m insert 0 cascade \
-				-label [mc "Fetch from"] \
-				-menu $fetch_m
-		}
+		make_sure_remote_submenues_exist $remote_m
 
 		$fetch_m add command \
 			-label $r \
@@ -222,6 +207,29 @@ proc add_push_entry {r} {
 	}
 }
 
+proc make_sure_remote_submenues_exist {remote_m} {
+	set fetch_m $remote_m.fetch
+	set prune_m $remote_m.prune
+	set remove_m $remote_m.remove
+
+	if {![winfo exists $fetch_m]} {
+		menu $remove_m
+		$remote_m insert 0 cascade \
+			-label [mc "Remove Remote"] \
+			-menu $remove_m
+
+		menu $prune_m
+		$remote_m insert 0 cascade \
+			-label [mc "Prune from"] \
+			-menu $prune_m
+
+		menu $fetch_m
+		$remote_m insert 0 cascade \
+			-label [mc "Fetch from"] \
+			-menu $fetch_m
+	}
+}
+
 proc populate_remotes_menu {} {
 	global all_remotes
 
-- 
1.7.4.rc3.4.g155c4

[RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

The commandline fetch already has this option for some time.  Since this
was not available at the time git gui was written lets implement it now.

Signed-off-by: Heiko Voigt <redacted>
---
It just came to my mind that I probably should implement a version check
of the commandline to ensure that this option is available. Thats why I
tagged only this patch with RFC.

Cheers Heiko

 lib/remote.tcl    |   45 +++++++++++++++++++++++++++++++++++++++++++++
 lib/transport.tcl |   29 +++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index d9eab78..7011681 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -230,6 +230,45 @@ proc make_sure_remote_submenues_exist {remote_m} {
 	}
 }
 
+proc update_all_remotes_menu_entry {} {
+	global all_remotes
+
+	set have_remote 0
+	foreach r $all_remotes {
+		set have_remote 1
+	}
+
+	set remote_m .mbar.remote
+	set fetch_m $remote_m.fetch
+	set prune_m $remote_m.prune
+	if {$have_remote} {
+		make_sure_remote_submenues_exist $remote_m
+		if {[$fetch_m entrycget 0 -label] ne "All"} {
+
+			$fetch_m insert 0 separator
+			$fetch_m insert 0 command \
+				-label "All" \
+				-command fetch_from_all
+
+			$prune_m insert 0 separator
+			$prune_m insert 0 command \
+	  			-label "All" \
+				-command prune_from_all
+		}
+	} else {
+		if {[winfo exists $fetch_m]} {
+			if {[$fetch_m type end] eq "separator"} {
+
+				delete_from_menu $fetch_m 0
+				delete_from_menu $fetch_m 0
+
+				delete_from_menu $prune_m 0
+				delete_from_menu $prune_m 0
+			}
+		}
+	}
+}
+
 proc populate_remotes_menu {} {
 	global all_remotes
 
@@ -237,6 +276,8 @@ proc populate_remotes_menu {} {
 		add_fetch_entry $r
 		add_push_entry $r
 	}
+
+	update_all_remotes_menu_entry
 }
 
 proc add_single_remote {name location} {
@@ -252,6 +293,8 @@ proc add_single_remote {name location} {
 
 	add_fetch_entry $name
 	add_push_entry $name
+
+	update_all_remotes_menu_entry
 }
 
 proc delete_from_menu {menu name} {
@@ -281,4 +324,6 @@ proc remove_remote {name} {
 	delete_from_menu $remote_m.remove $name
 	# Not all remotes are in the push menu
 	catch { delete_from_menu $remote_m.push $name }
+
+	update_all_remotes_menu_entry
 }
diff --git a/lib/transport.tcl b/lib/transport.tcl
index 3067058..7fad9b7 100644
--- a/lib/transport.tcl
+++ b/lib/transport.tcl
@@ -20,6 +20,35 @@ proc prune_from {remote} {
 	console::exec $w [list git remote prune $remote]
 }
 
+proc fetch_from_all {} {
+	set w [console::new \
+		[mc "fetch all remotes"] \
+		[mc "Fetching new changes from all remotes"]]
+
+	set cmd [list git fetch --all]
+	if {[is_config_true gui.pruneduringfetch]} {
+		lappend cmd --prune
+	}
+
+	console::exec $w $cmd
+}
+
+proc prune_from_all {} {
+	global all_remotes
+
+	set w [console::new \
+		[mc "remote prune all remotes"] \
+		[mc "Pruning tracking branches deleted from all remotes"]]
+
+	set cmd [list git remote prune]
+
+	foreach r $all_remotes {
+		lappend cmd $r
+	}
+
+	console::exec $w $cmd
+}
+
 proc push_to {remote} {
 	set w [console::new \
 		[mc "push %s" $remote] \
-- 
1.7.4.rc3.4.g155c4

Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

Hi Pat,

On Sun, Feb 13, 2011 at 01:20:14PM +0000, Pat Thoyts wrote:
On 12 February 2011 16:43, Heiko Voigt [off-list ref] wrote:
quoted
lsearch and lreplace both take the variable content as argument and not
just their name.

Signed-off-by: Heiko Voigt <redacted>
---
 lib/remote.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index b92b429..1383e97 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -264,8 +264,8 @@ proc remove_remote {name} {
               unset repo_config(remote.$name.push)
       }

-       set i [lsearch -exact all_remotes $name]
-       lreplace all_remotes $i $i
+       set i [lsearch -exact $all_remotes $name]
+       set all_remotes [lreplace $all_remotes $i $i]
If you were going to please wait with applying it. I just found another
location where this variable is changed in a wrong manner. I will update
the patch accordingly.

Cheers Heiko

Re: Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:33

Hi Pat,

On Sun, Feb 13, 2011 at 03:05:23PM +0100, Heiko Voigt wrote:
If you were going to please wait with applying it. I just found another
location where this variable is changed in a wrong manner. I will update
the patch accordingly.
Please forget this comment. I mistakenly found an lappend call with the
same usage pattern, but for lappend this is obviously correct.

Cheers Heiko

Re: [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:50:38

Am 13.02.2011 14:57, schrieb Heiko Voigt:
The commandline fetch already has this option for some time.  Since this
was not available at the time git gui was written lets implement it now.
I really like this feature, I wanted to have that for quite some time!

After testing it, I noticed two minor things:

1) It would be nice if the new menu entry would only appear when there
   is more than one remote to fetch from.

2) I would rather like to see it at the *end* of the submenu, not at the
   beginning. Being used to always click on the first menu entry only
   to learn that the remote that used to be there got with something
   else is kind of surprising ;-)

What do others think?

[PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:38

In case there is only one remote a fetch/prune all entry
is redundant.

Signed-off-by: Heiko Voigt <redacted>
---

On Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:
1) It would be nice if the new menu entry would only appear when there
   is more than one remote to fetch from.
How about this? Disclaimer: Only superficially tested on OSX.

 lib/remote.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index 42d2061..18d3d06 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -237,13 +237,13 @@ proc update_all_remotes_menu_entry {} {
 
 	set have_remote 0
 	foreach r $all_remotes {
-		set have_remote 1
+		incr have_remote
 	}
 
 	set remote_m .mbar.remote
 	set fetch_m $remote_m.fetch
 	set prune_m $remote_m.prune
-	if {$have_remote} {
+	if {$have_remote > 1} {
 		make_sure_remote_submenues_exist $remote_m
 		set index [expr {[$fetch_m type 0] eq "tearoff" ? 1 : 0}]
 		if {[$fetch_m entrycget $index -label] ne "All"} {
-- 
1.7.4.1.30.gd0a3

[PATCH 2/2] git-gui: fetch/prune all entry appears last

From: Heiko Voigt <hidden>
Date: 2016-06-15 22:50:38

The user might have got used to the order the remotes appeared previously.
Lets add the all entry last so the all entry does not confuse previous
users.

Signed-off-by: Heiko Voigt <redacted>
---

On Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:
2) I would rather like to see it at the *end* of the submenu, not at the
   beginning. Being used to always click on the first menu entry only
   to learn that the remote that used to be there got with something
   else is kind of surprising ;-)
And this? Disclaimer: Also only superficially tested on OSX.

 lib/remote.tcl |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index 18d3d06..5e4e7f4 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -245,29 +245,27 @@ proc update_all_remotes_menu_entry {} {
 	set prune_m $remote_m.prune
 	if {$have_remote > 1} {
 		make_sure_remote_submenues_exist $remote_m
-		set index [expr {[$fetch_m type 0] eq "tearoff" ? 1 : 0}]
-		if {[$fetch_m entrycget $index -label] ne "All"} {
+		if {[$fetch_m entrycget end -label] ne "All"} {
 
-			$fetch_m insert $index separator
-			$fetch_m insert $index command \
+			$fetch_m insert end separator
+			$fetch_m insert end command \
 				-label "All" \
 				-command fetch_from_all
 
-			$prune_m insert $index separator
-			$prune_m insert $index command \
+			$prune_m insert end separator
+			$prune_m insert end command \
 				-label "All" \
 				-command prune_from_all
 		}
 	} else {
 		if {[winfo exists $fetch_m]} {
-			set index [expr {[$fetch_m type 0] eq "tearoff" ? 1 : 0}]
-			if {[$fetch_m type end] eq "separator"} {
+			if {[$fetch_m entrycget end -label] eq "All"} {
 
-				delete_from_menu $fetch_m $index
-				delete_from_menu $fetch_m $index
+				delete_from_menu $fetch_m end
+				delete_from_menu $fetch_m end
 
-				delete_from_menu $prune_m $index
-				delete_from_menu $prune_m $index
+				delete_from_menu $prune_m end
+				delete_from_menu $prune_m end
 			}
 		}
 	}
-- 
1.7.4.1.30.gd0a3

Re: [PATCH 2/2] git-gui: fetch/prune all entry appears last

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:50:38

Am 22.02.2011 20:30, schrieb Heiko Voigt:
The user might have got used to the order the remotes appeared previously.
Lets add the all entry last so the all entry does not confuse previous
users.

Signed-off-by: Heiko Voigt <redacted>
I tested both patches under Linux, looks great now.

Tested-by: Jens Lehmann <redacted>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help