Git development
 help / color / mirror / Atom feed
* RE: [PATCH] Re: Gitk --all error when there are more than 797 refs in a repository
From: Murphy, John @ 2009-09-21 14:02 UTC (permalink / raw)
  To: Paul Mackerras, Pat Thoyts, Johannes Sixt; +Cc: git
In-Reply-To: <19124.8378.975976.347711@cargo.ozlabs.ibm.com>

Johannes Sixt writes:

> I cannot reproduce the error. I have a repository with 100 commits in
a
> linear history and 5000 refs (50 refs per commit). They are named
> refs/heads/branch-XXXX. I don't see any problems with 'gitk --all'.

That still leave you with only 100 unique refs.
You need over 797 unique refs.

> The error must be in some other git invocation.

I put many debug pop-ups around the code and I believe that this the
call that is dying on.
This is the only part of the code that has the error text "fatal:
ambiguous argument".


Paul Mackerras writes:

> I knew about the 32k command-line limit under windows, but I don't see
> how that applies in this case unless it is $revs that is too long (and
> if that is the case then I don't see how your patch helps).  Is there
> also a 32k limit on the size of data returned by a command executed
> with [exec]?

In this case $revs is "--all"

I believe what I am experiencing is a 32K limit with [exec]

Additional info:
My repo has: 17,737 commits

^ permalink raw reply

* Re: [PATCH] pre-commit.sample: don't use [...] around a tr range
From: Jeff King @ 2009-09-21 13:44 UTC (permalink / raw)
  To: Jim Meyering; +Cc: Alex Riesen, git list
In-Reply-To: <87ocp4imul.fsf@meyering.net>

On Mon, Sep 21, 2009 at 01:00:34PM +0200, Jim Meyering wrote:

> > We have (had?) people trying to support Git on HP-UX and SunOS.
> > Do these count?
> 
> I had my doubts, but have just confirmed that Solaris 10's
> /usr/bin/tr is still doing it the SYSV way:
> 
>     $ echo foo | LC_ALL=C /usr/bin/tr a-z A-Z
>     foo
> 
> There, you have to use /usr/xpg4/bin/tr to get the expected behavior:
> 
>     $ echo foo | LC_ALL=C /usr/xpg4/bin/tr a-z A-Z
>     FOO
> 
> So you're right.  Thanks!

See:

  http://article.gmane.org/gmane.comp.version-control.git/76991

> +	# Note that the use of brackets around a tr range is ok here, (it's
> +	# even required, for portability to Solaris 10's /usr/bin/tr), since
> +	# the square bracket bytes happen to fall in the designated range.
>  	test "$(git diff --cached --name-only --diff-filter=A -z |
>  	  LC_ALL=C tr -d '[ -~]\0')"

Does this work on non-bracket systems? I would think that enumerating
the sequence would be the most portable thing.

-Peff

^ permalink raw reply

* [RFC PATCH 7/8] Add documentation for git gui variables for email and delete after push
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

