Hi,
this is the second version of my patch series to support handling of
pseudo-opts like `--all` or `--glob=` in both git-rev-list(1)'s and
git-log(1)'s `--stdin` mode.
Changes compared to v1:
- Patch 2/3: Changed the small refactoring to hoist out the check
for "--" out of the `if (sb.buf[0] == '-')` block completely to
make for an easier read.
- Patch 3/3: Implemented support for `--end-of-options` so that it
becomes possible to ask for references that have a leading dash.
- Patch 3/3: Fixed an issue where invalid pseudo-opts (e.g.
`--no-walk=garbage`) would not be recognized.
This is based on Junio's feedback, thank you!
Patrick
Patrick Steinhardt (3):
revision: reorder `read_revisions_from_stdin()`
revision: small readability improvement for reading from stdin
revision: handle pseudo-opts in `--stdin` mode
Documentation/rev-list-options.txt | 9 ++--
revision.c | 82 +++++++++++++++++-------------
t/t6017-rev-list-stdin.sh | 51 ++++++++++++++++++-
3 files changed, 103 insertions(+), 39 deletions(-)
Range-diff against v1:
1: 6cd4f79482 = 1: 6cd4f79482 revision: reorder `read_revisions_from_stdin()`
2: 38c0415ee9 ! 2: 5c1a9a0d08 revision: small readability improvement for reading from stdin
@@ Commit message
revision: small readability improvement for reading from stdin
The code that reads lines from standard input manually compares whether
- the read line matches "--", which is a bit awkward to read. Refactor it
- to instead use strcmp(3P) for a small code style improvement.
+ the read line matches "--", which is a bit awkward to read. Furthermore,
+ we're about to extend the code to also support reading pseudo-options
+ via standard input, requiring more elaborate handling of lines with a
+ leading dash.
+
+ Refactor the code by hoisting out the check for "--" outside of the
+ block that checks for a leading dash.
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
@@ revision.c: static void read_revisions_from_stdin(struct rev_info *revs,
- int len = sb.len;
- if (!len)
+ if (!sb.len)
- break;
++ break;
+
- if (sb.buf[0] == '-') {
++ if (!strcmp(sb.buf, "--")) {
++ seen_dashdash = 1;
+ break;
+- if (sb.buf[0] == '-') {
- if (len == 2 && sb.buf[1] == '-') {
-+ if (!strcmp(sb.buf, "--")) {
- seen_dashdash = 1;
- break;
- }
-+
- die("options not supported in --stdin mode");
+- seen_dashdash = 1;
+- break;
+- }
+- die("options not supported in --stdin mode");
}
++
++ if (sb.buf[0] == '-')
++ die("options not supported in --stdin mode");
+
if (handle_revision_arg(sb.buf, revs, 0,
REVARG_CANNOT_BE_FILENAME))
3: 4575917127 ! 3: 0cf189b378 revision: handle pseudo-opts in `--stdin` mode
@@ revision.c: static int handle_revision_pseudo_opt(struct rev_info *revs,
{
struct strbuf sb;
int seen_dashdash = 0;
++ int seen_end_of_options = 0;
+ int save_warning;
+
+ save_warning = warn_on_object_refname_ambiguity;
@@ revision.c: static void read_revisions_from_stdin(struct rev_info *revs,
break;
+ }
- if (sb.buf[0] == '-') {
-+ const char *argv[2] = { sb.buf, NULL };
-+
- if (!strcmp(sb.buf, "--")) {
- seen_dashdash = 1;
- break;
- }
-
+- if (sb.buf[0] == '-')
- die("options not supported in --stdin mode");
-+ if (handle_revision_pseudo_opt(revs, argv, flags))
++ if (!seen_end_of_options && sb.buf[0] == '-') {
++ const char *argv[] = { sb.buf, NULL };
++
++ if (!strcmp(sb.buf, "--end-of-options")) {
++ seen_end_of_options = 1;
+ continue;
++ }
+
-+ die(_("option '%s' not supported in --stdin mode"), sb.buf);
- }
++ if (handle_revision_pseudo_opt(revs, argv, flags) > 0)
++ continue;
++
++ die(_("invalid option '%s' in --stdin mode"), sb.buf);
++ }
if (handle_revision_arg(sb.buf, revs, 0,
+ REVARG_CANNOT_BE_FILENAME))
@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
}
if (revs->read_from_stdin++)
@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *re
## t/t6017-rev-list-stdin.sh ##
+@@ t/t6017-rev-list-stdin.sh: test_expect_success setup '
+ git add file-$i &&
+ test_tick &&
+ git commit -m side-$i || exit
+- done
++ done &&
++
++ git update-ref refs/heads/-dashed-branch HEAD
+ )
+ '
+
@@ t/t6017-rev-list-stdin.sh: check side-1 ^side-7 -- file-2
check side-3 ^side-4 -- file-3
check side-3 ^side-2
@@ t/t6017-rev-list-stdin.sh: check side-1 ^side-7 -- file-2
+check --glob=refs/heads
+check --glob=refs/heads --
+check --glob=refs/heads -- file-1
++check --end-of-options -dashed-branch
test_expect_success 'not only --stdin' '
cat >expect <<-EOF &&
@@ t/t6017-rev-list-stdin.sh: test_expect_success 'not only --stdin' '
+ test_cmp expect error
+'
+
-+test_expect_success 'unknown options' '
++test_expect_success 'pseudo-opt with invalid value' '
++ cat >input <<-EOF &&
++ --no-walk=garbage
++ EOF
++
++ cat >expect <<-EOF &&
++ error: invalid argument to --no-walk
++ fatal: invalid option ${SQ}--no-walk=garbage${SQ} in --stdin mode
++ EOF
++
++ test_must_fail git rev-list --stdin <input 2>error &&
++ test_cmp expect error
++'
++
++test_expect_success 'unknown option without --end-of-options' '
+ cat >input <<-EOF &&
-+ --unknown=value
++ -dashed-branch
+ EOF
+
+ cat >expect <<-EOF &&
-+ fatal: option ${SQ}--unknown=value${SQ} not supported in --stdin mode
++ fatal: invalid option ${SQ}-dashed-branch${SQ} in --stdin mode
+ EOF
+
+ test_must_fail git rev-list --stdin <input 2>error &&
--
2.41.0
Attachment:
signature.asc
Description: PGP signature
