{"thread":{"id":"34849","subject":"[PATCH] git-gui: Modify push dialog to support Gerrit review","startedAt":"2013-09-02T08:54:19Z","lastAt":"2013-09-06T08:17:57Z","messageCount":11,"participants":["Joergen Edelbo","Heiko Voigt","Johannes Sixt","Jørgen Edelbo","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"226729","messageId":"20130904150853.52EC4121B7E@jed-dev-01.labnet","threadId":"34849","inReplyTo":null,"subject":"[PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Joergen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-02T08:54:19Z","receivedAt":"2013-09-02T08:54:19Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"Problem: It is not possible to push for Gerrit review\nas you will always try to push to /refs/heads/... on\nthe remote. As you should not be forced to work on a\nbranch with the same name as some branch on the remote,\nsome more flexibility in the selection of destination\nbranch is also needed.\n\nChanges done:\n\nRemove selection of branches to push - push always HEAD.\nThis can be justified by the fact that this far the most\ncommon thing to do.\nSpecify both the remote repository to push to as well as\nthe specific branch on that remote. This gives the flexibility.\n\nAdd option to specify \"Gerrit review\". If selected, replace\nthe traditional \"heads\" with the artificial \"for\" in the\ndestination ref spec. This is what actually solved the trigger\nproblem.\n\nLimit the branches to select from to the known branches\nfor currently selected remote. This is motivated in better\nusability. Works only when \"usettk\" is true - it is left for\nfurther study how to change the values in tk_optionMenu on the\nfly.\n\nSigned-off-by: Joergen Edelbo \n---\nHi there,\n\nWe are at Napatech A/S just about to roll out a Git/Gerrit/Jenkins\nsolution. It will really help the gui oriented people in pushing\ncommits if this can be done directly in git-gui.\n\nBR\nJoergen Edelbo\nNapatech A/S\n\n lib/transport.tcl | 184 +++++++++++++++++++++++++++++++++++++++++------------\n 1 file changed, 142 insertions(+), 42 deletions(-)\n\ndiff --git a/lib/transport.tcl b/lib/transport.tcl\nindex e5d211e..4c20ef7 100644\n--- a/lib/transport.tcl\n+++ b/lib/transport.tcl\n@@ -59,20 +59,42 @@ proc push_to {remote} {\n \tconsole::exec $w $cmd\n }\n \n-proc start_push_anywhere_action {w} {\n-\tglobal push_urltype push_remote push_url push_thin push_tags\n-\tglobal push_force\n-\tglobal repo_config\n-\n-\tset is_mirror 0\n-\tset r_url {}\n+proc get_remote_rep {} {\n+\tglobal push_urltype push_remote push_url\n+\tset rep {}\n \tswitch -- $push_urltype {\n-\tremote {\n-\t\tset r_url $push_remote\n-\t\tcatch {set is_mirror $repo_config(remote.$push_remote.mirror)}\n+\tremote { set rep $push_remote }\n+\turl { set rep $push_url }\n \t}\n-\turl {set r_url $push_url}\n+\treturn $rep\n+}\n+\n+proc get_remote_branch {} {\n+\tglobal push_branchtype push_branch push_new\n+\tset branch {}\n+\tswitch -- $push_branchtype {\n+\texisting { set branch $push_branch }\n+\tcreate { set branch $push_new }\n+\t}\n+ return $branch\n+}\n+\n+proc get_remote_ref_spec {} {\n+\tglobal gerrit_review\n+\tset push_branch [get_remote_branch]\n+\tif {$gerrit_review} {\n+\t\treturn \"refs/for/$push_branch\"\n+\t} else {\n+\t\treturn \"refs/heads/$push_branch\"\n \t}\n+}\n+\n+proc start_push_anywhere_action {w} {\n+\tglobal push_thin push_tags push_force\n+\tglobal repo_config current_branch is_detached\n+\n+\tset is_mirror 0\n+\tset r_url [get_remote_rep]\n \tif {$r_url eq {}} return\n \n \tset cmd [list git push]\n@@ -87,28 +109,25 @@ proc start_push_anywhere_action {w} {\n \t\tlappend cmd --tags\n \t}\n \tlappend cmd $r_url\n+\n+\tcatch {set is_mirror $repo_config(remote.$r_url.mirror)}\n \tif {$is_mirror} {\n \t\tset cons [console::new \\\n \t\t\t[mc \"push %s\" $r_url] \\\n \t\t\t[mc \"Mirroring to %s\" $r_url]]\n \t} else {\n-\t\tset cnt 0\n-\t\tforeach i [$w.source.l curselection] {\n-\t\t\tset b [$w.source.l get $i]\n-\t\t\tlappend cmd \"refs/heads/$b:refs/heads/$b\"\n-\t\t\tincr cnt\n-\t\t}\n-\t\tif {$cnt == 0} {\n-\t\t\treturn\n-\t\t} elseif {$cnt == 1} {\n-\t\t\tset unit branch\n+\t\tif {$is_detached} {\n+\t\t\tset src HEAD\n \t\t} else {\n-\t\t\tset unit branches\n+\t\t\tset src $current_branch\n \t\t}\n+\t\tset dest [get_remote_ref_spec]\n+\n+\t\tlappend cmd \"$src:$dest\"\n \n \t\tset cons [console::new \\\n \t\t\t[mc \"push %s\" $r_url] \\\n-\t\t\t[mc \"Pushing %s %s to %s\" $cnt $unit $r_url]]\n+\t\t\t[mc \"Pushing %s to %s\" $src $dest]]\n \t}\n \tconsole::exec $cons $cmd\n \tdestroy $w\n@@ -117,10 +136,58 @@ proc start_push_anywhere_action {w} {\n trace add variable push_remote write \\\n \t[list radio_selector push_urltype remote]\n \n+proc update_branchtype {br} {\n+\tglobal current_branch push_branch push_branchtype\n+\tif {$br eq {}} {\n+\t\tset push_branchtype create\n+\t\tset push_branch {}\n+\t} else {\n+\t\tset push_branchtype existing\n+\t\tif {[lsearch -sorted -exact $br $current_branch] != -1} {\n+\t\t\tset push_branch $current_branch\n+\t\t} elseif {[lsearch -sorted -exact $br master] != -1} {\n+\t\t\tset push_branch master\n+\t\t} else {\n+\t\t\tset push_branch [lindex $br 0]\n+\t\t}\n+\t}\n+}\n+\n+proc all_branches_combined {} {\n+\tset branches [list]\n+\tforeach spec [all_tracking_branches] {\n+\t\tset refn [lindex $spec 2]\n+\t\tregsub ^refs/heads/ $refn {} name\n+\t\tif { $name ne {HEAD} && [lsearch $branches $name] eq -1} {\n+\t\t\tlappend branches $name\n+\t\t}\n+\t}\n+\tupdate_branchtype $branches\n+\treturn $branches\n+}\n+\n+proc update_branches {} {\n+\tglobal push_remote branch_combo\n+\tset branches [list]\n+\tforeach spec [all_tracking_branches] {\n+\t\tif {[lindex $spec 1] eq $push_remote} {\n+\t\t\tset refn [lindex $spec 0]\n+\t\t\tregsub ^refs/(heads|remotes)/$push_remote/ $refn {} name\n+\t\t\tif {$name ne {HEAD}} {\n+\t\t\t\tlappend branches $name\n+\t\t\t}\n+\t\t}\n+\t}\n+\tupdate_branchtype $branches\n+\t$branch_combo configure -values $branches\n+\treturn $branches\n+}\n+\n proc do_push_anywhere {} {\n-\tglobal all_remotes current_branch\n-\tglobal push_urltype push_remote push_url push_thin push_tags\n-\tglobal push_force use_ttk NS\n+\tglobal all_remotes use_ttk branch_combo\n+\tglobal push_urltype push_remote push_url\n+\tglobal push_branchtype push_branch push_new\n+\tglobal push_thin push_tags push_force NS gerrit_review\n \n \tset w .push_setup\n \ttoplevel $w\n@@ -129,7 +196,7 @@ proc do_push_anywhere {} {\n \twm geometry $w \"+[winfo rootx .]+[winfo rooty .]\"\n \tpave_toplevel $w\n \n-\t${NS}::label $w.header -text [mc \"Push Branches\"] \\\n+\t${NS}::label $w.header -text [mc \"Push current HEAD\"] \\\n \t\t-font font_uibold -anchor center\n \tpack $w.header -side top -fill x\n \n@@ -144,21 +211,6 @@ proc do_push_anywhere {} {\n \tpack $w.buttons.cancel -side right -padx 5\n \tpack $w.buttons -side bottom -fill x -pady 10 -padx 10\n \n-\t${NS}::labelframe $w.source -text [mc \"Source Branches\"]\n-\tslistbox $w.source.l \\\n-\t\t-height 10 \\\n-\t\t-width 70 \\\n-\t\t-selectmode extended\n-\tforeach h [load_all_heads] {\n-\t\t$w.source.l insert end $h\n-\t\tif {$h eq $current_branch} {\n-\t\t\t$w.source.l select set end\n-\t\t\t$w.source.l yview end\n-\t\t}\n-\t}\n-\tpack $w.source.l -side left -fill both -expand 1\n-\tpack $w.source -fill both -expand 1 -pady 5 -padx 5\n-\n \t${NS}::labelframe $w.dest -text [mc \"Destination Repository\"]\n \tif {$all_remotes ne {}} {\n \t\t${NS}::radiobutton $w.dest.remote_r \\\n@@ -202,7 +254,51 @@ proc do_push_anywhere {} {\n \tgrid columnconfigure $w.dest 1 -weight 1\n \tpack $w.dest -anchor nw -fill x -pady 5 -padx 5\n \n+\t${NS}::labelframe $w.destbr -text [mc \"Destination Branches\"]\n+\tset all_branches [all_branches_combined]\n+\tif {$all_branches ne {}} {\n+\t\t${NS}::radiobutton $w.destbr.remote_b \\\n+\t\t\t-text [mc \"Known Branch: \"] \\\n+\t\t\t-value existing \\\n+\t\t\t-variable push_branchtype\n+\t\tif {$use_ttk} {\n+\t\t\tttk::combobox $w.destbr.remote_n -state readonly \\\n+\t\t\t\t-exportselection false \\\n+\t\t\t\t-textvariable push_branch\n+\t\t\tset branch_combo $w.destbr.remote_n\n+\t\t\tupdate_branches\n+\t\t} else {\n+\t\t\teval tk_optionMenu $w.destbr.remote_n push_branch $all_branches\n+\t\t}\n+\t\tgrid $w.destbr.remote_b $w.destbr.remote_n -sticky w\n+\t}\n+\n+\t${NS}::radiobutton $w.destbr.branch_r \\\n+\t\t-text [mc \"Arbitrary Branch:\"] \\\n+\t\t-value create \\\n+\t\t-variable push_branchtype\n+\t${NS}::entry $w.destbr.branch_t \\\n+\t\t-width 50 \\\n+\t\t-textvariable push_new \\\n+\t\t-validate key \\\n+\t\t-validatecommand {\n+\t\t\tif {%d == 1 && [regexp {\\s} %S]} {return 0}\n+\t\t\tif {%d == 1 && [string length %S] > 0} {\n+\t\t\t\tset push_branchtype create\n+\t\t\t}\n+\t\t\treturn 1\n+\t\t}\n+\tgrid $w.destbr.branch_r $w.destbr.branch_t -sticky we -padx {0 5}\n+\t${NS}::checkbutton $w.destbr.gerrit \\\n+\t\t-text [mc \"Push for Gerrit review (refs/for/...)\"] \\\n+\t\t-variable gerrit_review\n+\tgrid $w.destbr.gerrit -columnspan 2 -sticky w\n+\n+\tgrid columnconfigure $w.destbr 1 -weight 1\n+\tpack $w.destbr -anchor nw -fill x -pady 5 -padx 5\n+\n \t${NS}::labelframe $w.options -text [mc \"Transfer Options\"]\n+\n \t${NS}::checkbutton $w.options.force \\\n \t\t-text [mc \"Force overwrite existing branch (may discard changes)\"] \\\n \t\t-variable push_force\n@@ -222,10 +318,14 @@ proc do_push_anywhere {} {\n \tset push_force 0\n \tset push_thin 0\n \tset push_tags 0\n+\tset gerrit_review 0\n \n \tbind $w \"grab $w; focus $w.buttons.create\"\n \tbind $w \"destroy $w\"\n \tbind $w [list start_push_anywhere_action $w]\n+\tif {$all_remotes ne {}} {\n+\t\tbind $w.dest.remote_m <> { update_branches }\n+\t}\n \twm title $w [append \"[appname] ([reponame]): \" [mc \"Push\"]]\n \twm deiconify $w\n \ttkwait window $w\n-- \n1.7.9.5\n"},{"id":"226824","messageId":"20130905062045.GA4197@sandbox-ub","threadId":"34849","inReplyTo":"20130904150853.52EC4121B7E@jed-dev-01.labnet","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-09-05T06:20:45Z","receivedAt":"2013-09-05T06:20:45Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nOn Mon, Sep 02, 2013 at 10:54:19AM +0200, Joergen Edelbo wrote:\n> Problem: It is not possible to push for Gerrit review\n> as you will always try to push to /refs/heads/... on\n> the remote. As you should not be forced to work on a\n> branch with the same name as some branch on the remote,\n> some more flexibility in the selection of destination\n> branch is also needed.\n> \n> Changes done:\n> \n> Remove selection of branches to push - push always HEAD.\n> This can be justified by the fact that this far the most\n> common thing to do.\n> Specify both the remote repository to push to as well as\n> the specific branch on that remote. This gives the flexibility.\n> \n> Add option to specify \"Gerrit review\". If selected, replace\n> the traditional \"heads\" with the artificial \"for\" in the\n> destination ref spec. This is what actually solved the trigger\n> problem.\n> \n> Limit the branches to select from to the known branches\n> for currently selected remote. This is motivated in better\n> usability. Works only when \"usettk\" is true - it is left for\n> further study how to change the values in tk_optionMenu on the\n> fly.\n> \n> Signed-off-by: Joergen Edelbo \n> ---\n> Hi there,\n> \n> We are at Napatech A/S just about to roll out a Git/Gerrit/Jenkins\n> solution. It will really help the gui oriented people in pushing\n> commits if this can be done directly in git-gui.\n\nI like where this is heading. I myself was thinking about this some time\nago. Lets include Pat in the CC, since he is the current maintainer of\ngit-gui.\n\nGive me some time to review/testdrive the patch.\n\nCheers Heiko\n"},{"id":"226827","messageId":"522827CE.5040303@viscovery.net","threadId":"34849","inReplyTo":"20130904150853.52EC4121B7E@jed-dev-01.labnet","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-09-05T06:42:22Z","receivedAt":"2013-09-05T06:42:22Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 9/2/2013 10:54, schrieb Joergen Edelbo:\n> Changes done:\n> \n> Remove selection of branches to push - push always HEAD.\n> This can be justified by the fact that this far the most\n> common thing to do.\n\nWhat are your plans to support a topic-based workflow? \"Far the most\ncommon thing to happen\" is that someone forgets to push completed topics.\nWith this change, aren't those people forced to relinguish their current\nwork because they have to checkout the completed topics to push them?\n\n-- Hannes\n"},{"id":"226832","messageId":"F8C23E99F5C6884EB99E2A1966D743637F3619A6@cph-gen-exch01.napatech.com","threadId":"34849","inReplyTo":"522827CE.5040303@viscovery.net","subject":"RE: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Jørgen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-05T08:29:18Z","receivedAt":"2013-09-05T08:29:18Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"Hi,\n\nI am not quite sure what your concern is.\n\nAnyway - I don't think you are any more restricted with this change than before. If you currently work on a topic branch, and the same branch exists on the chosen remote, then this branch will be the default one to push to. \n\nBR Jørgen Edelbo\n\n-----Original Message-----\nFrom: Johannes Sixt [mailto:j.sixt@viscovery.net] \nSent: 5. september 2013 08:42\nTo: Jørgen Edelbo\nCc: git@vger.kernel.org; spearce@spearce.org; hvoigt@hvoigt.net\nSubject: Re: [PATCH] git-gui: Modify push dialog to support Gerrit review\n\nAm 9/2/2013 10:54, schrieb Joergen Edelbo:\n> Changes done:\n> \n> Remove selection of branches to push - push always HEAD.\n> This can be justified by the fact that this far the most common thing \n> to do.\n\nWhat are your plans to support a topic-based workflow? \"Far the most common thing to happen\" is that someone forgets to push completed topics.\nWith this change, aren't those people forced to relinguish their current work because they have to checkout the completed topics to push them?\n\n-- Hannes\n"},{"id":"226833","messageId":"5228476A.2070505@viscovery.net","threadId":"34849","inReplyTo":"F8C23E99F5C6884EB99E2A1966D743637F3619A6@cph-gen-exch01.napatech.com","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-09-05T08:57:14Z","receivedAt":"2013-09-05T08:57:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Please do not top-post.\n\nAm 9/5/2013 10:29, schrieb Jørgen Edelbo:\n> -----Original Message----- From: Johannes Sixt\n>> Am 9/2/2013 10:54, schrieb Joergen Edelbo:\n>>> Changes done:\n>>> \n>>> Remove selection of branches to push - push always HEAD. This can be\n>>> justified by the fact that this far the most common thing to do.\n>> \n>> What are your plans to support a topic-based workflow? \"Far the most\n>> common thing to happen\" is that someone forgets to push completed\n>> topics. With this change, aren't those people forced to relinguish\n>> their current work because they have to checkout the completed topics\n>> to push them?\n> \n> I am not quite sure what your concern is.\n\nWhen I have completed topics A and B, but forgot to push them, and now I\nam working on topic C, how do I push topics A and B?\n\nYou say I can only push HEAD. I understand this that I have to stop work\non C (perhaps commit or stash any unfinished work), then checkout A, push\nit, checkout B, push it, checkout C and unstash the unfinished work. If my\nunderstanding is correct, the new restriction is a no-go.\n\n-- Hannes\n"},{"id":"226834","messageId":"F8C23E99F5C6884EB99E2A1966D743637F3619CB@cph-gen-exch01.napatech.com","threadId":"34849","inReplyTo":"5228476A.2070505@viscovery.net","subject":"RE: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Jørgen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-05T09:18:25Z","receivedAt":"2013-09-05T09:18:25Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"> -----Original Message-----\n> From: Johannes Sixt [mailto:j.sixt@viscovery.net]\n> Sent: 5. september 2013 10:57\n> \n> Please do not top-post.\n> \n> Am 9/5/2013 10:29, schrieb Jørgen Edelbo:\n> > -----Original Message----- From: Johannes Sixt\n> >> Am 9/2/2013 10:54, schrieb Joergen Edelbo:\n> >>> Changes done:\n> >>>\n> >>> Remove selection of branches to push - push always HEAD. This can be\n> >>> justified by the fact that this far the most common thing to do.\n> >>\n> >> What are your plans to support a topic-based workflow? \"Far the most\n> >> common thing to happen\" is that someone forgets to push completed\n> >> topics. With this change, aren't those people forced to relinguish\n> >> their current work because they have to checkout the completed topics\n> >> to push them?\n> >\n> > I am not quite sure what your concern is.\n> \n> When I have completed topics A and B, but forgot to push them, and now I\n> am working on topic C, how do I push topics A and B?\n> \n> You say I can only push HEAD. I understand this that I have to stop work on C\n> (perhaps commit or stash any unfinished work), then checkout A, push it,\n> checkout B, push it, checkout C and unstash the unfinished work. If my\n> understanding is correct, the new restriction is a no-go.\n\nOk, this way of working is not supported. It just never occurred to me that\nyou would work this way. Forgetting to push something that you have just \ncompleted is very far from what I am used to. I think it comes most natural\nto push what you have done before changing topic. The reason I make a commit\nis to get it out of the door.\n\n> \n> -- Hannes\n\n- Jørgen\n"},{"id":"226835","messageId":"52284F54.4080405@viscovery.net","threadId":"34849","inReplyTo":"F8C23E99F5C6884EB99E2A1966D743637F3619CB@cph-gen-exch01.napatech.com","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-09-05T09:31:00Z","receivedAt":"2013-09-05T09:31:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 9/5/2013 11:18, schrieb Jørgen Edelbo:\n> Forgetting to push something that you have just \n> completed is very far from what I am used to.\n\n\"Forgetting to push\" is just one of many reasons why a branch that is not\nequal to HEAD is not yet pushed... The new restriction is just too tight.\n\n-- Hannes\n"},{"id":"226865","messageId":"xmqqioyfjhok.fsf@gitster.dls.corp.google.com","threadId":"34849","inReplyTo":"F8C23E99F5C6884EB99E2A1966D743637F3619CB@cph-gen-exch01.napatech.com","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-05T17:34:51Z","receivedAt":"2013-09-05T17:34:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jørgen Edelbo writes:\n\n>> You say I can only push HEAD. I understand this that I have to stop work on C\n>> (perhaps commit or stash any unfinished work), then checkout A, push it,\n>> checkout B, push it, checkout C and unstash the unfinished work. If my\n>> understanding is correct, the new restriction is a no-go.\n>\n> Ok, this way of working is not supported. It just never occurred to me that\n> you would work this way. Forgetting to push something that you have just \n> completed is very far from what I am used to. I think it comes most natural\n> to push what you have done before changing topic. The reason I make a commit\n> is to get it out of the door.\n\nPeople work in very different ways, and mine and yours are extreme\nopposites. I almost never push out a commit that is just off the\npress, I use topic branches heavily and work on multiple topics\n(either related or unrelated) in parallel all the time, so it is\nvery usual for me to push out more than one branches with a single\npush---by definition, if we support pushing only the current branch\nout, \"working on more than one topics and after perfecting these,\npush them together\" cannot be done.\n\nIf one sets push.default to something other than the matching, and\ndoes not have any remote.*.push refspec in the configuration, J6t's\n\"I forgot to push while I was on that branch\" and my \"I deliberately\ndid not push those branches out while I was on them, but now I know\nI am ready to push them out\" cannot be done without explicit\nrefspecs on the command line.\n\nBut stepping back a bit, I think this \"I commit because I want to\nget it out the door\" is coming from the same desire that led to a\npossible design mistake that made various push.default settings push\nonly the current branch out.\n\nThe possible design mistake that has been disturbing me is the\nfollowing.\n\nThe \"push.default\" setting controls two unrelated things, and that\nis one too many. Between the matching modes and the rest, it tells\nwhat branch(es) are pushed out (either \"all the branches with the\nsame name\" or \"the current branch\"). That is one thing, and I tend\nto agree that \"push only the current branch\" would be a sane default\nmany people would prefer (and that is the reason we made it the\ndefault for Git 2.0).\n\nAmong the various non matching modes, it also determines where a\nbranch that is pushed out would go (\"current\" pushes to the same\nname, \"upstream\" pushes to @{upstream}, etc.). But this \"once I\nchoose what branch to push out, the branch being pushed out knows\nwhere it wants to go\", does not take effect if one explicitly names\nwhat to push, e.g. in this sequence where one forgets to push\n'frotz' out:\n\n j6t$ git checkout frotz\n ... work work work ...\n j6t$ git checkout xyzzy\n ... work work work ...\n ... realize 'frotz' is done\n j6t$ git push origin frotz\n\nThe last push may work differently from the push in this sequence:\n\n j6t$ git checkout frotz\n ... work work work ...\n j6t$ git push ;# or \"git push origin\"\n\nIn the latter sequence, the niceties of push.upstream to push\n'frotz' out to the frotz@{upstream} (and your \"git push origin\nrefs/heads/frotz:refs/for/frotz\" mapping done in git-gui) will take\neffect, but in the former, the refspec \"frotz\" on the command line\nis taken as a short-hand for \"frotz:frotz\".\n\nWe may want to teach \"git push\" that the command line refspec that\nnames a local branch may not be pushing to the same name but wants\nto go through the same \"mapping\" used when \"git push\" is done while\nthe branch is checked out, with some mechanism.\n\nIt is a separate but very related topic, I think.\n"},{"id":"226867","messageId":"xmqqtxhzi12h.fsf@gitster.dls.corp.google.com","threadId":"34849","inReplyTo":"20130904150853.52EC4121B7E@jed-dev-01.labnet","subject":"Re: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-05T18:19:02Z","receivedAt":"2013-09-05T18:19:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joergen Edelbo writes:\n\n> +proc get_remote_branch {} {\n> +\tglobal push_branchtype push_branch push_new\n> +\tset branch {}\n> +\tswitch -- $push_branchtype {\n> +\texisting { set branch $push_branch }\n> +\tcreate { set branch $push_new }\n> +\t}\n> + return $branch\n> +}\n> +\n> +proc get_remote_ref_spec {} {\n> +\tglobal gerrit_review\n> +\tset push_branch [get_remote_branch]\n> +\tif {$gerrit_review} {\n> +\t\treturn \"refs/for/$push_branch\"\n> +\t} else {\n> +\t\treturn \"refs/heads/$push_branch\"\n> \t}\n> +}\n\nI am puzzled. This may be fine for those who use Git-GUI and\nnothing else to push, but will not help whose who use both Git-GUI\nand the command line.\n\nIsn't the right way to improve the situation to let the command line\ntools know how the user wants to push things out and just have\nGit-GUI delegate the choice to the underlying \"git push\"?\n\nFor example, if you are working on a topic 'frotz', and if the\nlocation you push is managed by Gerrit, isn't it the case that you\nalways want to push it to \"refs/for/frotz\", whether you are pushing\nvia Git-GUI or from the command line?\n\nI think we discussed during 1.8.4 cycle a configuration like this:\n\n\t[branch \"frotz\"]\n \tpush = refs/heads/frotz:refs/for/frotz\n\nas part of the \"triangular workflow\" topic that allows you to\nspecify that \"when 'frotz' is pushed out, it goes to\nrefs/for/frotz\", or something like that, but I do not recall what\ncame out of that.\n"},{"id":"226882","messageId":"20130905210402.GA2939@sandbox-ub","threadId":"34849","inReplyTo":"F8C23E99F5C6884EB99E2A1966D743637F3619CB@cph-gen-exch01.napatech.com","subject":"Re: RE: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-09-05T21:04:02Z","receivedAt":"2013-09-05T21:04:02Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Thu, Sep 05, 2013 at 09:18:25AM +0000, Jørgen Edelbo wrote:\n> > -----Original Message-----\n> > From: Johannes Sixt [mailto:j.sixt@viscovery.net]\n> > Sent: 5. september 2013 10:57\n> > \n> > Please do not top-post.\n> > \n> > Am 9/5/2013 10:29, schrieb Jørgen Edelbo:\n> > > -----Original Message----- From: Johannes Sixt\n> > >> Am 9/2/2013 10:54, schrieb Joergen Edelbo:\n> > >>> Changes done:\n> > >>>\n> > >>> Remove selection of branches to push - push always HEAD. This can be\n> > >>> justified by the fact that this far the most common thing to do.\n> > >>\n> > >> What are your plans to support a topic-based workflow? \"Far the most\n> > >> common thing to happen\" is that someone forgets to push completed\n> > >> topics. With this change, aren't those people forced to relinguish\n> > >> their current work because they have to checkout the completed topics\n> > >> to push them?\n> > >\n> > > I am not quite sure what your concern is.\n> > \n> > When I have completed topics A and B, but forgot to push them, and now I\n> > am working on topic C, how do I push topics A and B?\n> > \n> > You say I can only push HEAD. I understand this that I have to stop work on C\n> > (perhaps commit or stash any unfinished work), then checkout A, push it,\n> > checkout B, push it, checkout C and unstash the unfinished work. If my\n> > understanding is correct, the new restriction is a no-go.\n> \n> Ok, this way of working is not supported. It just never occurred to me that\n> you would work this way. Forgetting to push something that you have just \n> completed is very far from what I am used to. I think it comes most natural\n> to push what you have done before changing topic. The reason I make a commit\n> is to get it out of the door.\n\nFWIW, I also think that we should keep the box which allows you to\nselect the branch to push. I did not realize that you were removing it\nwhen I first glanced at your patch.\n\nEven if your reasoning that pushing the currently checked out branch is\ncorrect: This box has been around for too long, so it will annoy people\nthat got used to the fact that they can select the branch to push.\n\nAnother problem: It is not very intuitive to only select the branch to\npush to. You can do that on the command line but IMO using\n\n\tgit push origin HEAD:refs/heads/\n\nis way less common than\n\n\tgit push origin \n\nand I think that should also be reflected in the gui. It might be more\ncommon for a gerrit user but for the typical git user without gerrit it\nis not.\n\nSo to make it easy for the user to transition from gui to commandline\nand back with your patch I would expect: The user selects a branch\nto push. The new \"Destination Branches\" section automatically selects/shows\nthe same name for the default case as destination (like the cli). So\nif I only select the branch to push it behaves the same as before.\n\nIf you detect (I assume that is possible somehow) that the remote is a\ngerrit remote: \"Push for Gerrit review\" would automatically be ticked and\nthe branch a git pull would merge (e.g. the one from branch..merge)\nis selected as the destination branch under refs/for/... . If there is\nno config for that, fallback to \"master\".\n\nThis is what I would expect with no further extension of the current git\ncommand line behavior and config options. So that way your patch will be\nan *extension* and not a change of behavior.\n\nAnother unrelated thing that is currently left out: You can transport\nthe local branchname when pushing to the magical gerrit refs/for/... . I\nwould like to see that appended as well. But opposed to the branch\nselection that is not a show stopper for the patch more a side note.\n\nCheers Heiko\n"},{"id":"226920","messageId":"F8C23E99F5C6884EB99E2A1966D743637F361D2A@cph-gen-exch01.napatech.com","threadId":"34849","inReplyTo":"xmqqioyfjhok.fsf@gitster.dls.corp.google.com","subject":"RE: [PATCH] git-gui: Modify push dialog to support Gerrit review","fromName":"Jørgen Edelbo","fromEmail":"jed@napatech.com","sentAt":"2013-09-06T08:17:57Z","receivedAt":"2013-09-06T08:17:57Z","isPatch":true,"sender":{"key":"jed@napatech.com","avatar":null},"body":"Junio C Hamano < gitster@pobox.com> writes:\n\n> Isn't the right way to improve the situation to let the command line tools\n> know how the user wants to push things out and just have Git-GUI delegate\n> the choice to the underlying \"git push\"?\n\nThank you for all the constructive feedback. I realize that it is not the way forward to remove the selection of branches to push.\n\nWhat I consider now, is to pursue the idea that Junio presents above: just let the gui tool use whatever is configured for the selected remote. I have, however not been able to come up with a solution that looks nice. What I have been trying now is the following little modification:\n\ndiff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl\nindex e5d211e..1709464 100644\n--- a/git-gui/lib/transport.tcl\n+++ b/git-gui/lib/transport.tcl\n@@ -95,7 +95,9 @@ proc start_push_anywhere_action {w} {\n set cnt 0\n foreach i [$w.source.l curselection] {\n set b [$w.source.l get $i]\n- lappend cmd \"refs/heads/$b:refs/heads/$b\"\n+ if {$b nw {}}{\n+ lappend cmd \"refs/heads/$b:refs/heads/$b\"\n+ }\n incr cnt\n }\n if {$cnt == 0} {\n@@ -149,6 +151,7 @@ proc do_push_anywhere {} {\n -height 10 \\\n -width 70 \\\n -selectmode extended\n+ $w.source.l insert end \n foreach h [load_all_heads] {\n $w.source.l insert end $h\n if {$h eq $current_branch} {\n\nThe idea is to insert a \"\" entry in the branch selection list, and then skip that when building the command. Then you can avoid having refs on the command line and just let git act according to configuration.\n\nHow about that? Any smarter way to do it?\n\nBR Jørgen\n"}]}