* [PATCH 1/2] git-gui: fix deleting item from all_remotes variable
@ 2011-02-12 16:43 Heiko Voigt
2011-02-13 13:20 ` Pat Thoyts
0 siblings, 1 reply; 17+ messages in thread
From: Heiko Voigt @ 2011-02-12 16:43 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Jens Lehmann
lsearch and lreplace both take the variable content as argument and not
just their name.
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable
2011-02-12 16:43 [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
@ 2011-02-13 13:20 ` Pat Thoyts
2011-02-13 13:47 ` Heiko Voigt
2011-02-13 14:05 ` Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
0 siblings, 2 replies; 17+ messages in thread
From: Pat Thoyts @ 2011-02-13 13:20 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Pat Thoyts, git, Jens Lehmann
On 12 February 2011 16:43, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> lsearch and lreplace both take the variable content as argument and not
> just their name.
>
> Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable
2011-02-13 13:20 ` Pat Thoyts
@ 2011-02-13 13:47 ` Heiko Voigt
2011-02-13 13:50 ` [PATCH 1/2] git-gui: refactor remote submenu creation into subroutine Heiko Voigt
2011-02-13 13:57 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Heiko Voigt
2011-02-13 14:05 ` Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
1 sibling, 2 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-13 13:47 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, git, Jens Lehmann
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] git-gui: refactor remote submenu creation into subroutine
2011-02-13 13:47 ` Heiko Voigt
@ 2011-02-13 13:50 ` Heiko Voigt
2011-02-13 13:57 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Heiko Voigt
1 sibling, 0 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-13 13:50 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, git, Jens Lehmann
Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes
2011-02-13 13:47 ` Heiko Voigt
2011-02-13 13:50 ` [PATCH 1/2] git-gui: refactor remote submenu creation into subroutine Heiko Voigt
@ 2011-02-13 13:57 ` Heiko Voigt
2011-02-14 13:03 ` [PATCH] git-gui: Include version check and test for tearoff menu entry Pat Thoyts
2011-02-22 18:36 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Jens Lehmann
1 sibling, 2 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-13 13:57 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, git, Jens Lehmann
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 <heiko.voigt@mahr.de>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable
2011-02-13 13:20 ` Pat Thoyts
2011-02-13 13:47 ` Heiko Voigt
@ 2011-02-13 14:05 ` Heiko Voigt
2011-02-13 14:15 ` Heiko Voigt
1 sibling, 1 reply; 17+ messages in thread
From: Heiko Voigt @ 2011-02-13 14:05 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, git, Jens Lehmann
Hi Pat,
On Sun, Feb 13, 2011 at 01:20:14PM +0000, Pat Thoyts wrote:
> On 12 February 2011 16:43, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > lsearch and lreplace both take the variable content as argument and not
> > just their name.
> >
> > Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> > ---
> > 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable
2011-02-13 14:05 ` Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
@ 2011-02-13 14:15 ` Heiko Voigt
0 siblings, 0 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-13 14:15 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, git, Jens Lehmann
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] git-gui: Include version check and test for tearoff menu entry
2011-02-13 13:57 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Heiko Voigt
@ 2011-02-14 13:03 ` Pat Thoyts
2011-02-14 21:31 ` Heiko Voigt
2011-02-22 18:36 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Jens Lehmann
1 sibling, 1 reply; 17+ messages in thread
From: Pat Thoyts @ 2011-02-14 13:03 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Pat Thoyts, Pat Thoyts, git, Jens Lehmann
The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding
the menu items.
Sometimes people use tearoff menus and these offset the entry indices by one.
Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
---
Heiko Voigt <hvoigt@hvoigt.net> writes:
>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
The posted patch seems fine except that an error is reported if tearoff
menus are present. So this patch accommodates tearoff's. I looked up
when the --all option was added (1.6.6) and skip adding the menu entry
if we have an older version.
Seems to do the right thing.
lib/remote.tcl | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/lib/remote.tcl b/lib/remote.tcl
index 817ca1b..b88f6e5 100644
--- a/lib/remote.tcl
+++ b/lib/remote.tcl
@@ -233,6 +233,8 @@ proc make_sure_remote_submenues_exist {remote_m} {
proc update_all_remotes_menu_entry {} {
global all_remotes
+ if {[git-version < 1.6.6]} { return }
+
set have_remote 0
foreach r $all_remotes {
set have_remote 1
@@ -243,27 +245,29 @@ proc update_all_remotes_menu_entry {} {
set prune_m $remote_m.prune
if {$have_remote} {
make_sure_remote_submenues_exist $remote_m
- if {[$fetch_m entrycget 0 -label] ne "All"} {
+ set index [expr {[$fetch_m type 0] eq "tearoff" ? 1 : 0}]
+ if {[$fetch_m entrycget $index -label] ne "All"} {
- $fetch_m insert 0 separator
- $fetch_m insert 0 command \
+ $fetch_m insert $index separator
+ $fetch_m insert $index command \
-label "All" \
-command fetch_from_all
- $prune_m insert 0 separator
- $prune_m insert 0 command \
+ $prune_m insert $index separator
+ $prune_m insert $index 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"} {
- delete_from_menu $fetch_m 0
- delete_from_menu $fetch_m 0
+ delete_from_menu $fetch_m $index
+ delete_from_menu $fetch_m $index
- delete_from_menu $prune_m 0
- delete_from_menu $prune_m 0
+ delete_from_menu $prune_m $index
+ delete_from_menu $prune_m $index
}
}
}
--
1.7.4.47.gb308bf
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] git-gui: Include version check and test for tearoff menu entry
2011-02-14 13:03 ` [PATCH] git-gui: Include version check and test for tearoff menu entry Pat Thoyts
@ 2011-02-14 21:31 ` Heiko Voigt
2011-02-15 0:31 ` Pat Thoyts
0 siblings, 1 reply; 17+ messages in thread
From: Heiko Voigt @ 2011-02-14 21:31 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, Pat Thoyts, git, Jens Lehmann
Hi,
On Mon, Feb 14, 2011 at 01:03:24PM +0000, Pat Thoyts wrote:
> The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding
> the menu items.
> Sometimes people use tearoff menus and these offset the entry indices by one.
>
> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
> ---
>
> Heiko Voigt <hvoigt@hvoigt.net> writes:
> >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
>
> The posted patch seems fine except that an error is reported if tearoff
> menus are present. So this patch accommodates tearoff's. I looked up
> when the --all option was added (1.6.6) and skip adding the menu entry
> if we have an older version.
>
> Seems to do the right thing.
Works and looks good to me as well. Did not know about tearoff menues
how do you get those?
Cheers Heiko
P.S.: I discovered a whitespace issue in line 258 which came from my patch.
Could you correct that?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] git-gui: Include version check and test for tearoff menu entry
2011-02-14 21:31 ` Heiko Voigt
@ 2011-02-15 0:31 ` Pat Thoyts
2011-02-17 20:06 ` Heiko Voigt
0 siblings, 1 reply; 17+ messages in thread
From: Pat Thoyts @ 2011-02-15 0:31 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Pat Thoyts, Pat Thoyts, git, Jens Lehmann
Heiko Voigt <hvoigt@hvoigt.net> writes:
>Hi,
>
>On Mon, Feb 14, 2011 at 01:03:24PM +0000, Pat Thoyts wrote:
>> The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding
>> the menu items.
>> Sometimes people use tearoff menus and these offset the entry indices by one.
>>
>> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
>> ---
>>
>> Heiko Voigt <hvoigt@hvoigt.net> writes:
>> >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
>>
>> The posted patch seems fine except that an error is reported if tearoff
>> menus are present. So this patch accommodates tearoff's. I looked up
>> when the --all option was added (1.6.6) and skip adding the menu entry
>> if we have an older version.
>>
>> Seems to do the right thing.
>
>Works and looks good to me as well. Did not know about tearoff menues
>how do you get those?
>
>Cheers Heiko
>
>P.S.: I discovered a whitespace issue in line 258 which came from my patch.
>Could you correct that?
Sure - squashed in.
The tearoff's appear by default on unix but are disabled on windows as
they are not normal gui features on that platform. Search for
*Menu.tearOff 0 in git-gui.sh. Unix users can disable these using the
.Xresources file adding *Menu.tearOff: 0
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: Re: [PATCH] git-gui: Include version check and test for tearoff menu entry
2011-02-15 0:31 ` Pat Thoyts
@ 2011-02-17 20:06 ` Heiko Voigt
0 siblings, 0 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-17 20:06 UTC (permalink / raw)
To: Pat Thoyts; +Cc: Pat Thoyts, Pat Thoyts, git, Jens Lehmann
On Tue, Feb 15, 2011 at 12:31:39AM +0000, Pat Thoyts wrote:
> The tearoff's appear by default on unix but are disabled on windows as
> they are not normal gui features on that platform. Search for
> *Menu.tearOff 0 in git-gui.sh. Unix users can disable these using the
> .Xresources file adding *Menu.tearOff: 0
Thanks for the enlightenment. They are visible on my linux box. I
probably did never see them because I did not know they were there.
Cheers Heiko
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes
2011-02-13 13:57 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Heiko Voigt
2011-02-14 13:03 ` [PATCH] git-gui: Include version check and test for tearoff menu entry Pat Thoyts
@ 2011-02-22 18:36 ` Jens Lehmann
2011-02-22 19:28 ` [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry Heiko Voigt
2011-02-22 19:30 ` [PATCH 2/2] git-gui: fetch/prune all entry appears last Heiko Voigt
1 sibling, 2 replies; 17+ messages in thread
From: Jens Lehmann @ 2011-02-22 18:36 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Pat Thoyts, Pat Thoyts, git
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?
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry
2011-02-22 18:36 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Jens Lehmann
@ 2011-02-22 19:28 ` Heiko Voigt
2011-02-24 0:02 ` Pat Thoyts
2011-02-22 19:30 ` [PATCH 2/2] git-gui: fetch/prune all entry appears last Heiko Voigt
1 sibling, 1 reply; 17+ messages in thread
From: Heiko Voigt @ 2011-02-22 19:28 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Pat Thoyts, Pat Thoyts, git
In case there is only one remote a fetch/prune all entry
is redundant.
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] git-gui: fetch/prune all entry appears last
2011-02-22 18:36 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Jens Lehmann
2011-02-22 19:28 ` [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry Heiko Voigt
@ 2011-02-22 19:30 ` Heiko Voigt
2011-02-23 19:19 ` Jens Lehmann
2011-02-24 0:09 ` Pat Thoyts
1 sibling, 2 replies; 17+ messages in thread
From: Heiko Voigt @ 2011-02-22 19:30 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Pat Thoyts, Pat Thoyts, git
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 <hvoigt@hvoigt.net>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] git-gui: fetch/prune all entry appears last
2011-02-22 19:30 ` [PATCH 2/2] git-gui: fetch/prune all entry appears last Heiko Voigt
@ 2011-02-23 19:19 ` Jens Lehmann
2011-02-24 0:09 ` Pat Thoyts
1 sibling, 0 replies; 17+ messages in thread
From: Jens Lehmann @ 2011-02-23 19:19 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Pat Thoyts, Pat Thoyts, git
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 <hvoigt@hvoigt.net>
I tested both patches under Linux, looks great now.
Tested-by: Jens Lehmann <Jens.Lehmann@web.de>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry
2011-02-22 19:28 ` [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry Heiko Voigt
@ 2011-02-24 0:02 ` Pat Thoyts
0 siblings, 0 replies; 17+ messages in thread
From: Pat Thoyts @ 2011-02-24 0:02 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Jens Lehmann, git
Heiko Voigt <hvoigt@hvoigt.net> writes:
>In case there is only one remote a fetch/prune all entry
>is redundant.
>
>Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
>---
>
>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"} {
This is fine - applied and checked it on Windows.
I'll add a Suggested-by from Jens as this was a response to his request.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] git-gui: fetch/prune all entry appears last
2011-02-22 19:30 ` [PATCH 2/2] git-gui: fetch/prune all entry appears last Heiko Voigt
2011-02-23 19:19 ` Jens Lehmann
@ 2011-02-24 0:09 ` Pat Thoyts
1 sibling, 0 replies; 17+ messages in thread
From: Pat Thoyts @ 2011-02-24 0:09 UTC (permalink / raw)
To: Heiko Voigt; +Cc: Jens Lehmann, git
Heiko Voigt <hvoigt@hvoigt.net> writes:
>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 <hvoigt@hvoigt.net>
>---
>
>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
> }
> }
> }
This is fine as well. Tested it on windows. Applied to master.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-02-24 0:09 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-12 16:43 [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
2011-02-13 13:20 ` Pat Thoyts
2011-02-13 13:47 ` Heiko Voigt
2011-02-13 13:50 ` [PATCH 1/2] git-gui: refactor remote submenu creation into subroutine Heiko Voigt
2011-02-13 13:57 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Heiko Voigt
2011-02-14 13:03 ` [PATCH] git-gui: Include version check and test for tearoff menu entry Pat Thoyts
2011-02-14 21:31 ` Heiko Voigt
2011-02-15 0:31 ` Pat Thoyts
2011-02-17 20:06 ` Heiko Voigt
2011-02-22 18:36 ` [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes Jens Lehmann
2011-02-22 19:28 ` [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry Heiko Voigt
2011-02-24 0:02 ` Pat Thoyts
2011-02-22 19:30 ` [PATCH 2/2] git-gui: fetch/prune all entry appears last Heiko Voigt
2011-02-23 19:19 ` Jens Lehmann
2011-02-24 0:09 ` Pat Thoyts
2011-02-13 14:05 ` Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable Heiko Voigt
2011-02-13 14:15 ` Heiko Voigt
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).