[PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Henri GEIST]

[-- Attachment #1: Type: text/plain, Size: 2714 bytes --]

Permit to do a 'git clone --recursive' through git-gui.

Signed-off-by: Henri GEIST <geist.henri@laposte.net>
---
I have set the default checkbox state to 'true' by default has all my gui users
use it all the time this way.
But as it change the default behavior you may prefer to set it to 'false' by
default.

 git-gui/lib/choose_repository.tcl |   34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
index 3c10bc6..47d436b 100644
--- a/git-gui/lib/choose_repository.tcl
+++ b/git-gui/lib/choose_repository.tcl
@@ -18,6 +18,7 @@ field local_path       {} ; # Where this repository is locally
 field origin_url       {} ; # Where we are cloning from
 field origin_name  origin ; # What we shall call 'origin'
 field clone_type hardlink ; # Type of clone to construct
+field recursive      true ; # Recursive cloning flag
 field readtree_err        ; # Error output from read-tree (if any)
 field sorted_recent       ; # recent repositories (sorted)
 
@@ -525,6 +526,11 @@ method _do_clone {} {
 	foreach r $w_types {
 		pack $r -anchor w
 	}
+	${NS}::checkbutton $args.type_f.recursive \
+		-text [mc "Recursive (For submodules)"] \
+		-variable @recursive \
+		-onvalue true -offvalue false
+	pack $args.type_f.recursive
 	grid $args.type_l $args.type_f -sticky new
 
 	grid columnconfigure $args 1 -weight 1
@@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
 	fileevent $fd readable [cb _readtree_wait $fd]
 }
 
+method _do_validate_submodule_cloning {ok} {
+	if {$ok} {
+		$o_cons done $ok
+		set done 1
+	} else {
+		_clone_failed $this [mc "Cannot clone submodules."]
+	}
+}
+
+method _do_clone_submodules {} {
+	if {$recursive eq {true}} {
+		destroy $w_body
+		set o_cons [console::embed \
+			$w_body \
+			[mc "Cloning submodules"]]
+		pack $w_body -fill both -expand 1 -padx 10
+		$o_cons exec \
+			[list git submodule update --init --recursive] \
+			[cb _do_validate_submodule_cloning]
+	} else {
+		set done 1
+	}
+}
+
 method _readtree_wait {fd} {
 	set buf [read $fd]
 	$o_cons update_meter $buf
@@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
 		fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
 		fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
 	} else {
-		set done 1
+		_do_clone_submodules $this
 	}
 }
 
@@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
 			hook_failed_popup post-checkout $pch_error 0
 		}
 		unset pch_error
-		set done 1
+		_do_clone_submodules $this
 		return
 	}
 	fconfigure $fd_ph -blocking 0
-- 
1.7.9.3.369.gd715.dirty


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Jens Lehmann]

Am 05.03.2014 00:01, schrieb Henri GEIST:
> Permit to do a 'git clone --recursive' through git-gui.

I really like where this is heading!

Some minor issues:

- I think we should be more verbose in the commit message,
  including that and why the default should be "on". Maybe
  like this?

  "Permit to do a 'git clone --recursive' through git-gui.
  Add a 'recursive' checkbox in the clone menu which allows
  users to clone a repository and all its submodules in one
  go (unless the 'update' flag is set to "none" in the
  .gitmodules file for a submodule, in that case that
  specific submodule is not cloned automatically).

  Enable this new option per default, as most users want to
  clone all submodules too when cloning the superproject
  (This is currently not possible without leaving git gui
  or adding a custom tool entry for that)."


- I'd rather change the button text from "Recursive (For
  submodules)" to something like "Recursively clone
  submodules too" or such.


- Wouldn't it be easier to pass the '--recurse-submodules"
  option to the "git clone" call for the superproject instead
  of adding the _do_clone_submodules() function doing a
  subsequent "git submodule update --init --recursive"? That
  is also be more future proof with respect to the autoclone
  config option we have in mind (which would add that behavior
  for "git clone" itself, making the call you added redundant).