---
 Documentation/config.txt |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c6f09f8..4174ab4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -957,6 +957,36 @@ gui.blamehistoryctx::
 	Context` menu item is invoked from 'git gui blame'. If this
 	variable is set to zero, the whole history is shown.
 
+gui.emailafterpush::
+	The push dialog can compose a review email if you push a feature
+	branch. If this option is set to true the checkbox will be
+	automatically enabled when you select a feature branch. The
+	upstream (main) branch is determined from the
+	`gui.upstreambranch` option.
+
+gui.deleteafterpush::
+	If you push a branch in git gui you can enable a checkbox which
+	will open the remote delete branch dialog after a push. Setting
+	this variable to true will automatically check that checkbox
+	when you select to push the upstream (main) branch. The upstream
+	branch can be configured using the `gui.upstreambranch` configuration
+	variable.
+
+gui.upstreambranch::
+	This configures which branchname will be referred to as the
+	upstream (main) branch on remote repositories. Currently this
+	option is used in combination with the options
+	`gui.emailafterpush` and `gui.deleteafterpush`.
+
+gui.fetchonstartup::
+	Boolean, if set all branches from remote determined by
+	`gui.defaulremote` will be fetched on startup of git gui.
+
+gui.defaultremote::
+	String variable which determines from which remote git gui will
+	automatically start a fetch on startup. If this variable is not
+	set the automatic fetch is disabled.
+
 guitool.<name>.cmd::
 	Specifies the shell command line to execute when the corresponding item
 	of the linkgit:git-gui[1] `Tools` menu is invoked. This option is
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 6/8] git gui: add "review topic" menuitem which supports feature branch workflow
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

We have a setup where each developer publishes feature branches on a
central server. These branches then become merged into the upstream
branch (master in most times) by another developer designated as the
reviewer.

The review process usually involves several steps (fetch remote branches,
update local upstream branch to server branch, review in gitk, merge).
To combine this operation into one step we add a new menu item "Review
Topic" under the merge menu which guides the user through all the steps.

After "Review Topic" has finished the only thing left to do is to push
the current branch (which is the upstream branch) and delete the feature
branch. Support for which has been implemented in an ealier patch in
this series.
---
 git-gui/git-gui.sh    |    4 ++
 git-gui/lib/merge.tcl |   83 ++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 2a2bd38..3e3c7cd 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2557,6 +2557,10 @@ if {[is_enabled branch]} {
 		-accelerator $M1T-M
 	lappend disable_on_lock \
 		[list .mbar.merge entryconf [.mbar.merge index last] -state]
+	.mbar.merge add command -label [mc "Review Topic..."] \
+		-command {merge::dialog review}
+	lappend disable_on_lock \
+		[list .mbar.merge entryconf [.mbar.merge index last] -state]
 	.mbar.merge add command -label [mc "Abort Merge..."] \
 		-command merge::reset_hard
 	lappend disable_on_lock \
diff --git a/git-gui/lib/merge.tcl b/git-gui/lib/merge.tcl
index 283e491..3716cf3 100644
--- a/git-gui/lib/merge.tcl
+++ b/git-gui/lib/merge.tcl
@@ -81,14 +81,70 @@ method _visualize {} {
 	}
 }
 
+method _update_upstream {} {
+	global repo_config index_lock_type
+
+	set upstream_branch $repo_config(gui.upstreambranch)
+	set remote $repo_config(gui.defaultremote)
+
+	if {$upstream_branch eq {} || $remote eq {}} {
+		info_popup [mc "You need to set the configuration \
+			variables gui.upstreambranch and gui.defaultremote so \
+			review topic can work"]
+		return 0
+	}
+
+	#TODO: find out why this does not return on failure
+	if {![fetch_from $remote {close_on_success}]} {
+		return 0
+	}
+
+	set co [::checkout_op::new \
+		$remote/$upstream_branch \
+		{} \
+		refs/heads/$upstream_branch]
+
+	#TODO:
+	#$co parent $w
+	$co enable_create   1
+	$co enable_merge    ff
+	$co enable_checkout 1
+
+	set spec [list refs/remotes/$remote/$upstream_branch \
+	                       $remote \
+			       refs/heads/$upstream_branch]
+
+	$co enable_fetch $spec
+	$co remote_source $spec
+
+	if {[$co run]} {
+		while {$index_lock_type ne {none}} {
+			vwait index_lock_type
+		}
+		return 1
+	} else {
+		while {$index_lock_type ne {none}} {
+			vwait index_lock_type
+		}
+		return 0
+	}
+}
+
 method _start {} {
-	global HEAD current_branch remote_url
+	global HEAD current_branch remote_url merge_type
 
 	set name [_rev $this]
 	if {$name eq {}} {
 		return
 	}
 
+	if {$merge_type eq "review"} {
+		_visualize $this
+		$w.buttons.merge configure -text [mc Merge]
+		set merge_type normal
+		return
+	}
+
 	set spec [$w_rev get_tracking_branch]
 	set cmit [$w_rev get_commit]
 
@@ -137,17 +193,34 @@ method _finish {cons ok} {
 	delete_this
 }
 
-constructor dialog {} {
+constructor dialog {{dialog_type normal}} {
 	global current_branch
 	global M1B
+	global merge_type
+
+	set merge_type $dialog_type
+	set start_button [mc Merge]
+	set action_name "Merge Into"
+	set dialog_title [mc "Merge"]
+
+	if { $merge_type eq "review" } {
+		if {![_update_upstream $this]} {
+			delete_this
+			return
+		}
+		set start_button [mc Review]
+		set action_name [mc "Review for Merge Into"]
+		set dialog_title [mc Review]
+	}
 
 	if {![_can_merge $this]} {
+		info_popup failed_can_merge
 		delete_this
 		return
 	}
 
 	make_toplevel top w
-	wm title $top [append "[appname] ([reponame]): " [mc "Merge"]]
+	wm title $top [append "[appname] ([reponame]): " $dialog_title]
 	if {$top ne {.}} {
 		wm geometry $top "+[winfo rootx .]+[winfo rooty .]"
 	}
@@ -155,7 +228,7 @@ constructor dialog {} {
 	set _start [cb _start]
 
 	label $w.header \
-		-text [mc "Merge Into %s" $current_branch] \
+		-text [mc "$action_name %s" $current_branch] \
 		-font font_uibold
 	pack $w.header -side top -fill x
 
@@ -165,7 +238,7 @@ constructor dialog {} {
 		-command [cb _visualize]
 	pack $w.buttons.visualize -side left
 	button $w.buttons.merge \
-		-text [mc Merge] \
+		-text $start_button \
 		-command $_start
 	pack $w.buttons.merge -side right
 	button $w.buttons.cancel \
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 8/8] git gui: make gc hint and fetch on startup sequential
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

On large repositories (especially on Windows) doing any harddrive usage
during the initial status scan slows the whole process down. So this
postpones the question for compressing loose objects and fetching of the
default remote until after that.
---
 git-gui/git-gui.sh |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 3e3c7cd..19b1ad6 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -2031,8 +2031,22 @@ proc do_quit {{rc {1}}} {
 	destroy .
 }
 
+proc ui_finalize_startup {} {
+	global repo_config
+
+	set defaultremote $repo_config(gui.defaultremote)
+	if {[is_config_true gui.fetchonstartup] && $defaultremote ne {}} {
+		fetch_from $defaultremote {close_on_success}
+	}
+
+	if {[is_enabled multicommit]} {
+		hint_gc
+	}
+	ui_ready
+}
+
 proc do_rescan {} {
-	rescan ui_ready
+	rescan ui_finalize_startup
 }
 
 proc ui_do_rescan {} {
@@ -3502,14 +3516,6 @@ after 1 {
 	}
 }
 
-set defaultremote $repo_config(gui.defaultremote)
-if {[is_config_true gui.fetchonstartup] && $defaultremote ne {}} {
-	after 500 {fetch_from $defaultremote {close_on_success}}
-}
-
-if {[is_enabled multicommit]} {
-	after 1000 hint_gc
-}
 if {[is_enabled retcode]} {
 	bind . <Destroy> {+terminate_me %W}
 }
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 5/8] git gui: do fetch from a default remote on startup
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

In many situations the very first thing you do after starting git gui is
to fetch the newest changes from your main remote. Lets have git gui do
the work automatically and save us some clicking.

We add two configuration variables (one to enable the feature and one
specifying the default remote) so the remote can be reused in another
patch which adds a "Review Topic" merge item later in this series.
---
 git-gui/git-gui.sh     |    8 ++++++++
 git-gui/lib/option.tcl |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index b32b8e7..2a2bd38 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -740,6 +740,8 @@ set default_config(gui.fastcopyblame) false
 set default_config(gui.emailafterpush) false
 set default_config(gui.deleteafterpush) false
 set default_config(gui.upstreambranch) {master}
+set default_config(gui.defaultremote) {origin}
+set default_config(gui.fetchonstartup) false
 set default_config(gui.copyblamethreshold) 40
 set default_config(gui.blamehistoryctx) 7
 set default_config(gui.diffcontext) 5
@@ -3495,6 +3497,12 @@ after 1 {
 		$ui_comm configure -state disabled -background gray
 	}
 }
+
+set defaultremote $repo_config(gui.defaultremote)
+if {[is_config_true gui.fetchonstartup] && $defaultremote ne {}} {
+	after 500 {fetch_from $defaultremote {close_on_success}}
+}
+
 if {[is_enabled multicommit]} {
 	after 1000 hint_gc
 }
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index 6bfe343..defeb54 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -149,7 +149,9 @@ proc do_options {} {
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
 		{b gui.emailafterpush {mc "Open Email Composer after Push of Feature Branch"}}
 		{b gui.deleteafterpush {mc "Open Remote Delete Dialog On Push of Upstream Branch"}}
+		{b gui.fetchonstartup {mc "Fetch default remote on startup"}}
 		{t gui.upstreambranch {mc "Upstream branch"}}
+		{t gui.defaultremote {mc "Default remote repository"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..300 gui.blamehistoryctx {mc "Blame History Context Radius (days)"}}
 		{i-1..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 1/8] git gui: add a "compose review email" checkbox to push dialog
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

This is useful when working with a central server. If a developer wants
to send his changes to a reviewer we can help him fill out the necessary
information about the branch he has pushed and launch his default email
composer.

Composition of such an email is currently only allowed for pushing one
branch because the subject line size is limited.
---
 git-gui/git-gui.sh        |    2 +
 git-gui/lib/option.tcl    |    2 +
 git-gui/lib/transport.tcl |   75 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 3c0ce26..dfc1652 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -737,6 +737,8 @@ set default_config(gui.matchtrackingbranch) false
 set default_config(gui.pruneduringfetch) false
 set default_config(gui.trustmtime) false
 set default_config(gui.fastcopyblame) false
+set default_config(gui.emailafterpush) false
+set default_config(gui.upstreambranch) {master}
 set default_config(gui.copyblamethreshold) 40
 set default_config(gui.blamehistoryctx) 7
 set default_config(gui.diffcontext) 5
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index 1d55b49..55ecfe6 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -147,6 +147,8 @@ proc do_options {} {
 		{b gui.pruneduringfetch {mc "Prune Tracking Branches During Fetch"}}
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
+		{b gui.emailafterpush {mc "Open Email Composer after Push of Feature Branch"}}
+		{t gui.upstreambranch {mc "Upstream branch"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..300 gui.blamehistoryctx {mc "Blame History Context Radius (days)"}}
 		{i-1..99 gui.diffcontext {mc "Number of Diff Context Lines"}}
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index b18d9c7..77a2f46 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -30,10 +30,63 @@ proc push_to {remote} {
 	console::exec $w $cmd
 }
 
+proc compose_email {to subject} {
+	set mail_link mailto:
+	append mail_link $to "?subject=" $subject
+	start_browser $mail_link
+}
+
+proc after_push_anywhere_action {cons ok} {
+	global push_email r_url branches
+
+	console::done $cons $ok
+
+	if {$ok} {
+		if {$push_email} {
+			set rconfig_var "remote.$r_url.url"
+			set remote_url [exec git config $rconfig_var]
+			set remote_short_name [lindex [split $remote_url / ] end ]
+			set remote_short_name [lindex [split $remote_short_name . ] 0 ]
+			# TODO: create a configuration variable for the
+			# subject
+			set email_subject "Review%20request:%20$remote_short_name:$branches"
+			compose_email {} $email_subject
+		}
+	}
+}
+
+proc do_push_on_change_branch {w} {
+	global upstream_branch
+
+	$w.options.email select
+
+	set cnt 0
+	set b {}
+	foreach i [$w.source.l curselection] {
+		set b [$w.source.l get $i]
+		incr cnt
+	}
+
+	if {$cnt != 1} {
+		$w.options.email deselect
+		$w.options.email configure -state disabled
+		return
+	}
+
+	$w.options.email configure -state normal
+
+	if { $b eq $upstream_branch } {
+		if {[is_config_true gui.emailafterpush]} {
+			$w.options.email deselect
+		}
+	}
+}
+
 proc start_push_anywhere_action {w} {
 	global push_urltype push_remote push_url push_thin push_tags
 	global push_force
 	global repo_config
+	global r_url branches
 
 	set is_mirror 0
 	set r_url {}
@@ -64,9 +117,11 @@ proc start_push_anywhere_action {w} {
 			[mc "Mirroring to %s" $r_url]]
 	} else {
 		set cnt 0
+		set branches {}
 		foreach i [$w.source.l curselection] {
 			set b [$w.source.l get $i]
 			lappend cmd "refs/heads/$b:refs/heads/$b"
+			lappend branches $b
 			incr cnt
 		}
 		if {$cnt == 0} {
@@ -81,7 +136,7 @@ proc start_push_anywhere_action {w} {
 			[mc "push %s" $r_url] \
 			[mc "Pushing %s %s to %s" $cnt $unit $r_url]]
 	}
-	console::exec $cons $cmd
+	console::exec $cons $cmd "after_push_anywhere_action $cons"
 	destroy $w
 }
 
@@ -91,7 +146,8 @@ trace add variable push_remote write \
 proc do_push_anywhere {} {
 	global all_remotes current_branch
 	global push_urltype push_remote push_url push_thin push_tags
-	global push_force
+	global push_force push_email upstream_branch
+	global repo_config
 
 	set w .push_setup
 	toplevel $w
@@ -117,6 +173,9 @@ proc do_push_anywhere {} {
 		-width 70 \
 		-selectmode extended \
 		-yscrollcommand [list $w.source.sby set]
+	if {[is_config_true gui.emailafterpush]} {
+		bind $w.source.l <ButtonRelease-1> [list do_push_on_change_branch $w]
+	}
 	foreach h [load_all_heads] {
 		$w.source.l insert end $h
 		if {$h eq $current_branch} {
@@ -179,6 +238,11 @@ proc do_push_anywhere {} {
 		-text [mc "Include tags"] \
 		-variable push_tags
 	grid $w.options.tags -columnspan 2 -sticky w
+	checkbutton $w.options.email \
+		-text [mc "Compose email with review request"] \
+		-variable push_email
+	grid $w.options.email -columnspan 2 -sticky w
+	$w.options.email deselect
 	grid columnconfigure $w.options 1 -weight 1
 	pack $w.options -anchor nw -fill x -pady 5 -padx 5
 
@@ -186,6 +250,13 @@ proc do_push_anywhere {} {
 	set push_force 0
 	set push_thin 0
 	set push_tags 0
+	set push_email 0
+
+	set upstream_branch $repo_config(gui.upstreambranch)
+
+	if {[is_config_true gui.emailafterpush]} {
+		do_push_on_change_branch $w
+	}
 
 	bind $w <Visibility> "grab $w; focus $w.buttons.create"
 	bind $w <Key-Escape> "destroy $w"
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 4/8] git gui: teach fetch operation to close dialog on successful completion
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

---
 git-gui/lib/console.tcl   |    6 ++++++
 git-gui/lib/transport.tcl |   12 ++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/console.tcl b/git-gui/lib/console.tcl
index c112464..84eef13 100644
--- a/git-gui/lib/console.tcl
+++ b/git-gui/lib/console.tcl
@@ -168,6 +168,7 @@ method chain {cmdlist {ok 1}} {
 	} else {
 		done $this $ok
 	}
+	return $ok
 }
 
 method insert {txt} {
@@ -178,6 +179,11 @@ method insert {txt} {
 	$w_t conf -state disabled
 }
 
+method close_window {} {
+	wm protocol $w WM_DELETE_WINDOW {}
+	destroy $w
+}
+
 method done {ok} {
 	if {$ok} {
 		if {[winfo exists $w.m.s]} {
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index d717468..39443ab 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -1,7 +1,7 @@
 # git-gui transport (fetch/push) support
 # Copyright (C) 2006, 2007 Shawn Pearce
 
-proc fetch_from {remote} {
+proc fetch_from {remote {close_after {}}} {
 	set w [console::new \
 		[mc "fetch %s" $remote] \
 		[mc "Fetching new changes from %s" $remote]]
@@ -10,7 +10,15 @@ proc fetch_from {remote} {
 	if {[is_config_true gui.pruneduringfetch]} {
 		lappend cmds [list exec git remote prune $remote]
 	}
-	console::chain $w $cmds
+	set ok [console::chain $w $cmds]
+
+	if {$ok} {
+		if {$close_after ne {}} {
+			console::close_window $w
+		}
+		return 1
+	}
+	return 0
 }
 
 proc prune_from {remote} {
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 0/8] Extend git gui for better "centralized" workflow integration
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce

This series is adding some more support for a centralized workflow with
code review based on feature branches. Most of it is plugging existing
functionality together so the most typical steps can be done at once.

In our team at $dayjob we use a workflow were each developer creates
feature branches and once he is satified with his change he sends out a
review request to another developer (the reviewer) which is most
familiar with the code parts involved in the change. If the reviewer
likes the change he merges it into the upstream branch (master). He then
publishes the new master and deletes the feature branch. Thats a quick
description of the workflow I am trying to add more support for with
this series.

There are still some TODO markers in there and if someone can help me
with those I would be very glad. This is the first time I am doing some
more extensive programming in tcl/tk and git gui so I hope you do not
tear everything apart ;)

The last patch is not really about the same topic but since it currently
relies on the changes of this series I included it nevertheless.

cheers Heiko


Heiko Voigt (8):
  git gui: add a "compose review email" checkbox to push dialog
  git gui: teach delete dialog to open with a default remote
  git gui: add checkbox to launch remote branch delete after push
  git gui: teach fetch operation to close dialog on successful
    completion
  git gui: do fetch from a default remote on startup
  git gui: add "review topic" menuitem which supports feature branch
    workflow
  Add documentation for git gui variables for email and delete after
    push
  git gui: make gc hint and fetch on startup sequential

 Documentation/config.txt             |   30 ++++++++++
 git-gui/git-gui.sh                   |   29 ++++++++-
 git-gui/lib/console.tcl              |    6 ++
 git-gui/lib/merge.tcl                |   83 ++++++++++++++++++++++++--
 git-gui/lib/option.tcl               |    5 ++
 git-gui/lib/remote_branch_delete.tcl |    6 +-
 git-gui/lib/transport.tcl            |  107 ++++++++++++++++++++++++++++++++-
 7 files changed, 250 insertions(+), 16 deletions(-)

^ permalink raw reply

* [RFC PATCH 3/8] git gui: add checkbox to launch remote branch delete after push
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

This is useful when working with a central server and feature branches.
After a reviewer merged a feature branch into the upstream branch he
usually wants to delete this branch on the server after pushing the
upstream branch.

Save some clicks and open the delete dialog after pushing the upstream
branch.
---
 git-gui/git-gui.sh        |    1 +
 git-gui/lib/option.tcl    |    1 +
 git-gui/lib/transport.tcl |   28 ++++++++++++++++++++++++----
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index dfc1652..b32b8e7 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -738,6 +738,7 @@ set default_config(gui.pruneduringfetch) false
 set default_config(gui.trustmtime) false
 set default_config(gui.fastcopyblame) false
 set default_config(gui.emailafterpush) false
+set default_config(gui.deleteafterpush) false
 set default_config(gui.upstreambranch) {master}
 set default_config(gui.copyblamethreshold) 40
 set default_config(gui.blamehistoryctx) 7
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index 55ecfe6..6bfe343 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -148,6 +148,7 @@ proc do_options {} {
 		{b gui.matchtrackingbranch {mc "Match Tracking Branches"}}
 		{b gui.fastcopyblame {mc "Blame Copy Only On Changed Files"}}
 		{b gui.emailafterpush {mc "Open Email Composer after Push of Feature Branch"}}
+		{b gui.deleteafterpush {mc "Open Remote Delete Dialog On Push of Upstream Branch"}}
 		{t gui.upstreambranch {mc "Upstream branch"}}
 		{i-20..200 gui.copyblamethreshold {mc "Minimum Letters To Blame Copy On"}}
 		{i-0..300 gui.blamehistoryctx {mc "Blame History Context Radius (days)"}}
diff --git a/git-gui/lib/transport.tcl b/git-gui/lib/transport.tcl
index 77a2f46..d717468 100644
--- a/git-gui/lib/transport.tcl
+++ b/git-gui/lib/transport.tcl
@@ -37,7 +37,7 @@ proc compose_email {to subject} {
 }
 
 proc after_push_anywhere_action {cons ok} {
-	global push_email r_url branches
+	global push_email delete_after_push r_url branches
 
 	console::done $cons $ok
 
@@ -52,13 +52,21 @@ proc after_push_anywhere_action {cons ok} {
 			set email_subject "Review%20request:%20$remote_short_name:$branches"
 			compose_email {} $email_subject
 		}
+		if {$delete_after_push} {
+			remote_branch_delete::dialog $r_url
+		}
 	}
 }
 
 proc do_push_on_change_branch {w} {
 	global upstream_branch
 
-	$w.options.email select
+	if {[is_config_true gui.emailafterpush]} {
+		$w.options.email select
+	}
+	if {[is_config_true gui.deleteafterpush]} {
+		$w.options.delete_after_push deselect
+	}
 
 	set cnt 0
 	set b {}
@@ -70,6 +78,7 @@ proc do_push_on_change_branch {w} {
 	if {$cnt != 1} {
 		$w.options.email deselect
 		$w.options.email configure -state disabled
+		$w.options.delete_after_push deselect
 		return
 	}
 
@@ -79,6 +88,9 @@ proc do_push_on_change_branch {w} {
 		if {[is_config_true gui.emailafterpush]} {
 			$w.options.email deselect
 		}
+		if {[is_config_true gui.deleteafterpush]} {
+			$w.options.delete_after_push select
+		}
 	}
 }
 
@@ -173,7 +185,8 @@ proc do_push_anywhere {} {
 		-width 70 \
 		-selectmode extended \
 		-yscrollcommand [list $w.source.sby set]
-	if {[is_config_true gui.emailafterpush]} {
+	if {[is_config_true gui.emailafterpush] \
+	    || [is_config_true gui.deleteafterpush]} {
 		bind $w.source.l <ButtonRelease-1> [list do_push_on_change_branch $w]
 	}
 	foreach h [load_all_heads] {
@@ -243,6 +256,11 @@ proc do_push_anywhere {} {
 		-variable push_email
 	grid $w.options.email -columnspan 2 -sticky w
 	$w.options.email deselect
+	checkbutton $w.options.delete_after_push \
+		-text [mc "Launch delete dialog after push"] \
+		-variable delete_after_push
+	grid $w.options.delete_after_push -columnspan 2 -sticky w
+	$w.options.delete_after_push deselect
 	grid columnconfigure $w.options 1 -weight 1
 	pack $w.options -anchor nw -fill x -pady 5 -padx 5
 
@@ -251,10 +269,12 @@ proc do_push_anywhere {} {
 	set push_thin 0
 	set push_tags 0
 	set push_email 0
+	set delete_after_push 0
 
 	set upstream_branch $repo_config(gui.upstreambranch)
 
-	if {[is_config_true gui.emailafterpush]} {
+	if {[is_config_true gui.emailafterpush] \
+	    || [is_config_true gui.deleteafterpush]} {
 		do_push_on_change_branch $w
 	}
 
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* [RFC PATCH 2/8] git gui: teach delete dialog to open with a default remote
From: Heiko Voigt @ 2009-09-21 13:06 UTC (permalink / raw)
  To: git; +Cc: spearce
In-Reply-To: <cover.1253481338.git.hvoigt@hvoigt.net>

This is useful if you want to open the delete dialog with a preselected
remote. We use this in the next patch from this series.
---
 git-gui/lib/remote_branch_delete.tcl |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-gui/lib/remote_branch_delete.tcl b/git-gui/lib/remote_branch_delete.tcl
index 2416420..e0dbc25 100644
--- a/git-gui/lib/remote_branch_delete.tcl
+++ b/git-gui/lib/remote_branch_delete.tcl
@@ -22,7 +22,7 @@ field head_cache
 field full_cache
 field cached
 
-constructor dialog {} {
+constructor dialog {{initial_remote origin}} {
 	global all_remotes M1B
 
 	make_toplevel top w
@@ -52,8 +52,8 @@ constructor dialog {} {
 			-variable @urltype
 		eval tk_optionMenu $w.dest.remote_m @remote $all_remotes
 		grid $w.dest.remote_r $w.dest.remote_m -sticky w
-		if {[lsearch -sorted -exact $all_remotes origin] != -1} {
-			set remote origin
+		if {[lsearch -sorted -exact $all_remotes $initial_remote] != -1} {
+			set remote $initial_remote
 		} else {
 			set remote [lindex $all_remotes 0]
 		}
-- 
1.6.5.rc1.12.gc72fe

^ permalink raw reply related

* Re: [PATCH] pre-commit.sample: don't use [...] around a tr range
From: Alex Riesen @ 2009-09-21 12:45 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git list
In-Reply-To: <87ocp4imul.fsf@meyering.net>

On Mon, Sep 21, 2009 at 13:00, Jim Meyering <jim@meyering.net> wrote:
> Alex Riesen wrote:
>> On Mon, Sep 21, 2009 at 11:09, Jim Meyering <jim@meyering.net> wrote:
>>> However, if the use of [] is deliberate, because git still
>>> cares about portability to ancient SYSV versions of tr that
>>> require that notation, then let me know and I'll undo that part
>>> of the change and add a comment to that effect.
>>
>> We have (had?) people trying to support Git on HP-UX and SunOS.
>> Do these count?
>
> I had my doubts, but have just confirmed that Solaris 10's
> /usr/bin/tr is still doing it the SYSV way:
>
>    $ echo foo | LC_ALL=C /usr/bin/tr a-z A-Z
>    foo
>
> There, you have to use /usr/xpg4/bin/tr to get the expected behavior:
>
>    $ echo foo | LC_ALL=C /usr/xpg4/bin/tr a-z A-Z
>    FOO
>
> So you're right.  ...

Sorry about that. I even made a simple tr once for Git tests
exactly because of compatibility problems (look for "poor man tr"
in the list archives, if needed).

^ permalink raw reply

* Re: [PATCH] pre-commit.sample: don't use [...] around a tr range
From: Jim Meyering @ 2009-09-21 11:00 UTC (permalink / raw)
  To: Alex Riesen; +Cc: git list
In-Reply-To: <81b0412b0909210243h58633509r7b427c69f62928fe@mail.gmail.com>

Alex Riesen wrote:
> On Mon, Sep 21, 2009 at 11:09, Jim Meyering <jim@meyering.net> wrote:
>> However, if the use of [] is deliberate, because git still
>> cares about portability to ancient SYSV versions of tr that
>> require that notation, then let me know and I'll undo that part
>> of the change and add a comment to that effect.
>
> We have (had?) people trying to support Git on HP-UX and SunOS.
> Do these count?

I had my doubts, but have just confirmed that Solaris 10's
/usr/bin/tr is still doing it the SYSV way:

    $ echo foo | LC_ALL=C /usr/bin/tr a-z A-Z
    foo

There, you have to use /usr/xpg4/bin/tr to get the expected behavior:

    $ echo foo | LC_ALL=C /usr/xpg4/bin/tr a-z A-Z
    FOO

So you're right.  Thanks!
Here's an updated patch:

>From 4048a5b393fa5f35dfe8157e98a6ea475b0efb2d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 21 Sep 2009 10:58:02 +0200
Subject: [PATCH] pre-commit.sample: add comment re tr portability; fix grammar

Add a comment explaining why square brackets around a tr range
are not only ok, but actually required in this case.
Correct spelling and grammar.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 templates/hooks--pre-commit.sample |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b11ad6a..128c8cc 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -14,15 +14,18 @@ allownonascii=$(git config hooks.allownonascii)
 # them from being added to the repository. We exploit the fact that the
 # printable range starts at the space character and ends with tilde.
 if [ "$allownonascii" != "true" ] &&
+	# Note that the use of brackets around a tr range is ok here, (it's
+	# even required, for portability to Solaris 10's /usr/bin/tr), since
+	# the square bracket bytes happen to fall in the designated range.
 	test "$(git diff --cached --name-only --diff-filter=A -z |
 	  LC_ALL=C tr -d '[ -~]\0')"
 then
-	echo "Error: Attempt to add a non-ascii filename."
+	echo "Error: Attempt to add a non-ascii file name."
 	echo
-	echo "This can cause problems if you want to work together"
-	echo "with people on other platforms than you."
+	echo "This can cause problems if you want to work"
+	echo "with people on other platforms."
 	echo
-	echo "To be portable it is adviseable to rename the file ..."
+	echo "To be portable it is advisable to rename the file ..."
 	echo
 	echo "If you know what you are doing you can disable this"
 	echo "check using:"
--
1.6.5.rc1.214.g13c5a

^ permalink raw reply related

* Re: [PATCH] pre-commit.sample: don't use [...] around a tr range
From: Alex Riesen @ 2009-09-21  9:43 UTC (permalink / raw)
  To: Jim Meyering; +Cc: git list
In-Reply-To: <871vm0k6jm.fsf@meyering.net>

On Mon, Sep 21, 2009 at 11:09, Jim Meyering <jim@meyering.net> wrote:
> However, if the use of [] is deliberate, because git still
> cares about portability to ancient SYSV versions of tr that
> require that notation, then let me know and I'll undo that part
> of the change and add a comment to that effect.

We have (had?) people trying to support Git on HP-UX and SunOS.
Do these count?

^ permalink raw reply

* [PATCH] pre-commit.sample: don't use [...] around a tr range
From: Jim Meyering @ 2009-09-21  9:09 UTC (permalink / raw)
  To: git list

Using square brackets in tr ranges is risky.
From the documentation of GNU tr:

  Ranges
       The notation `M-N' expands to all of the characters from M through
       N, in ascending order.  M should collate before N; if it doesn't,
       an error results.  As an example, `0-9' is the same as
       `0123456789'.

       GNU `tr' does not support the System V syntax that uses square
       brackets to enclose ranges.  Translations specified in that format
       sometimes work as expected, since the brackets are often
       transliterated to themselves.  However, they should be avoided
       because they sometimes behave unexpectedly.  For example, `tr -d
       '[0-9]'' deletes brackets as well as digits.

