Re: [PATCH v3 1/2] rev-parse: add --prefix option
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Apr 19, 2013, 09:53 UTC
- Message-ID
- <1366365217-17674-1-git-send-email-artagnon@gmail.com>
- In-Reply-To
- <82316c445320b3cec3a35cbc87ad94a04ed26d1e.1366314439.git.john@keeping.me.uk>
John Keeping wrote:
> Changes since v2: > - Rewrite commit message > - Add a new test for 'prefix ignored with HEAD:top' > - Use '<<-\EOF' where appropriate in t1513
Thanks for the re-roll.
In the previous iteration, I wasn't sure this was the right approach because I thought it would be better to pass RUN_SETUP and let run_builtin_command() take care of the prefix-setting. Unfortunately, as 5410a02 (git-rev-parse --parseopt, 2007-11-06) indicates, we have to run setup_git_directory() in cmd_rev_parse() after parsing --parseopt, as 'git rev-parse --parseopt' can be run outside a git repository. You might want to include this note in your commit message for the benefit of other readers.
Other than that, I just have one small suggestion: it's possible to avoid passing output_prefix around, and simplify show_file() a bit if we acknowledge that printing "--" is not the same as printing a file (although the condition is the same). Would you like to squash this in?
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> -- 8< --
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index de894c7..7e69b3f 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -29,6 +29,7 @@ static int abbrev; static int abbrev_ref; static int abbrev_ref_strict; static int output_sq; +static int output_prefix; /* * Some arguments are relevant "revision" arguments, @@ -212,15 +213,13 @@ static void show_datestring(const char *flag, const char *datestr) show(buffer); } -static int show_file(const char *arg, int output_prefix) +static int show_file(const char *arg) { show_default(); if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) { if (output_prefix) { const char *prefix = startup_info->prefix; - show(prefix_filename(prefix, - prefix ? strlen(prefix) : 0, - arg)); + show(prefix_filename(prefix, strlen(prefix), arg)); } else show(arg); return 1; @@ -228,6 +227,16 @@ static int show_file(const char *arg, int output_prefix) return 0; } +static int show_dashdash() +{ + show_default(); + if ((filter & (DO_NONFLAGS | DO_NOREV)) == (DO_NONFLAGS | DO_NOREV)) { + show("--"); + return 1; + } + return 0; +} + static int try_difference(const char *arg) { char *dotdot; @@ -476,7 +485,6 @@ N_("git rev-parse --parseopt [options] -- [<args>...]\n" int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0; - int output_prefix = 0; unsigned char sha1[20]; const char *name = NULL; @@ -510,7 +518,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) const char *arg = argv[i]; if (as_is) { - if (show_file(arg, output_prefix) && as_is < 2) + if (show_file(arg) && as_is < 2) verify_filename(prefix, arg, 0); continue; } @@ -534,7 +542,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) as_is = 2; /* Pass on the "--" if we show anything but files.. */ if (filter & (DO_FLAGS | DO_REVS)) - show_file(arg, 0); + show_dashdash(); continue; } if (!strcmp(arg, "--default")) { @@ -543,7 +551,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--prefix")) { - prefix = argv[i+1]; + prefix = argv[i + 1]; startup_info->prefix = prefix; output_prefix = 1; i++; @@ -768,7 +776,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (verify) die_no_single_rev(quiet); as_is = 1; - if (!show_file(arg, output_prefix)) + if (!show_file(arg)) continue; verify_filename(prefix, arg, 1); }