threads / rfc / 25238

RFC v2, 4 partsrev-parse: allow --flags to output rev-parse-like flags

Subject: [PATCH/RFC v2 0/4] rev-parse: allow --flags to output rev-parse-like flags

## tl;dr

8 messages between Sep 25, 2010 and Sep 25, 2010. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Jon Seymour· Sep 25, 2010, 09:04 UTC · lore

This series allows git rev-parse --flags to output remaining flag-like arguments even if such arguments are valid options to git rev-parse itself.

Previously:
  $> git rev-parse --flags -q -X --no-flags -- Y -Z
  -X

Now:
  $> git rev-parse --flags -q -X --no-flags -- Y -Z
  -q -X --no-flags

Also:
  $> git rev-parse --symbolic --no-flags --flags -X HEAD
  HEAD

Note: git rev-parse --flags still seems broken w.r.t. documentation because:
  $> git rev-parse --symbolic --flags HEAD
  HEAD
even though the documentation states that --flags does not output non-flag
arguments.

Jon Seymour (4):
  rev-parse: stop interpreting flags as options to rev-parse once
    --flags is specified
  rev-parse: Don't recognise --flags as an option if --no-flags has
    been specified.
  rev-parse: add tests for git rev-parse --flags.
  rev-parse: update documentation of --flags and --no-flags options

 Documentation/git-rev-parse.txt |   10 +++-
 builtin/rev-parse.c             |   11 ++++
 t/t1510-rev-parse-flags.sh      |  116 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100755 t/t1510-rev-parse-flags.sh

-- 
1.7.3.1.gc81ce.dirty

Jon Seymour· Sep 25, 2010, 09:04 UTC · re: Jon Seymour · lore

[PATCH/RFC v2 0/4] *** SUBJECT HERE ***

*** BLURB HERE ***

Jon Seymour (4):
  rev-parse: stop interpreting flags as options to rev-parse once
    --flags is specified
  rev-parse: Don't recognise --flags as an option if --no-flags has
    been specified.
  rev-parse: add tests for git rev-parse --flags.
  rev-parse: update documentation of --flags and --no-flags options

 Documentation/git-rev-parse.txt |   10 +++-
 builtin/rev-parse.c             |   11 ++++
 t/t1510-rev-parse-flags.sh      |  116 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100755 t/t1510-rev-parse-flags.sh

-- 
1.7.3.1.gc81ce.dirty

Jon Seymour· Sep 25, 2010, 09:04 UTC · re: Jon Seymour · lore

[PATCH/RFC v2 1/4] rev-parse: stop interpreting flags as options to rev-parse once --flags is specified

Current git rev-parse behaviour makes --flags hard to use if the remaining arguments to git rev-parse contain an option that would otherwise be interpreted as an option by git rev-parse itself.

So, for example:

  $> git rev-parse --flags -q -X
  -X

Normally one might expect to use -- to prevent -q being interpreted:

  $> git rev-parse --flags -- -q -X
  -q -X

But we can't really use -- in this way, because commands that use git rev-parse might reasonably expect:

  $> git rev-parse --flags -Y -- -q -X
  -Y

That is, -Y to be regarded as a flag but everything after -- to be uninterpreted.

This proposed change modifies git rev-parse so that git rev-parse stops interpreting flag arguments as options to git rev-parse once --flags is interpreted. We also exit early once -- is found.

Tests will follow in subsequent iterations of this patch, if the consensus is that the approach is correct.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 builtin/rev-parse.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

Show changes to builtin/rev-parse.c +7 −0

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index a5a1c86..9e340c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -497,8 +497,15 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				/* Pass on the "--" if we show anything but files.. */
 				if (filter & (DO_FLAGS | DO_REVS))
 					show_file(arg);
+				if (!(filter & DO_NONFLAGS)) {
+					return 0;
+				}
 				continue;
 			}
+			if (!(filter & DO_NONFLAGS)) {
+				 show_flag(arg);
+				 continue;
+			}
 			if (!strcmp(arg, "--default")) {
 				def = argv[i+1];
 				i++;

-- 
1.7.3.1.gc81ce.dirty

Jon Seymour· Sep 25, 2010, 09:04 UTC · re: Jon Seymour · lore

[PATCH/RFC v2 2/4] rev-parse: Don't recognise --flags as an option if --no-flags has been specified.

Previously:
   $ git rev-parse --no-flags --flags -X
   -X

now:
   $ git rev-parse --no-flags --flags -X

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 builtin/rev-parse.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Show changes to builtin/rev-parse.c +4 −0

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 9e340c7..d403d29 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -520,6 +520,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--flags")) {
+				if (!(filter & DO_FLAGS)) {
+					// --preceding --no-flags means --flags is ignored
+					continue;
+				}
 				filter &= ~DO_NONFLAGS;
 				continue;
 			}

-- 
1.7.3.1.gc81ce.dirty

Ævar Arnfjörð Bjarmason· Sep 25, 2010, 10:26 UTC · re: Jon Seymour · lore

Re: [PATCH/RFC v2 2/4] rev-parse: Don't recognise --flags as an option if --no-flags has been specified.