However, if the use of [] is deliberate, because git still
cares about portability to ancient SYSV versions of tr that
require that notation, then let me know and I'll undo that part
of the change and add a comment to that effect.

>From cd3c17c6a48c67d2a598001272f3283714b1df19 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Mon, 21 Sep 2009 10:58:02 +0200
Subject: [PATCH] pre-commit.sample: don't use [...] around a tr range

Using square brackets around a tr range is especially risky when using
tr's -d (delete) option.  In this case, the brackets happen to be harmless,
since the two bracket bytes happen to be included in the desired range.
Correct spelling and grammar.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 templates/hooks--pre-commit.sample |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample
index b11ad6a..06a5afa 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -15,14 +15,14 @@ allownonascii=$(git config hooks.allownonascii)
 # printable range starts at the space character and ends with tilde.
 if [ "$allownonascii" != "true" ] &&
 	test "$(git diff --cached --name-only --diff-filter=A -z |
-	  LC_ALL=C tr -d '[ -~]\0')"
+	  LC_ALL=C tr -d ' -~\0')"
 then
-	echo "Error: Attempt to add a non-ascii filename."
+	echo "Error: Attempt to add a non-ascii file name."
 	echo
-	echo "This can cause problems if you want to work together"
-	echo "with people on other platforms than you."
+	echo "This can cause problems if you want to work"
+	echo "with people on other platforms."
 	echo
