{"thread":{"id":"26482","subject":"[PATCH 1/2] git-gui: fix deleting item from all_remotes variable","startedAt":"2011-02-12T16:43:44Z","lastAt":"2011-02-24T00:09:28Z","messageCount":17,"participants":["Heiko Voigt","Pat Thoyts","Jens Lehmann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"160953","messageId":"20110212164344.GA19433@book.hvoigt.net","threadId":"26482","inReplyTo":null,"subject":"[PATCH 1/2] git-gui: fix deleting item from all_remotes variable","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-12T16:43:44Z","receivedAt":"2011-02-12T16:43:44Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"lsearch and lreplace both take the variable content as argument and not\njust their name.\n\nSigned-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n---\n lib/remote.tcl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex b92b429..1383e97 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -264,8 +264,8 @@ proc remove_remote {name} {\n \t\tunset repo_config(remote.$name.push)\n \t}\n \n-\tset i [lsearch -exact all_remotes $name]\n-\tlreplace all_remotes $i $i\n+\tset i [lsearch -exact $all_remotes $name]\n+\tset all_remotes [lreplace $all_remotes $i $i]\n \n \tset remote_m .mbar.remote\n \tdelete_from_menu $remote_m.fetch $name\n-- \n1.7.4.34.gd2cb1\n"},{"id":"161000","messageId":"AANLkTi=hY1XpBNfhNDfM8kwgnitQXN-97mM-dkhCpTac@mail.gmail.com","threadId":"26482","inReplyTo":"20110212164344.GA19433@book.hvoigt.net","subject":"Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable","fromName":"Pat Thoyts","fromEmail":"patthoyts@gmail.com","sentAt":"2011-02-13T13:20:14Z","receivedAt":"2011-02-13T13:20:14Z","isPatch":true,"sender":{"key":"patthoyts@gmail.com","avatar":"https://gravatar.com/avatar/bee887a777c790bd241f398217723fbe4b854428671db83db32216a28654cb25?d=mp&s=160"},"body":"On 12 February 2011 16:43, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> lsearch and lreplace both take the variable content as argument and not\n> just their name.\n>\n> Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n> ---\n>  lib/remote.tcl |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/lib/remote.tcl b/lib/remote.tcl\n> index b92b429..1383e97 100644\n> --- a/lib/remote.tcl\n> +++ b/lib/remote.tcl\n> @@ -264,8 +264,8 @@ proc remove_remote {name} {\n>                unset repo_config(remote.$name.push)\n>        }\n>\n> -       set i [lsearch -exact all_remotes $name]\n> -       lreplace all_remotes $i $i\n> +       set i [lsearch -exact $all_remotes $name]\n> +       set all_remotes [lreplace $all_remotes $i $i]\n>\n>        set remote_m .mbar.remote\n>        delete_from_menu $remote_m.fetch $name\n> --\n> 1.7.4.34.gd2cb1\n>\n>\nThis fix is good and clearly resolves a bug in the tcl code --\nhowever, what does it actually fix in the application? It looks like\nremoving a remote works anyway even though this variable is not being\nupdated.\nPat Thoyts\n"},{"id":"161002","messageId":"20110213134753.GC31986@book.hvoigt.net","threadId":"26482","inReplyTo":"AANLkTi=hY1XpBNfhNDfM8kwgnitQXN-97mM-dkhCpTac@mail.gmail.com","subject":"Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-13T13:47:53Z","receivedAt":"2011-02-13T13:47:53Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Pat,\n\nOn Sun, Feb 13, 2011 at 01:20:14PM +0000, Pat Thoyts wrote:\n> This fix is good and clearly resolves a bug in the tcl code --\n> however, what does it actually fix in the application? It looks like\n> removing a remote works anyway even though this variable is not being\n> updated.\n\nI do not know the other implications but I needed this fix for a patch I\nwrote. I did not send it because it is quite long and I wanted to wait\nuntil my other patches are ok so you do not have to review too much.\nBut since you asked I will reply with the two patches to this email.\n\nCheers Heiko\n"},{"id":"161003","messageId":"20110213135038.GD31986@book.hvoigt.net","threadId":"26482","inReplyTo":"20110213134753.GC31986@book.hvoigt.net","subject":"[PATCH 1/2] git-gui: refactor remote submenu creation into subroutine","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-13T13:50:38Z","receivedAt":"2011-02-13T13:50:38Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n---\n lib/remote.tcl |   40 ++++++++++++++++++++++++----------------\n 1 files changed, 24 insertions(+), 16 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex b92b429..d9eab78 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -157,22 +157,7 @@ proc add_fetch_entry {r} {\n \t}\n \n \tif {$enable} {\n-\t\tif {![winfo exists $fetch_m]} {\n-\t\t\tmenu $remove_m\n-\t\t\t$remote_m insert 0 cascade \\\n-\t\t\t\t-label [mc \"Remove Remote\"] \\\n-\t\t\t\t-menu $remove_m\n-\n-\t\t\tmenu $prune_m\n-\t\t\t$remote_m insert 0 cascade \\\n-\t\t\t\t-label [mc \"Prune from\"] \\\n-\t\t\t\t-menu $prune_m\n-\n-\t\t\tmenu $fetch_m\n-\t\t\t$remote_m insert 0 cascade \\\n-\t\t\t\t-label [mc \"Fetch from\"] \\\n-\t\t\t\t-menu $fetch_m\n-\t\t}\n+\t\tmake_sure_remote_submenues_exist $remote_m\n \n \t\t$fetch_m add command \\\n \t\t\t-label $r \\\n@@ -222,6 +207,29 @@ proc add_push_entry {r} {\n \t}\n }\n \n+proc make_sure_remote_submenues_exist {remote_m} {\n+\tset fetch_m $remote_m.fetch\n+\tset prune_m $remote_m.prune\n+\tset remove_m $remote_m.remove\n+\n+\tif {![winfo exists $fetch_m]} {\n+\t\tmenu $remove_m\n+\t\t$remote_m insert 0 cascade \\\n+\t\t\t-label [mc \"Remove Remote\"] \\\n+\t\t\t-menu $remove_m\n+\n+\t\tmenu $prune_m\n+\t\t$remote_m insert 0 cascade \\\n+\t\t\t-label [mc \"Prune from\"] \\\n+\t\t\t-menu $prune_m\n+\n+\t\tmenu $fetch_m\n+\t\t$remote_m insert 0 cascade \\\n+\t\t\t-label [mc \"Fetch from\"] \\\n+\t\t\t-menu $fetch_m\n+\t}\n+}\n+\n proc populate_remotes_menu {} {\n \tglobal all_remotes\n \n-- \n1.7.4.rc3.4.g155c4\n"},{"id":"161004","messageId":"20110213135714.GE31986@book.hvoigt.net","threadId":"26482","inReplyTo":"20110213134753.GC31986@book.hvoigt.net","subject":"[RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-13T13:57:15Z","receivedAt":"2011-02-13T13:57:15Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The commandline fetch already has this option for some time.  Since this\nwas not available at the time git gui was written lets implement it now.\n\nSigned-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n---\nIt just came to my mind that I probably should implement a version check\nof the commandline to ensure that this option is available. Thats why I\ntagged only this patch with RFC.\n\nCheers Heiko\n\n lib/remote.tcl    |   45 +++++++++++++++++++++++++++++++++++++++++++++\n lib/transport.tcl |   29 +++++++++++++++++++++++++++++\n 2 files changed, 74 insertions(+), 0 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex d9eab78..7011681 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -230,6 +230,45 @@ proc make_sure_remote_submenues_exist {remote_m} {\n \t}\n }\n \n+proc update_all_remotes_menu_entry {} {\n+\tglobal all_remotes\n+\n+\tset have_remote 0\n+\tforeach r $all_remotes {\n+\t\tset have_remote 1\n+\t}\n+\n+\tset remote_m .mbar.remote\n+\tset fetch_m $remote_m.fetch\n+\tset prune_m $remote_m.prune\n+\tif {$have_remote} {\n+\t\tmake_sure_remote_submenues_exist $remote_m\n+\t\tif {[$fetch_m entrycget 0 -label] ne \"All\"} {\n+\n+\t\t\t$fetch_m insert 0 separator\n+\t\t\t$fetch_m insert 0 command \\\n+\t\t\t\t-label \"All\" \\\n+\t\t\t\t-command fetch_from_all\n+\n+\t\t\t$prune_m insert 0 separator\n+\t\t\t$prune_m insert 0 command \\\n+\t  \t\t\t-label \"All\" \\\n+\t\t\t\t-command prune_from_all\n+\t\t}\n+\t} else {\n+\t\tif {[winfo exists $fetch_m]} {\n+\t\t\tif {[$fetch_m type end] eq \"separator\"} {\n+\n+\t\t\t\tdelete_from_menu $fetch_m 0\n+\t\t\t\tdelete_from_menu $fetch_m 0\n+\n+\t\t\t\tdelete_from_menu $prune_m 0\n+\t\t\t\tdelete_from_menu $prune_m 0\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n proc populate_remotes_menu {} {\n \tglobal all_remotes\n \n@@ -237,6 +276,8 @@ proc populate_remotes_menu {} {\n \t\tadd_fetch_entry $r\n \t\tadd_push_entry $r\n \t}\n+\n+\tupdate_all_remotes_menu_entry\n }\n \n proc add_single_remote {name location} {\n@@ -252,6 +293,8 @@ proc add_single_remote {name location} {\n \n \tadd_fetch_entry $name\n \tadd_push_entry $name\n+\n+\tupdate_all_remotes_menu_entry\n }\n \n proc delete_from_menu {menu name} {\n@@ -281,4 +324,6 @@ proc remove_remote {name} {\n \tdelete_from_menu $remote_m.remove $name\n \t# Not all remotes are in the push menu\n \tcatch { delete_from_menu $remote_m.push $name }\n+\n+\tupdate_all_remotes_menu_entry\n }\ndiff --git a/lib/transport.tcl b/lib/transport.tcl\nindex 3067058..7fad9b7 100644\n--- a/lib/transport.tcl\n+++ b/lib/transport.tcl\n@@ -20,6 +20,35 @@ proc prune_from {remote} {\n \tconsole::exec $w [list git remote prune $remote]\n }\n \n+proc fetch_from_all {} {\n+\tset w [console::new \\\n+\t\t[mc \"fetch all remotes\"] \\\n+\t\t[mc \"Fetching new changes from all remotes\"]]\n+\n+\tset cmd [list git fetch --all]\n+\tif {[is_config_true gui.pruneduringfetch]} {\n+\t\tlappend cmd --prune\n+\t}\n+\n+\tconsole::exec $w $cmd\n+}\n+\n+proc prune_from_all {} {\n+\tglobal all_remotes\n+\n+\tset w [console::new \\\n+\t\t[mc \"remote prune all remotes\"] \\\n+\t\t[mc \"Pruning tracking branches deleted from all remotes\"]]\n+\n+\tset cmd [list git remote prune]\n+\n+\tforeach r $all_remotes {\n+\t\tlappend cmd $r\n+\t}\n+\n+\tconsole::exec $w $cmd\n+}\n+\n proc push_to {remote} {\n \tset w [console::new \\\n \t\t[mc \"push %s\" $remote] \\\n-- \n1.7.4.rc3.4.g155c4\n"},{"id":"161005","messageId":"20110213140523.GF31986@book.hvoigt.net","threadId":"26482","inReplyTo":"AANLkTi=hY1XpBNfhNDfM8kwgnitQXN-97mM-dkhCpTac@mail.gmail.com","subject":"Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-13T14:05:23Z","receivedAt":"2011-02-13T14:05:23Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Pat,\n\nOn Sun, Feb 13, 2011 at 01:20:14PM +0000, Pat Thoyts wrote:\n> On 12 February 2011 16:43, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n> > lsearch and lreplace both take the variable content as argument and not\n> > just their name.\n> >\n> > Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>\n> > ---\n> >  lib/remote.tcl |    4 ++--\n> >  1 files changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/lib/remote.tcl b/lib/remote.tcl\n> > index b92b429..1383e97 100644\n> > --- a/lib/remote.tcl\n> > +++ b/lib/remote.tcl\n> > @@ -264,8 +264,8 @@ proc remove_remote {name} {\n> >                unset repo_config(remote.$name.push)\n> >        }\n> >\n> > -       set i [lsearch -exact all_remotes $name]\n> > -       lreplace all_remotes $i $i\n> > +       set i [lsearch -exact $all_remotes $name]\n> > +       set all_remotes [lreplace $all_remotes $i $i]\n\nIf you were going to please wait with applying it. I just found another\nlocation where this variable is changed in a wrong manner. I will update\nthe patch accordingly.\n\nCheers Heiko\n"},{"id":"161006","messageId":"20110213141501.GG31986@book.hvoigt.net","threadId":"26482","inReplyTo":"20110213140523.GF31986@book.hvoigt.net","subject":"Re: Re: Re: [PATCH 1/2] git-gui: fix deleting item from all_remotes variable","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-13T14:15:02Z","receivedAt":"2011-02-13T14:15:02Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi Pat,\n\nOn Sun, Feb 13, 2011 at 03:05:23PM +0100, Heiko Voigt wrote:\n> If you were going to please wait with applying it. I just found another\n> location where this variable is changed in a wrong manner. I will update\n> the patch accordingly.\n\nPlease forget this comment. I mistakenly found an lappend call with the\nsame usage pattern, but for lappend this is obviously correct.\n\nCheers Heiko\n"},{"id":"161049","messageId":"878vxilndt.fsf_-_@fox.patthoyts.tk","threadId":"26482","inReplyTo":"20110213135714.GE31986@book.hvoigt.net","subject":"[PATCH] git-gui: Include version check and test for tearoff menu entry","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2011-02-14T13:03:24Z","receivedAt":"2011-02-14T13:03:24Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding\nthe menu items.\nSometimes people use tearoff menus and these offset the entry indices by one.\n\nSigned-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>\n---\n\nHeiko Voigt <hvoigt@hvoigt.net> writes:\n>It just came to my mind that I probably should implement a version check\n>of the commandline to ensure that this option is available. Thats why I\n>tagged only this patch with RFC.\n>\n>Cheers Heiko\n\nThe posted patch seems fine except that an error is reported if tearoff\nmenus are present. So this patch accommodates tearoff's. I looked up\nwhen the --all option was added (1.6.6) and skip adding the menu entry\nif we have an older version.\n\nSeems to do the right thing.\n\n lib/remote.tcl |   22 +++++++++++++---------\n 1 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex 817ca1b..b88f6e5 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -233,6 +233,8 @@ proc make_sure_remote_submenues_exist {remote_m} {\n proc update_all_remotes_menu_entry {} {\n \tglobal all_remotes\n \n+\tif {[git-version < 1.6.6]} { return }\n+\n \tset have_remote 0\n \tforeach r $all_remotes {\n \t\tset have_remote 1\n@@ -243,27 +245,29 @@ proc update_all_remotes_menu_entry {} {\n \tset prune_m $remote_m.prune\n \tif {$have_remote} {\n \t\tmake_sure_remote_submenues_exist $remote_m\n-\t\tif {[$fetch_m entrycget 0 -label] ne \"All\"} {\n+\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n+\t\tif {[$fetch_m entrycget $index -label] ne \"All\"} {\n \n-\t\t\t$fetch_m insert 0 separator\n-\t\t\t$fetch_m insert 0 command \\\n+\t\t\t$fetch_m insert $index separator\n+\t\t\t$fetch_m insert $index command \\\n \t\t\t\t-label \"All\" \\\n \t\t\t\t-command fetch_from_all\n \n-\t\t\t$prune_m insert 0 separator\n-\t\t\t$prune_m insert 0 command \\\n+\t\t\t$prune_m insert $index separator\n+\t\t\t$prune_m insert $index command \\\n \t  \t\t\t-label \"All\" \\\n \t\t\t\t-command prune_from_all\n \t\t}\n \t} else {\n \t\tif {[winfo exists $fetch_m]} {\n+\t\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n \t\t\tif {[$fetch_m type end] eq \"separator\"} {\n \n-\t\t\t\tdelete_from_menu $fetch_m 0\n-\t\t\t\tdelete_from_menu $fetch_m 0\n+\t\t\t\tdelete_from_menu $fetch_m $index\n+\t\t\t\tdelete_from_menu $fetch_m $index\n \n-\t\t\t\tdelete_from_menu $prune_m 0\n-\t\t\t\tdelete_from_menu $prune_m 0\n+\t\t\t\tdelete_from_menu $prune_m $index\n+\t\t\t\tdelete_from_menu $prune_m $index\n \t\t\t}\n \t\t}\n \t}\n-- \n1.7.4.47.gb308bf\n"},{"id":"161109","messageId":"20110214213148.GB50815@book.hvoigt.net","threadId":"26482","inReplyTo":"878vxilndt.fsf_-_@fox.patthoyts.tk","subject":"Re: [PATCH] git-gui: Include version check and test for tearoff menu entry","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-14T21:31:48Z","receivedAt":"2011-02-14T21:31:48Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Mon, Feb 14, 2011 at 01:03:24PM +0000, Pat Thoyts wrote:\n> The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding\n> the menu items.\n> Sometimes people use tearoff menus and these offset the entry indices by one.\n> \n> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>\n> ---\n> \n> Heiko Voigt <hvoigt@hvoigt.net> writes:\n> >It just came to my mind that I probably should implement a version check\n> >of the commandline to ensure that this option is available. Thats why I\n> >tagged only this patch with RFC.\n> >\n> >Cheers Heiko\n> \n> The posted patch seems fine except that an error is reported if tearoff\n> menus are present. So this patch accommodates tearoff's. I looked up\n> when the --all option was added (1.6.6) and skip adding the menu entry\n> if we have an older version.\n> \n> Seems to do the right thing.\n\nWorks and looks good to me as well. Did not know about tearoff menues\nhow do you get those?\n\nCheers Heiko\n\nP.S.: I discovered a whitespace issue in line 258 which came from my patch.\nCould you correct that?\n"},{"id":"161136","messageId":"8762smdtp0.fsf@fox.patthoyts.tk","threadId":"26482","inReplyTo":"20110214213148.GB50815@book.hvoigt.net","subject":"Re: [PATCH] git-gui: Include version check and test for tearoff menu entry","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2011-02-15T00:31:39Z","receivedAt":"2011-02-15T00:31:39Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Heiko Voigt <hvoigt@hvoigt.net> writes:\n\n>Hi,\n>\n>On Mon, Feb 14, 2011 at 01:03:24PM +0000, Pat Thoyts wrote:\n>> The --all option for git fetch was added in v1.6.6 so ensure we have a usable version before adding\n>> the menu items.\n>> Sometimes people use tearoff menus and these offset the entry indices by one.\n>> \n>> Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>\n>> ---\n>> \n>> Heiko Voigt <hvoigt@hvoigt.net> writes:\n>> >It just came to my mind that I probably should implement a version check\n>> >of the commandline to ensure that this option is available. Thats why I\n>> >tagged only this patch with RFC.\n>> >\n>> >Cheers Heiko\n>> \n>> The posted patch seems fine except that an error is reported if tearoff\n>> menus are present. So this patch accommodates tearoff's. I looked up\n>> when the --all option was added (1.6.6) and skip adding the menu entry\n>> if we have an older version.\n>> \n>> Seems to do the right thing.\n>\n>Works and looks good to me as well. Did not know about tearoff menues\n>how do you get those?\n>\n>Cheers Heiko\n>\n>P.S.: I discovered a whitespace issue in line 258 which came from my patch.\n>Could you correct that?\n\nSure - squashed in.\n\nThe tearoff's appear by default on unix but are disabled on windows as\nthey are not normal gui features on that platform. Search for\n*Menu.tearOff 0 in git-gui.sh. Unix users can disable these using the\n.Xresources file adding *Menu.tearOff: 0\n\n-- \nPat Thoyts                            http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD\n"},{"id":"161447","messageId":"20110217200633.GB93859@book.hvoigt.net","threadId":"26482","inReplyTo":"8762smdtp0.fsf@fox.patthoyts.tk","subject":"Re: Re: [PATCH] git-gui: Include version check and test for tearoff menu entry","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-17T20:06:33Z","receivedAt":"2011-02-17T20:06:33Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Tue, Feb 15, 2011 at 12:31:39AM +0000, Pat Thoyts wrote:\n> The tearoff's appear by default on unix but are disabled on windows as\n> they are not normal gui features on that platform. Search for\n> *Menu.tearOff 0 in git-gui.sh. Unix users can disable these using the\n> .Xresources file adding *Menu.tearOff: 0\n\nThanks for the enlightenment. They are visible on my linux box. I\nprobably did never see them because I did not know they were there.\n\nCheers Heiko\n"},{"id":"161901","messageId":"4D640227.9090206@web.de","threadId":"26482","inReplyTo":"20110213135714.GE31986@book.hvoigt.net","subject":"Re: [RFC PATCH 2/2] git-gui: teach fetch/prune menu to do it for all remotes","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-22T18:36:23Z","receivedAt":"2011-02-22T18:36:23Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 13.02.2011 14:57, schrieb Heiko Voigt:\n> The commandline fetch already has this option for some time.  Since this\n> was not available at the time git gui was written lets implement it now.\n\nI really like this feature, I wanted to have that for quite some time!\n\nAfter testing it, I noticed two minor things:\n\n1) It would be nice if the new menu entry would only appear when there\n   is more than one remote to fetch from.\n\n2) I would rather like to see it at the *end* of the submenu, not at the\n   beginning. Being used to always click on the first menu entry only\n   to learn that the remote that used to be there got with something\n   else is kind of surprising ;-)\n\nWhat do others think?\n"},{"id":"161908","messageId":"20110222192835.GA28519@book.hvoigt.net","threadId":"26482","inReplyTo":"4D640227.9090206@web.de","subject":"[PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-22T19:28:36Z","receivedAt":"2011-02-22T19:28:36Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"In case there is only one remote a fetch/prune all entry\nis redundant.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n\nOn Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:\n> 1) It would be nice if the new menu entry would only appear when there\n>    is more than one remote to fetch from.\n\nHow about this? Disclaimer: Only superficially tested on OSX.\n\n lib/remote.tcl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex 42d2061..18d3d06 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -237,13 +237,13 @@ proc update_all_remotes_menu_entry {} {\n \n \tset have_remote 0\n \tforeach r $all_remotes {\n-\t\tset have_remote 1\n+\t\tincr have_remote\n \t}\n \n \tset remote_m .mbar.remote\n \tset fetch_m $remote_m.fetch\n \tset prune_m $remote_m.prune\n-\tif {$have_remote} {\n+\tif {$have_remote > 1} {\n \t\tmake_sure_remote_submenues_exist $remote_m\n \t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n \t\tif {[$fetch_m entrycget $index -label] ne \"All\"} {\n-- \n1.7.4.1.30.gd0a3\n"},{"id":"161909","messageId":"20110222193021.GB28519@book.hvoigt.net","threadId":"26482","inReplyTo":"4D640227.9090206@web.de","subject":"[PATCH 2/2] git-gui: fetch/prune all entry appears last","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2011-02-22T19:30:21Z","receivedAt":"2011-02-22T19:30:21Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"The user might have got used to the order the remotes appeared previously.\nLets add the all entry last so the all entry does not confuse previous\nusers.\n\nSigned-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n---\n\nOn Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:\n> 2) I would rather like to see it at the *end* of the submenu, not at the\n>    beginning. Being used to always click on the first menu entry only\n>    to learn that the remote that used to be there got with something\n>    else is kind of surprising ;-)\n\nAnd this? Disclaimer: Also only superficially tested on OSX.\n\n lib/remote.tcl |   22 ++++++++++------------\n 1 files changed, 10 insertions(+), 12 deletions(-)\n\ndiff --git a/lib/remote.tcl b/lib/remote.tcl\nindex 18d3d06..5e4e7f4 100644\n--- a/lib/remote.tcl\n+++ b/lib/remote.tcl\n@@ -245,29 +245,27 @@ proc update_all_remotes_menu_entry {} {\n \tset prune_m $remote_m.prune\n \tif {$have_remote > 1} {\n \t\tmake_sure_remote_submenues_exist $remote_m\n-\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n-\t\tif {[$fetch_m entrycget $index -label] ne \"All\"} {\n+\t\tif {[$fetch_m entrycget end -label] ne \"All\"} {\n \n-\t\t\t$fetch_m insert $index separator\n-\t\t\t$fetch_m insert $index command \\\n+\t\t\t$fetch_m insert end separator\n+\t\t\t$fetch_m insert end command \\\n \t\t\t\t-label \"All\" \\\n \t\t\t\t-command fetch_from_all\n \n-\t\t\t$prune_m insert $index separator\n-\t\t\t$prune_m insert $index command \\\n+\t\t\t$prune_m insert end separator\n+\t\t\t$prune_m insert end command \\\n \t\t\t\t-label \"All\" \\\n \t\t\t\t-command prune_from_all\n \t\t}\n \t} else {\n \t\tif {[winfo exists $fetch_m]} {\n-\t\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n-\t\t\tif {[$fetch_m type end] eq \"separator\"} {\n+\t\t\tif {[$fetch_m entrycget end -label] eq \"All\"} {\n \n-\t\t\t\tdelete_from_menu $fetch_m $index\n-\t\t\t\tdelete_from_menu $fetch_m $index\n+\t\t\t\tdelete_from_menu $fetch_m end\n+\t\t\t\tdelete_from_menu $fetch_m end\n \n-\t\t\t\tdelete_from_menu $prune_m $index\n-\t\t\t\tdelete_from_menu $prune_m $index\n+\t\t\t\tdelete_from_menu $prune_m end\n+\t\t\t\tdelete_from_menu $prune_m end\n \t\t\t}\n \t\t}\n \t}\n-- \n1.7.4.1.30.gd0a3\n"},{"id":"162046","messageId":"4D655DC8.50109@web.de","threadId":"26482","inReplyTo":"20110222193021.GB28519@book.hvoigt.net","subject":"Re: [PATCH 2/2] git-gui: fetch/prune all entry appears last","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-02-23T19:19:36Z","receivedAt":"2011-02-23T19:19:36Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 22.02.2011 20:30, schrieb Heiko Voigt:\n> The user might have got used to the order the remotes appeared previously.\n> Lets add the all entry last so the all entry does not confuse previous\n> users.\n> \n> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n\nI tested both patches under Linux, looks great now.\n\nTested-by: Jens Lehmann <Jens.Lehmann@web.de>\n"},{"id":"162081","messageId":"87fwrefgfx.fsf@fox.patthoyts.tk","threadId":"26482","inReplyTo":"20110222192835.GA28519@book.hvoigt.net","subject":"Re: [PATCH 1/2] git-gui: fetch/prune all entry only for more than one entry","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2011-02-24T00:02:10Z","receivedAt":"2011-02-24T00:02:10Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Heiko Voigt <hvoigt@hvoigt.net> writes:\n\n>In case there is only one remote a fetch/prune all entry\n>is redundant.\n>\n>Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n>---\n>\n>On Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:\n>> 1) It would be nice if the new menu entry would only appear when there\n>>    is more than one remote to fetch from.\n>\n>How about this? Disclaimer: Only superficially tested on OSX.\n>\n> lib/remote.tcl |    4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n>diff --git a/lib/remote.tcl b/lib/remote.tcl\n>index 42d2061..18d3d06 100644\n>--- a/lib/remote.tcl\n>+++ b/lib/remote.tcl\n>@@ -237,13 +237,13 @@ proc update_all_remotes_menu_entry {} {\n> \n> \tset have_remote 0\n> \tforeach r $all_remotes {\n>-\t\tset have_remote 1\n>+\t\tincr have_remote\n> \t}\n> \n> \tset remote_m .mbar.remote\n> \tset fetch_m $remote_m.fetch\n> \tset prune_m $remote_m.prune\n>-\tif {$have_remote} {\n>+\tif {$have_remote > 1} {\n> \t\tmake_sure_remote_submenues_exist $remote_m\n> \t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n> \t\tif {[$fetch_m entrycget $index -label] ne \"All\"} {\n\nThis is fine - applied and checked it on Windows.\nI'll add a Suggested-by from Jens as this was a response to his request.\n\n-- \nPat Thoyts                            http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD\n"},{"id":"162083","messageId":"87bp22fg3r.fsf@fox.patthoyts.tk","threadId":"26482","inReplyTo":"20110222193021.GB28519@book.hvoigt.net","subject":"Re: [PATCH 2/2] git-gui: fetch/prune all entry appears last","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2011-02-24T00:09:28Z","receivedAt":"2011-02-24T00:09:28Z","isPatch":true,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"Heiko Voigt <hvoigt@hvoigt.net> writes:\n\n>The user might have got used to the order the remotes appeared previously.\n>Lets add the all entry last so the all entry does not confuse previous\n>users.\n>\n>Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>\n>---\n>\n>On Tue, Feb 22, 2011 at 07:36:23PM +0100, Jens Lehmann wrote:\n>> 2) I would rather like to see it at the *end* of the submenu, not at the\n>>    beginning. Being used to always click on the first menu entry only\n>>    to learn that the remote that used to be there got with something\n>>    else is kind of surprising ;-)\n>\n>And this? Disclaimer: Also only superficially tested on OSX.\n>\n> lib/remote.tcl |   22 ++++++++++------------\n> 1 files changed, 10 insertions(+), 12 deletions(-)\n>\n>diff --git a/lib/remote.tcl b/lib/remote.tcl\n>index 18d3d06..5e4e7f4 100644\n>--- a/lib/remote.tcl\n>+++ b/lib/remote.tcl\n>@@ -245,29 +245,27 @@ proc update_all_remotes_menu_entry {} {\n> \tset prune_m $remote_m.prune\n> \tif {$have_remote > 1} {\n> \t\tmake_sure_remote_submenues_exist $remote_m\n>-\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n>-\t\tif {[$fetch_m entrycget $index -label] ne \"All\"} {\n>+\t\tif {[$fetch_m entrycget end -label] ne \"All\"} {\n> \n>-\t\t\t$fetch_m insert $index separator\n>-\t\t\t$fetch_m insert $index command \\\n>+\t\t\t$fetch_m insert end separator\n>+\t\t\t$fetch_m insert end command \\\n> \t\t\t\t-label \"All\" \\\n> \t\t\t\t-command fetch_from_all\n> \n>-\t\t\t$prune_m insert $index separator\n>-\t\t\t$prune_m insert $index command \\\n>+\t\t\t$prune_m insert end separator\n>+\t\t\t$prune_m insert end command \\\n> \t\t\t\t-label \"All\" \\\n> \t\t\t\t-command prune_from_all\n> \t\t}\n> \t} else {\n> \t\tif {[winfo exists $fetch_m]} {\n>-\t\t\tset index [expr {[$fetch_m type 0] eq \"tearoff\" ? 1 : 0}]\n>-\t\t\tif {[$fetch_m type end] eq \"separator\"} {\n>+\t\t\tif {[$fetch_m entrycget end -label] eq \"All\"} {\n> \n>-\t\t\t\tdelete_from_menu $fetch_m $index\n>-\t\t\t\tdelete_from_menu $fetch_m $index\n>+\t\t\t\tdelete_from_menu $fetch_m end\n>+\t\t\t\tdelete_from_menu $fetch_m end\n> \n>-\t\t\t\tdelete_from_menu $prune_m $index\n>-\t\t\t\tdelete_from_menu $prune_m $index\n>+\t\t\t\tdelete_from_menu $prune_m end\n>+\t\t\t\tdelete_from_menu $prune_m end\n> \t\t\t}\n> \t\t}\n> \t}\n\nThis is fine as well. Tested it on windows. Applied to master.\n\n-- \nPat Thoyts                            http://www.patthoyts.tk/\nPGP fingerprint 2C 6E 98 07 2C 59 C8 97  10 CE 11 E6 04 E0 B9 DD\n"}]}