git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Paul Mackerras <paulus@ozlabs.org>
To: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: git@vger.kernel.org, sunshine@sunshineco.com
Subject: Re: [PATCH v3] gitk: add -C <path> commandline parameter to change path
Date: Sat, 19 Dec 2015 14:13:32 +1100	[thread overview]
Message-ID: <20151219031332.GE422@fergus.ozlabs.ibm.com> (raw)
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.

      reply	other threads:[~2015-12-19  3:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-03 15:00 [PATCH 0/1] gitk: add --cwd=path commandline parameter to change path Juha-Pekka Heikkila
2015-11-03 15:00 ` [PATCH 1/1] " Juha-Pekka Heikkila
2015-11-03 18:27   ` Eric Sunshine
2015-11-05  9:19     ` [PATCH v2] gitk: add -C <path> " Juha-Pekka Heikkila
2015-11-06  9:48       ` Eric Sunshine
2015-11-06 10:49         ` Juha-Pekka Heikkila
2015-11-09 11:45           ` [PATCH v3] " Juha-Pekka Heikkila
2015-12-19  3:13             ` Paul Mackerras [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151219031332.GE422@fergus.ozlabs.ibm.com \
    --to=paulus@ozlabs.org \
    --cc=git@vger.kernel.org \
    --cc=juhapekka.heikkila@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).