-	echo "To be portable it is adviseable to rename the file ..."
+	echo "To be portable it is advisable to rename the file ..."
 	echo
 	echo "If you know what you are doing you can disable this"
 	echo "check using:"
--
1.6.5.rc1.214.g13c5a

^ permalink raw reply related

* Re: error: insufficient permission
From: Luciano Rocha @ 2009-09-21  8:52 UTC (permalink / raw)
  To: Dominic Rose; +Cc: git
In-Reply-To: <4AB73A07.3020703@bioinf.uni-leipzig.de>

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

On Mon, Sep 21, 2009 at 10:32:07AM +0200, Dominic Rose wrote:
> Hi,
> 
> may please somebody help me with the following issue. I try to
> checkin changes to a repository I created. Seems like I have some
> permission problems, but I can't figure out whats going wrong:
> 
> $ git commit -m 'test'
> Created commit 07b3a4a: test
> 1 files changed, 1 insertions(+), 1 deletions(-)
> $ git push
> Counting objects: 8, done.
> Compressing objects: 100% (6/6), done.
> Writing objects: 100% (6/6), 1.56 KiB, done.
> Total 6 (delta 4), reused 0 (delta 0)
> error: insufficient permission for adding an object to repository
> database ./objects
> 
> fatal: failed to write object
> error: unpack failed: unpacker exited with error code
> To /homes/[...]
> ! [remote rejected] master -> master (n/a (unpacker error))
> error: failed to push some refs to '/homes/[...]'