> Signed-off-by: Henri GEIST <geist.henri@laposte.net>
> ---
> I have set the default checkbox state to 'true' by default has all my gui users
> use it all the time this way.
> But as it change the default behavior you may prefer to set it to 'false' by
> default.
> 
>  git-gui/lib/choose_repository.tcl |   34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
> index 3c10bc6..47d436b 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -18,6 +18,7 @@ field local_path       {} ; # Where this repository is locally
>  field origin_url       {} ; # Where we are cloning from
>  field origin_name  origin ; # What we shall call 'origin'
>  field clone_type hardlink ; # Type of clone to construct
> +field recursive      true ; # Recursive cloning flag
>  field readtree_err        ; # Error output from read-tree (if any)
>  field sorted_recent       ; # recent repositories (sorted)
>  
> @@ -525,6 +526,11 @@ method _do_clone {} {
>  	foreach r $w_types {
>  		pack $r -anchor w
>  	}
> +	${NS}::checkbutton $args.type_f.recursive \
> +		-text [mc "Recursive (For submodules)"] \
> +		-variable @recursive \
> +		-onvalue true -offvalue false
> +	pack $args.type_f.recursive
>  	grid $args.type_l $args.type_f -sticky new
>  
>  	grid columnconfigure $args 1 -weight 1
> @@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
>  	fileevent $fd readable [cb _readtree_wait $fd]
>  }
>  
> +method _do_validate_submodule_cloning {ok} {
> +	if {$ok} {
> +		$o_cons done $ok
> +		set done 1
> +	} else {
> +		_clone_failed $this [mc "Cannot clone submodules."]
> +	}
> +}
> +
> +method _do_clone_submodules {} {
> +	if {$recursive eq {true}} {
> +		destroy $w_body
> +		set o_cons [console::embed \
> +			$w_body \
> +			[mc "Cloning submodules"]]
> +		pack $w_body -fill both -expand 1 -padx 10
> +		$o_cons exec \
> +			[list git submodule update --init --recursive] \
> +			[cb _do_validate_submodule_cloning]
> +	} else {
> +		set done 1
> +	}
> +}
> +
>  method _readtree_wait {fd} {
>  	set buf [read $fd]
>  	$o_cons update_meter $buf
> @@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
>  		fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
>  		fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
>  	} else {
> -		set done 1
> +		_do_clone_submodules $this
>  	}
>  }
>  
> @@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
>  			hook_failed_popup post-checkout $pch_error 0
>  		}
>  		unset pch_error
> -		set done 1
> +		_do_clone_submodules $this
>  		return
>  	}
>  	fconfigure $fd_ph -blocking 0
> 

Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Henri GEIST]

[-- Attachment #1: Type: text/plain, Size: 5179 bytes --]

Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
> Am 05.03.2014 00:01, schrieb Henri GEIST:
> > Permit to do a 'git clone --recursive' through git-gui.
> 
> I really like where this is heading!
> 
> Some minor issues:
> 
> - I think we should be more verbose in the commit message,
>   including that and why the default should be "on". Maybe
>   like this?
> 
>   "Permit to do a 'git clone --recursive' through git-gui.
>   Add a 'recursive' checkbox in the clone menu which allows
>   users to clone a repository and all its submodules in one
>   go (unless the 'update' flag is set to "none" in the
>   .gitmodules file for a submodule, in that case that
>   specific submodule is not cloned automatically).
> 
>   Enable this new option per default, as most users want to
>   clone all submodules too when cloning the superproject
>   (This is currently not possible without leaving git gui
>   or adding a custom tool entry for that)."
> 

Ok for me.

> 
> - I'd rather change the button text from "Recursive (For
>   submodules)" to something like "Recursively clone
>   submodules too" or such.
> 

Perfect

> 
> - Wouldn't it be easier to pass the '--recurse-submodules"
>   option to the "git clone" call for the superproject instead
>   of adding the _do_clone_submodules() function doing a
>   subsequent "git submodule update --init --recursive"? That
>   is also be more future proof with respect to the autoclone
>   config option we have in mind (which would add that behavior
>   for "git clone" itself, making the call you added redundant).
> 

That is what I planned to do at beginning.
But git-gui never call git clone anywhere.
It make the clone step by step with a long and complicated list of
commands just like a Tcl rewrite of git-clone.
Have a look on the function _do_clone2 in choose_repository.tcl.

As I suspect there should be a good reason for this that I did not
understand I have choose to not refactoring it.
And in fact looking in the code 'git clone --recursive' do nothing
else than calling 'git submodule update --init --recursive' like I
have done to complete this rewrite of 'git-clone'.


