From: Paul Mackerras <paulus@ozlabs.org>
Date: Sat, 19 Dec 2015 03:13:32 GMT
Subject: Re: [PATCH v3] gitk: add -C <path> commandline parameter to change path
Message-ID: <20151219031332.GE422@fergus.ozlabs.ibm.com>
In-Reply-To: <1447069522-19895-1-git-send-email-juhapekka.heikkila@gmail.com>

On Mon, Nov 09, 2015 at 01:45:22PM +0200, Juha-Pekka Heikkila wrote:
> This patch adds -C (change working directory) parameter to
> gitk. With this parameter, instead of need to cd to directory
> with .git folder, one can point the correct folder from
> commandline.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

Thanks.

While I like the idea, there are a couple of minor problems with the
patch.  First, the Documentation directory is in Junio's tree, not
mine, so the change to gitk and the change to Documentation need to be
separated.  Secondly, please use 4-space indentation rather than
8-space for consistency with the rest of the file.  See also the
comments below.

> +	"-C*" {
> +		if {[string length $arg] < 3} {
> +			incr i
> +			set cwd_path [lindex $argv [expr {$i}]]

No need to say [expr {$i}] here; [lindex $argv $i] works just fine.

Also, if i is now >= [llength $argv], we'll get an empty string in
cwd_path.  Is that what you meant?  Shouldn't we display an
appropriate error message instead of trying to cd to ""?

Paul.