Re: [PATCH 1/2] rev-parse: add --filename-prefix option
John Keeping writes: > On Mon, Apr 08, 2013 at 08:07:32AM -0700, Junio C Hamano wrote: >> John Keeping writes: >> >> > Yes (ish), the intended usage is something like this: >> > >> > prefix=$(git rev-parse --show-prefix) >> > cd_to_toplevel >> > ... parse options here ... >> > # Convert remaining arguments (filenames) into top-level paths: >> > eval "set $(git rev-parse --prefix "$prefix" --sq -- "$@")" >> > >> > The "ish" is that my current implementation introduced a new variable >> > instead of simply resetting the existing "prefix" variable, which I >> > assume is what you mean. >> >> This is very sensible. > > Which bit specifically? I assume you agree with the intended usage, but > do you also mean that resetting the prefix returned from > setup_git_directory is the right way to approach this? My gut feeling says yes, but you can persuade me easily why it is a bad idea if you have an example of why it would not work well. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rev-parse: add --filename-prefix option
On Mon, Apr 08, 2013 at 08:07:32AM -0700, Junio C Hamano wrote: > John Keeping writes: > > > Yes (ish), the intended usage is something like this: > > > > prefix=$(git rev-parse --show-prefix) > > cd_to_toplevel > > ... parse options here ... > > # Convert remaining arguments (filenames) into top-level paths: > > eval "set $(git rev-parse --prefix "$prefix" --sq -- "$@")" > > > > The "ish" is that my current implementation introduced a new variable > > instead of simply resetting the existing "prefix" variable, which I > > assume is what you mean. > > This is very sensible. Which bit specifically? I assume you agree with the intended usage, but do you also mean that resetting the prefix returned from setup_git_directory is the right way to approach this? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rev-parse: add --filename-prefix option
John Keeping writes: > Yes (ish), the intended usage is something like this: > > prefix=$(git rev-parse --show-prefix) > cd_to_toplevel > ... parse options here ... > # Convert remaining arguments (filenames) into top-level paths: > eval "set $(git rev-parse --prefix "$prefix" --sq -- "$@")" > > The "ish" is that my current implementation introduced a new variable > instead of simply resetting the existing "prefix" variable, which I > assume is what you mean. This is very sensible. > That is probably simpler than my > implementation, but loses the ability to be at an intermediate level, > for example: > > cd Documentation/ > eval "set $(git rev-parse --prefix technical/ --sq -- api-strbuf.txt)" I am not sure in what situation it makes sense to do this. Where does "technical/" come from? For a script that wants to work from a subdirectory, end-user input would come in the form relative to the current directory, i.e. "Documentation/" from the top-level, so... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rev-parse: add --filename-prefix option
On Sun, Apr 07, 2013 at 03:14:58PM -0700, Jonathan Nieder wrote: > John Keeping wrote: > > > This adds a prefix string to any filename arguments encountered after it > > has been specified. > > I assume this is a way of passing the prefix in? In that case, I > think a good UI would be > > git rev-parse --prefix=Documentation/ > > That sounds like a useful thing and would make the meaning very clear. Yes (ish), the intended usage is something like this: prefix=$(git rev-parse --show-prefix) cd_to_toplevel ... parse options here ... # Convert remaining arguments (filenames) into top-level paths: eval "set $(git rev-parse --prefix "$prefix" --sq -- "$@")" The "ish" is that my current implementation introduced a new variable instead of simply resetting the existing "prefix" variable, which I assume is what you mean. That is probably simpler than my implementation, but loses the ability to be at an intermediate level, for example: cd Documentation/ eval "set $(git rev-parse --prefix technical/ --sq -- api-strbuf.txt)" > How does this interact with the following options? > > * --resolve-git-dir some/relative/path It doesn't change this since --resolve-git-dir is handled separately from the other argument parsing at the moment and you cannot specify any other options with it. > * master:./path I hadn't considered this case, but I think it should be inserting the prefix into the path. I suspect the easiest thing to do is simply make the path part of that absolute, by combining both the prefix based on $PWD and the prefix specified on the command line, but I haven't looked at doing this yet. The other think that's missing at the moment is that the prefix passed to verify_filename should be modified by the one specified on the command line. > As for the patch itself, I haven't looked at it closely. My only > immediate reaction is that I wish it touched Documentation/ and t/. :) I'll make sure the next version does. This version was doing the minimum required to make patch 2/2 possible, it certainly needs some polish before it's more than a proof-of-concept. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] rev-parse: add --filename-prefix option
Hi, John Keeping wrote: > This adds a prefix string to any filename arguments encountered after it > has been specified. I assume this is a way of passing the prefix in? In that case, I think a good UI would be git rev-parse --prefix=Documentation/ That sounds like a useful thing and would make the meaning very clear. How does this interact with the following options? * --resolve-git-dir some/relative/path * master:./path As for the patch itself, I haven't looked at it closely. My only immediate reaction is that I wish it touched Documentation/ and t/. :) Hope that helps, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] rev-parse: add --filename-prefix option
This adds a prefix string to any filename arguments encountered after it
has been specified.
Signed-off-by: John Keeping
---
builtin/rev-parse.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f267a1d..11501a4 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -212,11 +212,12 @@ static void show_datestring(const char *flag, const char
*datestr)
show(buffer);
}
-static int show_file(const char *arg)
+static int show_file(const char *arg, const char *prefix)
{
show_default();
if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
- show(arg);
+ size_t prefixlen = prefix ? strlen(prefix) : 0;
+ show(prefix_filename(prefix, prefixlen, arg));
return 1;
}
return 0;
@@ -470,6 +471,7 @@ N_("git rev-parse --parseopt [options] -- [...]\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;
+ const char *filename_prefix = NULL;
unsigned char sha1[20];
const char *name = NULL;
@@ -503,7 +505,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) && as_is < 2)
+ if (show_file(arg, filename_prefix) && as_is < 2)
verify_filename(prefix, arg, 0);
continue;
}
@@ -527,7 +529,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);
+ show_file(arg, NULL);
continue;
}
if (!strcmp(arg, "--default")) {
@@ -535,6 +537,11 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
i++;
continue;
}
+ if (!strcmp(arg, "--filename-prefix")) {
+ filename_prefix = argv[i+1];
+ i++;
+ continue;
+ }
if (!strcmp(arg, "--revs-only")) {
filter &= ~DO_NOREV;
continue;
@@ -754,7 +761,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))
+ if (!show_file(arg, filename_prefix))
continue;
verify_filename(prefix, arg, 1);
}
--
1.8.2.694.ga76e9c3.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