On Sat, Sep 25, 2010 at 09:04, Jon Seymour <jon.seymour@gmail.com> wrote:

> +                               if (!(filter & DO_FLAGS)) {
> +                                       // --preceding --no-flags means --flags is ignored
> +                                       continue;
> +                               }

Please use a /* C89 comment */, not the C++ / C99 style.

Jon Seymour· Sep 25, 2010, 09:04 UTC · re: Jon Seymour · lore

[PATCH/RFC v2 3/4] rev-parse: add tests for git rev-parse --flags.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t1510-rev-parse-flags.sh |  116 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 116 insertions(+), 0 deletions(-)
 create mode 100755 t/t1510-rev-parse-flags.sh

Show changes to t/t1510-rev-parse-flags.sh +116 −0

diff --git a/t/t1510-rev-parse-flags.sh b/t/t1510-rev-parse-flags.sh
new file mode 100755
index 0000000..f41130d
--- /dev/null
+++ b/t/t1510-rev-parse-flags.sh
@@ -0,0 +1,116 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Jon Seymour
+#
+
+test_description='test git rev-parse --flags'
+. ./test-lib.sh
+
+test_commit "A"
+
+test_expect_success 'git rev-parse --flags -> ""' \
+'
+	output=$(git rev-parse --flags) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags X -> ""' \
+'
+	output=$(git rev-parse --flags X) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --no-revs --flags HEAD -> ""' \
+'
+	output=$(git rev-parse --no-revs --flags HEAD) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --symbolic --flags HEAD -> "HEAD"' \
+'
+	output=$(git rev-parse --symbolic --flags HEAD) &&
+	expected="HEAD" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -- -> ""' \
+'
+	output=$(git rev-parse --flags --) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -- X -> ""' \
+'
+	output=$(git rev-parse --flags -- X) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -- -X -> ""' \
+'
+	output=$(git rev-parse --flags -- -X) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -- -q --> ""' \
+'
+	output=$(git rev-parse --flags -- -q) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -X -> "-X"' \
+'
+	output="$(git rev-parse --flags -X)" &&
+	expected="-X" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -q -> "-q"' \
+'
+	output=$(git rev-parse --flags -q) &&
+	expected="-q" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags -X -- Y -Z -> "-X"' \
+'
+	output=$(git rev-parse --flags -X -- Y -Z) &&
+	expected="-X" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --flags --no-flags -> "--no-flags"' \
+'
+	output=$(git rev-parse --flags --no-flags) &&
+	expected="--no-flags" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --no-flags --flags -X -> ""' \
+'
+	output=$(git rev-parse --no-flags --flags -X) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --symbolic --no-flags --flags HEAD -> "HEAD"' \
+'
+	output=$(git rev-parse --symbolic --no-flags --flags HEAD) &&
+	expected="HEAD" &&
+	test "${output}" = "${expected}"
+'
+
+test_expect_success 'git rev-parse --no-flags --flags -X -> ""' \
+'
+	output=$(git rev-parse --no-flags --flags -X) &&
+	expected="" &&
+	test "${output}" = "${expected}"
+'
+
+test_done

-- 
1.7.3.1.gc81ce.dirty

Ævar Arnfjörð Bjarmason· Sep 25, 2010, 10:29 UTC · re: Jon Seymour · lore

Re: [PATCH/RFC v2 3/4] rev-parse: add tests for git rev-parse --flags.

Show 14 quoted lines

> +test_expect_success 'git rev-parse --flags -> ""' \
> +'
> +       output=$(git rev-parse --flags) &&
> +       expected="" &&
> +       test "${output}" = "${expected}"
> +'
> +
> +test_expect_success 'git rev-parse --flags X -> ""' \
> +'
> +       output=$(git rev-parse --flags X) &&
> +       expected="" &&
> +       test "${output}" = "${expected}"
> +'
> +

Please use test_cmp throughout the test instead:

    >expect &&
    git rev-parse --flags >actual &&
    test_cmp expect actual

etc.

Jon Seymour· Sep 25, 2010, 09:04 UTC · re: Jon Seymour · lore

[PATCH/RFC v2 4/4] rev-parse: update documentation of --flags and --no-flags options

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 Documentation/git-rev-parse.txt |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

Show changes to Documentation/git-rev-parse.txt +8 −2

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 341ca90..8fb8f6b 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -49,10 +49,16 @@ OPTIONS
 	'git rev-list' command.
 
 --flags::
-	Do not output non-flag parameters.
+	Do not output non-flag parameters. All remaining
+	arguments will not be interpreted as options
+	to 'git rev-parse' even if they match options that
+	'git rev-parse' might otherwise recognise. As such,
+	this option, if specified, should always be the
+	last option specified.
 
 --no-flags::
-	Do not output flag parameters.
+	Do not output flag parameters. If specified, this
+	option causes any subsequent `--flags` option to be ignored.
 
 --default <arg>::
 	If there is no parameter given by the user, use `<arg>`

-- 
1.7.3.1.gc81ce.dirty