Re: [PATCH v3] gitk: add -C <path> commandline parameter to change path

From: Paul Mackerras <paulus@ozlabs.org>
Date: Dec 19, 2015, 03:13 UTC
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:

Show 6 quoted lines

> 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.