That's a filesystem permissions problem on the server side. You'll have
to check if your user has permissions to write to the objects subdir of the
repository.

-- 
Luciano Rocha <luciano@eurotux.com>
Eurotux Informática, S.A. <http://www.eurotux.com/>

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: error: insufficient permission
From: Johannes Sixt @ 2009-09-21  8:53 UTC (permalink / raw)
  To: Dominic Rose; +Cc: git
In-Reply-To: <4AB73A07.3020703@bioinf.uni-leipzig.de>

Dominic Rose schrieb:
> error: insufficient permission for adding an object to repository
> database ./objects
> 
> fatal: failed to write object
> error: unpack failed: unpacker exited with error code
> To /homes/[...]
> ! [remote rejected] master -> master (n/a (unpacker error))
> error: failed to push some refs to '/homes/[...]'

Make sure you have permission to create new files and directories in
/homes/[...]/objects and that you have permission to create new files in
all directories that are already in /homes/[...]/objects.

-- Hannes

^ permalink raw reply

* error: insufficient permission
From: Dominic Rose @ 2009-09-21  8:32 UTC (permalink / raw)
  To: git

Hi,

may please somebody help me with the following issue. I try to checkin 
changes to a repository I created. Seems like I have some permission 
problems, but I can't figure out whats going wrong:

