threads / patch / 15731

patchRe: [PATCH] git-gui: Implement a 'clone' subcommand

Subject: Re: [PATCH] git-gui: Implement a 'clone' subcommand

## tl;dr

8 messages between Sep 30, 2008 and Oct 10, 2008. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Petr Baudis· Sep 30, 2008, 16:51 UTC · lore

[PATCH] git-gui: Implement a 'clone' subcommand

This enables git-gui to be started with the clone dialog opened right away, possibly with the URL prefilled when it is passed as another argument. git-gui can then be e.g. registered as the git:// protocol handler.

This is just a simple implementation - we construct the front actions page, then throw it away immediately; I wanted to avoid unnecessary refactoring and complication of the code, though.

Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
---
 Documentation/git-gui.txt         |    5 +++++
 git-gui/git-gui.sh                |   21 ++++++++++++++++++---
 git-gui/lib/choose_repository.tcl |   11 ++++++++++-
 3 files changed, 33 insertions(+), 4 deletions(-)
Show changes to 3 files +33 −4

Documentation/git-gui.txt, git-gui/git-gui.sh, git-gui/lib/choose_repository.tcl

diff --git a/Documentation/git-gui.txt b/Documentation/git-gui.txt
index 0e650f4..9ce63be 100644
--- a/Documentation/git-gui.txt
+++ b/Documentation/git-gui.txt
@@ -43,6 +43,11 @@ citool::
 	to only commit actions, slightly reducing the application's
 	startup time and simplifying the menubar.
 
+clone::
+	Start the 'git-gui' clone dialog, optionally taking
+	a source location as an extra argument to pre-fill
+	in the dialog.
+
 version::
 	Display the currently running version of 'git-gui'.
 
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 4085e8f..86f0151 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -950,6 +950,8 @@ enable_option multicommit
 enable_option branch
 enable_option transport
 disable_option bare