> 
> > Signed-off-by: Henri GEIST <geist.henri@laposte.net>
> > ---
> > I have set the default checkbox state to 'true' by default has all my gui users
> > use it all the time this way.
> > But as it change the default behavior you may prefer to set it to 'false' by
> > default.
> > 
> >  git-gui/lib/choose_repository.tcl |   34 ++++++++++++++++++++++++++++++++--
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/git-gui/lib/choose_repository.tcl b/git-gui/lib/choose_repository.tcl
> > index 3c10bc6..47d436b 100644
> > --- a/git-gui/lib/choose_repository.tcl
> > +++ b/git-gui/lib/choose_repository.tcl
> > @@ -18,6 +18,7 @@ field local_path       {} ; # Where this repository is locally
> >  field origin_url       {} ; # Where we are cloning from
> >  field origin_name  origin ; # What we shall call 'origin'
> >  field clone_type hardlink ; # Type of clone to construct
> > +field recursive      true ; # Recursive cloning flag
> >  field readtree_err        ; # Error output from read-tree (if any)
> >  field sorted_recent       ; # recent repositories (sorted)
> >  
> > @@ -525,6 +526,11 @@ method _do_clone {} {
> >  	foreach r $w_types {
> >  		pack $r -anchor w
> >  	}
> > +	${NS}::checkbutton $args.type_f.recursive \
> > +		-text [mc "Recursive (For submodules)"] \
> > +		-variable @recursive \
> > +		-onvalue true -offvalue false
> > +	pack $args.type_f.recursive
> >  	grid $args.type_l $args.type_f -sticky new
> >  
> >  	grid columnconfigure $args 1 -weight 1
> > @@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
> >  	fileevent $fd readable [cb _readtree_wait $fd]
> >  }
> >  
> > +method _do_validate_submodule_cloning {ok} {
> > +	if {$ok} {
> > +		$o_cons done $ok
> > +		set done 1
> > +	} else {
> > +		_clone_failed $this [mc "Cannot clone submodules."]
> > +	}
> > +}
> > +
> > +method _do_clone_submodules {} {
> > +	if {$recursive eq {true}} {
> > +		destroy $w_body
> > +		set o_cons [console::embed \
> > +			$w_body \
> > +			[mc "Cloning submodules"]]
> > +		pack $w_body -fill both -expand 1 -padx 10
> > +		$o_cons exec \
> > +			[list git submodule update --init --recursive] \
> > +			[cb _do_validate_submodule_cloning]
> > +	} else {
> > +		set done 1
> > +	}
> > +}
> > +
> >  method _readtree_wait {fd} {
> >  	set buf [read $fd]
> >  	$o_cons update_meter $buf
> > @@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
> >  		fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
> >  		fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
> >  	} else {
> > -		set done 1
> > +		_do_clone_submodules $this
> >  	}
> >  }
> >  
> > @@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
> >  			hook_failed_popup post-checkout $pch_error 0
> >  		}
> >  		unset pch_error
> > -		set done 1
> > +		_do_clone_submodules $this
> >  		return
> >  	}
> >  	fconfigure $fd_ph -blocking 0
> > 
> 



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Jens Lehmann]

Am 06.03.2014 01:15, schrieb Henri GEIST:
> Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
>> Am 05.03.2014 00:01, schrieb Henri GEIST:
>> - Wouldn't it be easier to pass the '--recurse-submodules"
>>   option to the "git clone" call for the superproject instead
>>   of adding the _do_clone_submodules() function doing a
>>   subsequent "git submodule update --init --recursive"? That
>>   is also be more future proof with respect to the autoclone
>>   config option we have in mind (which would add that behavior
>>   for "git clone" itself, making the call you added redundant).
> 
> That is what I planned to do at beginning.
> But git-gui never call git clone anywhere.
> It make the clone step by step with a long and complicated list of
> commands just like a Tcl rewrite of git-clone.
> Have a look on the function _do_clone2 in choose_repository.tcl.

You're right, it does fetch followed by read-tree ... so my
proposal doesn't make much sense here, sorry for bothering you
without checking the source first.

> As I suspect there should be a good reason for this that I did not
> understand I have choose to not refactoring it.

That makes sense. Shawn, could you shed some light on why clone
is coded again using plumbing in git gui instead of just calling
the clone command?