$ git commit -m 'test'
Created commit 07b3a4a: test
 1 files changed, 1 insertions(+), 1 deletions(-)
$ git push
Counting objects: 8, done.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 1.56 KiB, done.
Total 6 (delta 4), reused 0 (delta 0)
error: insufficient permission for adding an object to repository 
database ./objects

fatal: failed to write object
error: unpack failed: unpacker exited with error code
To /homes/[...]
 ! [remote rejected] master -> master (n/a (unpacker error))
error: failed to push some refs to '/homes/[...]'


Thanks,
Dominic

-- 
Dominic Rose
Professur für Bioinformatik
Institut für Informatik
Universität Leipzig
Härtelstr. 16-18
D-04107 Leipzig
WWW    http://www.bioinf.uni-leipzig.de
Phone: +49 341 97-16698
Fax:   +49 341 97-16679

^ permalink raw reply

* Re: [PATCH] test-genrandom: ensure stdout is set to _O_BINARY on Windows
From: Junio C Hamano @ 2009-09-21  7:36 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Marius Storm-Olsen, Git Mailing List, Li Frank, msysGit
In-Reply-To: <4AB72CA2.1020808@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> From: Johannes Sixt <j6t@kdbg.org>
>
> Commit a6ca8c62 (Set _O_BINARY as default fmode for both MinGW and MSVC)
> removed the definition of _CRT_fmode from mingw.c. Before this commit,
> since test-genrandom is linked against libgit.a, the MinGW process
> initialization code would pick up that definition of _CRT_fmode, which was
> initialized to _O_BINARY. After this commit, however, text mode is used
> for std(in|out|err) because it is the default in absence of _CRT_fmode.
> In order to fix that, we must use git-compat-util.h, which overrides
> main() to set the mode to binary.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Unfortunately, this change in an updated patch 04/15 of the MSVC series
>  went to the Big Void. ;)