+set init_action {}
+set clone_url {}
 
 switch -- $subcommand {
 browser -
@@ -960,6 +962,13 @@ blame {
 	disable_option branch
 	disable_option transport
 }
+clone {
+	set init_action "clone"
+	if {[llength $argv] > 0} {
+		set clone_url [lindex $argv 0]
+		set argv [lrange $argv 1 end]
+	}
+}
 citool {
 	enable_option singlecommit
 	enable_option retcode
@@ -995,7 +1004,12 @@ citool {
 ##
 ## repository setup
 
-if {[catch {
+if {$init_action != ""} {
+	load_config 1
+	apply_config
+	choose_repository::pick $init_action
+
+} elseif {[catch {
 		set _gitdir $env(GIT_DIR)
 		set _prefix {}
 		}]
@@ -1005,7 +1019,7 @@ if {[catch {
 	} err]} {
 	load_config 1
 	apply_config
-	choose_repository::pick
+	choose_repository::pick {}
 }
 if {![file isdirectory $_gitdir] && [is_Cygwin]} {
 	catch {set _gitdir [exec cygpath --windows $_gitdir]}
@@ -2622,6 +2636,7 @@ blame {
 	return
 }
 citool -
+clone -
 gui {
 	if {[llength $argv] != 0} {
 		puts -nonewline stderr "usage: $argv0"
@@ -2635,7 +2650,7 @@ gui {
 	# fall through to setup UI for commits
 }
 default {
-	puts stderr "usage: $argv0 \[{blame|browser|citool}\]"
+	puts stderr "usage: $argv0 \[{blame|browser|citool|clone}\]"
 	exit 1
 }
 }
diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
index 3180786..290fbc0 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -21,7 +21,7 @@ field clone_type hardlink ; # Type of clone to construct
 field readtree_err        ; # Error output from read-tree (if any)
 field sorted_recent       ; # recent repositories (sorted)
 
-constructor pick {} {
+constructor pick {action} {
 	global M1T M1B
 
 	make_toplevel top w
@@ -195,6 +195,11 @@ constructor pick {} {
 		bind $top <Visibility> {}
 	"
 	wm deiconify $top
+
+	if {$action != ""} {
+		_next $this $action
+	}
+
 	tkwait variable @done
 
 	if {$top eq {.}} {
@@ -447,6 +452,10 @@ proc _new_ok {p} {
 ## Clone Existing Repository
 
 method _do_clone {} {
+	if {$::clone_url != ""} {
+		set origin_url $::clone_url
+	}
+
 	$w_next conf \
 		-state disabled \
 		-command [cb _do_clone2] \
-- 
tg: (9800c0d..) t/git-gui/clonecmd (depends on: vanilla/master)
Shawn O. Pearce· Sep 30, 2008, 19:53 UTC · re: Petr Baudis · lore
Petr Baudis <pasky@suse.cz> wrote:
Show 16 quoted lines
> This enables git-gui to be started with the clone dialog opened right
> away, possibly with the URL prefilled when it is passed as another
> argument. git-gui can then be e.g. registered as the git:// protocol
> handler.
> 
> This is just a simple implementation - we construct the front actions
> page, then throw it away immediately; I wanted to avoid unnecessary
> refactoring and complication of the code, though.
> 
> Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
> 
> ---
>  Documentation/git-gui.txt         |    5 +++++
>  git-gui/git-gui.sh                |   21 ++++++++++++++++++---
>  git-gui/lib/choose_repository.tcl |   11 ++++++++++-
>  3 files changed, 33 insertions(+), 4 deletions(-)

This looks fine, except for the diffstat. You can't patch the docs and the code in the same patch as they are in two different repositories... :-|

Also, I don't know if you've noticed but I think tg is sending duplicate "To" headers in the messages:

	From: Petr Baudis <pasky@suse.cz>
*	To: git@vger.kernel.org
	Cc: spearce@spearce.org, Petr Baudis <petr.baudis@novartis.com>
	Subject: [PATCH] git-gui: Implement a 'clone' subcommand
	Date: Tue, 30 Sep 2008 18:51:41 +0200
	Message-Id: <1222793501-17997-1-git-send-email-pasky@suse.cz>
	X-Mailer: git-send-email 1.5.6.3.392.g292f1
*	To: git@vger.kernel.org
	Sender: git-owner@vger.kernel.org
	Precedence: bulk
	List-ID: <git.vger.kernel.org>
	X-Mailing-List: git@vger.kernel.org
 
-- 
Shawn.
Shawn O. Pearce· Sep 30, 2008, 19:58 UTC · re: Shawn O. Pearce · lore
"Shawn O. Pearce" <spearce@spearce.org> wrote:
Show 19 quoted lines
> Petr Baudis <pasky@suse.cz> wrote:
> > This enables git-gui to be started with the clone dialog opened right
> > away, possibly with the URL prefilled when it is passed as another
> > argument. git-gui can then be e.g. registered as the git:// protocol
> > handler.
> > 
> > This is just a simple implementation - we construct the front actions
> > page, then throw it away immediately; I wanted to avoid unnecessary
> > refactoring and complication of the code, though.
> > 
> > Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
> > 
> > ---
> >  Documentation/git-gui.txt         |    5 +++++
> >  git-gui/git-gui.sh                |   21 ++++++++++++++++++---
> >  git-gui/lib/choose_repository.tcl |   11 ++++++++++-
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> 
> This looks fine [...]

I take that back. "git gui clone git://repo.or.cz/alt-git.git" didn't show the URL in the clone dialog for me. This is on top of your previous series, including the "locator" feature, and with two locators configured in my .git/config.

-- 
Shawn.
Petr Baudis· Oct 3, 2008, 10:13 UTC · re: Shawn O. Pearce · lore

[PATCH] git-gui: Correctly set up locators in case of preset URL variable

This patch fixes locators setup in case the URL variable is already set, e.g. in the clone dialog during 'git gui clone'.

Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
---
 git-gui/lib/transport.tcl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
Show changes to git-gui/lib/transport.tcl +7 −2
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index 277e6b8..02c4eca 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
 
 	global _locator_template _locator_input _locator_var
 	trace remove variable _locator_input write locator_update
-	set _locator_template $default_locator
-	set _locator_input {}
+	if {[set $urlvar] == {}} {
+		set _locator_template $default_locator
+		set _locator_input {}
+	} else {
+		set _locator_template "URL"
+		set _locator_input [set $urlvar]
+	}
 	set _locator_var $urlvar
 	trace add variable _locator_input write locator_update
 
-- 
tg: (3c6c738..) t/git-gui/locator-preset (depends on: git-gui/locators t/git-gui/clonecmd)
H.Merijn Brand· Oct 3, 2008, 10:23 UTC · re: Petr Baudis · lore

Re: [PATCH] git-gui: Correctly set up locators in case of preset URL variable

On Fri,  3 Oct 2008 12:13:46 +0200, Petr Baudis <pasky@suse.cz> wrote:
> This patch fixes locators setup in case the URL variable is already set,
> e.g. in the clone dialog during 'git gui clone'.

While you are working on git-gui's transport.tcl, would you consider a user setting to set the default for pushing tags to remote repositories?

After installation/update I always `fix' that by hand now.
Show 25 quoted lines
> Signed-off-by: Petr Baudis <petr.baudis@novartis.com>
> 
> ---
>  git-gui/lib/transport.tcl |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
> index 277e6b8..02c4eca 100644
> --- a/git-gui/lib/transport.tcl
> +++ b/git-gui/lib/transport.tcl
> @@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
>  
>  	global _locator_template _locator_input _locator_var
>  	trace remove variable _locator_input write locator_update
> -	set _locator_template $default_locator
> -	set _locator_input {}
> +	if {[set $urlvar] == {}} {
> +		set _locator_template $default_locator
> +		set _locator_input {}
> +	} else {
> +		set _locator_template "URL"
> +		set _locator_input [set $urlvar]
> +	}
>  	set _locator_var $urlvar
>  	trace add variable _locator_input write locator_update
-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/
Petr Baudis· Oct 3, 2008, 10:48 UTC · re: H.Merijn Brand · lore

Re: [PATCH] git-gui: Correctly set up locators in case of preset URL variable

On Fri, Oct 03, 2008 at 12:23:29PM +0200, H.Merijn Brand wrote:
Show 9 quoted lines
> On Fri,  3 Oct 2008 12:13:46 +0200, Petr Baudis <pasky@suse.cz> wrote:
> 
> > This patch fixes locators setup in case the URL variable is already set,
> > e.g. in the clone dialog during 'git gui clone'.
> 
> While you are working on git-gui's transport.tcl, would you consider a
> user setting to set the default for pushing tags to remote repositories?
> 
> After installation/update I always `fix' that by hand now.

Actually, I think for the time being I'm finished with transport.tcl, I'm sorry. :-) I probably won't work further on git-gui now that my Novartis stay is almost over. My last stab will be probably trying to change locators support to use insteadOf and possibly configurability of the publish dialog if people decide they want it.

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.
Shawn O. Pearce· Oct 10, 2008, 16:42 UTC · re: Petr Baudis · lore

Re: [PATCH] git-gui: Correctly set up locators in case of preset URL variable

Petr Baudis <pasky@suse.cz> wrote:
> This patch fixes locators setup in case the URL variable is already set,
> e.g. in the clone dialog during 'git gui clone'.
Huh.  Better, but Remote->Push... still crashes with your series:
can't read "push_url": no such variable
can't read "push_url": no such variable
    while executing
"set $urlvar"
    (procedure "location_input" line 26)
    invoked from within
"location_input $w.dest.url_t push_url push"
    (procedure "do_push_anywhere" line 53)
    invoked from within
"do_push_anywhere"
    invoked from within
".#mbar.#mbar#remote invoke active"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 [list $w invoke active]"
    (procedure "tk::MenuInvoke" line 50)
    invoked from within
"tk::MenuInvoke .#mbar.#mbar#remote 1"
    (command bound to event)
 
Show 22 quoted lines
> diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
> index 277e6b8..02c4eca 100644
> --- a/git-gui/lib/transport.tcl
> +++ b/git-gui/lib/transport.tcl
> @@ -68,8 +68,13 @@ proc location_input {widget urlvar op} {
>  
>  	global _locator_template _locator_input _locator_var
>  	trace remove variable _locator_input write locator_update
> -	set _locator_template $default_locator
> -	set _locator_input {}
> +	if {[set $urlvar] == {}} {
> +		set _locator_template $default_locator
> +		set _locator_input {}
> +	} else {
> +		set _locator_template "URL"
> +		set _locator_input [set $urlvar]
> +	}
>  	set _locator_var $urlvar
>  	trace add variable _locator_input write locator_update
>  
> -- 
> tg: (3c6c738..) t/git-gui/locator-preset (depends on: git-gui/locators t/git-gui/clonecmd)
-- 
Shawn.
Petr Baudis· Oct 3, 2008, 10:17 UTC · re: Shawn O. Pearce · lore
On Tue, Sep 30, 2008 at 12:53:30PM -0700, Shawn O. Pearce wrote:
Show 9 quoted lines
> Petr Baudis <pasky@suse.cz> wrote:
> >  Documentation/git-gui.txt         |    5 +++++
> >  git-gui/git-gui.sh                |   21 ++++++++++++++++++---
> >  git-gui/lib/choose_repository.tcl |   11 ++++++++++-
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> 
> This looks fine, except for the diffstat.  You can't patch
> the docs and the code in the same patch as they are in two
> different repositories...  :-|

Oh, I forgot again, sorry. :-( But I think this shows rather bad design of the layout, doesn't it? Shouldn't this documentation be kept in git-gui/Documentation/{git-gui,config}.txt and included from git's Documentation/?

> Also, I don't know if you've noticed but I think tg is
> sending duplicate "To" headers in the messages:

Thanks - I have noticed already but I didn't have time yet to fix it and it seems generally pretty harmless. I will try to do something with that later.

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.

← back to recent threads