> And in fact looking in the code 'git clone --recursive' do nothing
> else than calling 'git submodule update --init --recursive' like I
> have done to complete this rewrite of 'git-clone'.

Yep. Note to self: Port everything we add to clone to git gui too.

Re: Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Heiko Voigt]

On Thu, Mar 06, 2014 at 08:35:48PM +0100, Jens Lehmann wrote:
> Am 06.03.2014 01:15, schrieb Henri GEIST:
> > Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
> >> Am 05.03.2014 00:01, schrieb Henri GEIST:
> >> - Wouldn't it be easier to pass the '--recurse-submodules"
> >>   option to the "git clone" call for the superproject instead
> >>   of adding the _do_clone_submodules() function doing a
> >>   subsequent "git submodule update --init --recursive"? That
> >>   is also be more future proof with respect to the autoclone
> >>   config option we have in mind (which would add that behavior
> >>   for "git clone" itself, making the call you added redundant).
> > 
> > That is what I planned to do at beginning.
> > But git-gui never call git clone anywhere.
> > It make the clone step by step with a long and complicated list of
> > commands just like a Tcl rewrite of git-clone.
> > Have a look on the function _do_clone2 in choose_repository.tcl.
> 
> You're right, it does fetch followed by read-tree ... so my
> proposal doesn't make much sense here, sorry for bothering you
> without checking the source first.
> 
> > As I suspect there should be a good reason for this that I did not
> > understand I have choose to not refactoring it.
> 
> That makes sense. Shawn, could you shed some light on why clone
> is coded again using plumbing in git gui instead of just calling
> the clone command?

I think because git gui is using plumbing everywhere, it is supposed to
be just another porcelain. And I guess that was an intended decision
because porcelain might change its output and break git gui. At least
thats what I inferred.

Cheers Heiko

Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Henri GEIST]

[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]

Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
> Am 05.03.2014 00:01, schrieb Henri GEIST:
> > Permit to do a 'git clone --recursive' through git-gui.
> 
> I really like where this is heading!
> 
> Some minor issues:
> 
> - I think we should be more verbose in the commit message,
>   including that and why the default should be "on". Maybe
>   like this?
> 
>   "Permit to do a 'git clone --recursive' through git-gui.
>   Add a 'recursive' checkbox in the clone menu which allows
>   users to clone a repository and all its submodules in one
>   go (unless the 'update' flag is set to "none" in the
>   .gitmodules file for a submodule, in that case that
>   specific submodule is not cloned automatically).
> 
>   Enable this new option per default, as most users want to
>   clone all submodules too when cloning the superproject
>   (This is currently not possible without leaving git gui
>   or adding a custom tool entry for that)."
> 
> 
> - I'd rather change the button text from "Recursive (For
>   submodules)" to something like "Recursively clone
>   submodules too" or such.
> 
>

Perfect.
Would you like me to send the new version of the patch in this thread
Or to make a new thread [patch v2] ?



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 230 bytes --]

Re: [PATCH/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Jens Lehmann]

Am 11.03.2014 12:07, schrieb Henri GEIST:
> Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
>> Am 05.03.2014 00:01, schrieb Henri GEIST:
>>> Permit to do a 'git clone --recursive' through git-gui.
>>
>> I really like where this is heading!
>>
>> Some minor issues:
>>
>> - I think we should be more verbose in the commit message,
>>   including that and why the default should be "on". Maybe
>>   like this?
>>
>>   "Permit to do a 'git clone --recursive' through git-gui.
>>   Add a 'recursive' checkbox in the clone menu which allows
>>   users to clone a repository and all its submodules in one
>>   go (unless the 'update' flag is set to "none" in the
>>   .gitmodules file for a submodule, in that case that
>>   specific submodule is not cloned automatically).
>>
>>   Enable this new option per default, as most users want to
>>   clone all submodules too when cloning the superproject
>>   (This is currently not possible without leaving git gui
>>   or adding a custom tool entry for that)."
>>
>>
>> - I'd rather change the button text from "Recursive (For
>>   submodules)" to something like "Recursively clone
>>   submodules too" or such.
>>
>>
> 
> Perfect.
> Would you like me to send the new version of the patch in this thread
> Or to make a new thread [patch v2] ?

It doesn't matter that much as long as you start the subject with
"[PATCH v2]". But I believe you should send it to Pat and create
the diff from inside the git-gui directory so he can simply apply
it to his repository.