Thanks.

^ permalink raw reply

* [PATCH] test-genrandom: ensure stdout is set to _O_BINARY on Windows
From: Johannes Sixt @ 2009-09-21  7:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marius Storm-Olsen, Git Mailing List, Li Frank, msysGit

From: Johannes Sixt <j6t@kdbg.org>

Commit a6ca8c62 (Set _O_BINARY as default fmode for both MinGW and MSVC)
removed the definition of _CRT_fmode from mingw.c. Before this commit,
since test-genrandom is linked against libgit.a, the MinGW process
initialization code would pick up that definition of _CRT_fmode, which was
initialized to _O_BINARY. After this commit, however, text mode is used
for std(in|out|err) because it is the default in absence of _CRT_fmode.
In order to fix that, we must use git-compat-util.h, which overrides
main() to set the mode to binary.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Unfortunately, this change in an updated patch 04/15 of the MSVC series
 went to the Big Void. ;)

 -- Hannes

 test-genrandom.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/test-genrandom.c b/test-genrandom.c
index 8ad276d..b3c28d9 100644
--- a/test-genrandom.c
+++ b/test-genrandom.c
@@ -4,8 +4,7 @@
  * Copyright (C) 2007 by Nicolas Pitre, licensed under the GPL version 2.
  */

-#include <stdio.h>
-#include <stdlib.h>
+#include "git-compat-util.h"

 int main(int argc, char *argv[])
 {
-- 
1.6.5.rc1.1051.gdc4fd

^ permalink raw reply related

* Re: [PATCH] Remove '< >' from [<options>] since it's not necessary.
From: Matthieu Moy @ 2009-09-21  6:50 UTC (permalink / raw)
  To: tom fogal; +Cc: git, Thiago Farina
In-Reply-To: <auto-000020523806@sci.utah.edu>

tom fogal <tfogal@alumni.unh.edu> writes:

> Thiago Farina <tfransosi@gmail.com> writes:
>> -	"git log [<options>] [<since>..<until>] [[--] <path>...]\n"
>> +	"git log [options] [<since>..<until>] [[--] <path>...]\n"
>>  	"   or: git show [options] <object>...";
>
> To me, "<blah>" implies to me that the author meant, "something which
> is sort-of `blah'-ish but is not actually the string literal, `blah'",
> whereas `blah' means, "the string literal, `blah'."

Strictly speaking, you're right, but in the particular case of
[options], it's rather well established that it means "one or several
options", and not the litteral "option". In any case, we should be
consistant:

Documentation$ grep -n '\[<options>\]' *.txt
git-log.txt:11:'git log' [<options>] [<since>..<until>] [[\--] <path>...]
git-stash.txt:11:'git stash' list [<options>]
git-stash.txt:67:list [<options>]::
Documentation$ grep -n '\[options\]' *.txt
git-annotate.txt:10:'git annotate' [options] file [revision]
gitcli.txt:84:usage: git-describe [options] <committish>*
git-cvsserver.txt:25:'git cvsserver' [options] [pserver|server] [<directory> ...]
git-fast-export.txt:11:'git fast-export [options]' | 'git fast-import'
git-fast-import.txt:11:frontend | 'git fast-import' [options]
git-rebase.txt:11:'git rebase' [-i | --interactive] [options] [--onto <newbase>]
git-rebase.txt:13:'git rebase' [-i | --interactive] [options] --onto <newbase>
git-rev-parse.txt:405:some-command [options] <args>...
git-send-email.txt:11:'git send-email' [options] <file|directory|rev-list options>...
git-show.txt:11:'git show' [options] <object>...
git-svn.txt:10:'git svn' <command> [options] [arguments]

=> either get rid of the 3 occurences of [<options>], not just one, or
get rid of all the [options]. My vote goes for the first.

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] Trivial fix: Display a more friendly message with git-shell.
From: Matthieu Moy @ 2009-09-21  6:45 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Thiago Farina, git
In-Reply-To: <4AB71A76.5010509@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> Every other use of git-shell is an error, and the current error message
> says exactly that, although in a very 31337 way (and, BTW, /me likes
> it :-)

Well, it's funny, but for a newcommer, trying "git shell" hoping to
get, for example what "bzr shell" gives (an interactive shell with
normal shell commands plus bzr ones without the 'bzr' prefix), it's
quite frustrating to be almost insulted by the tool for being curious.

> If an naive user runs git-shell from the command line, then your new
> message will not be helpful because git-shell will accept only a very
> restricted set of commands.

Perhaps one more line saying what 'man git-shell' says in addition
would help:

$ git-shell
git-shell: Restricted login shell for GIT-only SSH access
Usage: ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] Trivial fix: Display a more friendly message with git-shell.
From: Johannes Sixt @ 2009-09-21  6:17 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <1253466672-21051-1-git-send-email-tfransosi@gmail.com>

Thiago Farina schrieb:
> +static const char shell_usage[] = "git shell -c <command> <argument>";
> +
>  static int do_generic_cmd(const char *me, char *arg)
>  {
>  	const char *my_argv[4];
> @@ -74,7 +76,7 @@ int main(int argc, char **argv)
>  	 * where "cmd" is a very limited subset of git commands.
>  	 */
>  	else if (argc != 3 || strcmp(argv[1], "-c"))
> -		die("What do you think I am? A shell?");
> +		usage(shell_usage);

I don't think this is an improvement.

git-shell has a very limited use: It is to be used as the login-shell of
accounts where the admin will allow ssh access, but only to git repositories.

Every other use of git-shell is an error, and the current error message
says exactly that, although in a very 31337 way (and, BTW, /me likes it :-)

If an naive user runs git-shell from the command line, then your new
message will not be helpful because git-shell will accept only a very
restricted set of commands.

-- Hannes

^ permalink raw reply

* Re: [PATCH] Trivial fix: Display a more friendly message with  git-shell.
From: Tay Ray Chuan @ 2009-09-21  4:21 UTC (permalink / raw)
  To: Thiago Farina; +Cc: git
In-Reply-To: <1253466672-21051-1-git-send-email-tfransosi@gmail.com>

Hi,

On Mon, Sep 21, 2009 at 1:11 AM, Thiago Farina <tfransosi@gmail.com> wrote:
> Instead of simply die without give a helpful message, displays the usage

s/give/giving/.

Not sure if the same should be done for die (like die()'ing).

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [PATCH] Remove '< >' from [<options>] since it's not necessary.
From: tom fogal @ 2009-09-20 21:12 UTC (permalink / raw)
  To: git; +Cc: Thiago Farina
In-Reply-To: <auto-000020523806@sci.utah.edu>

tom fogal <tfogal@alumni.unh.edu> writes:
> Thiago Farina <tfransosi@gmail.com> writes:
> > -	"git log [<options>] [<since>..<until>] [[--] <path>...]\n"
> > +	"git log [options] [<since>..<until>] [[--] <path>...]\n"
> >  	"   or: git show [options] <object>...";
> 
> To me, "<blah>" implies to me that the other meant, "something which
                                         ^^^^^
... author.  sorry.

-tom

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox