Re: [PATCH 2/2] reset: support the --stdin option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 11, 2016, 17:53 UTC
- Message-ID
- <xmqqeg3mai1b.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <2c7a52e43be710c7f37c4886629bda38df183c21.1476202100.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 16 quoted lines
> + if (read_from_stdin) {
> + strbuf_getline_fn getline_fn = nul_term_line ?
> + strbuf_getline_nul : strbuf_getline_lf;
> + int flags = PATHSPEC_PREFER_FULL |
> + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
> + struct strbuf buf = STRBUF_INIT;
> + struct strbuf unquoted = STRBUF_INIT;
> +
> + if (patch_mode)
> + die(_("--stdin is incompatible with --patch"));
> +
> + if (pathspec.nr)
> + die(_("--stdin is incompatible with path arguments"));
> +
> + if (patch_mode)
> + flags |= PATHSPEC_PREFIX_ORIGIN;Didn't we already die above under that mode?
Show 16 quoted lines
> + while (getline_fn(&buf, stdin) != EOF) {
> + if (!nul_term_line && buf.buf[0] == '"') {
> + strbuf_reset(&unquoted);
> + if (unquote_c_style(&unquoted, buf.buf, NULL))
> + die(_("line is badly quoted"));
> + strbuf_swap(&buf, &unquoted);
> + }
> + ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
> + stdin_paths[stdin_nr++] = xstrdup(buf.buf);
> + strbuf_reset(&buf);
> + }
> + strbuf_release(&unquoted);
> + strbuf_release(&buf);
> +
> + ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
> + stdin_paths[stdin_nr++] = NULL;It makes sense to collect, but...
> + parse_pathspec(&pathspec, 0, flags, prefix, > + (const char **)stdin_paths);
...letting them be used as if they are pathspec is wrong when stdin_paths[] contain wildcard, isn't it?
I think flags |= PATHSPEC_LITERAL_PATH can help fixing it. 0/2 said this mimicks checkout-index and I think it should by not treating the input as wildcarded patterns (i.e. "echo '*.c' | reset --stdin" shouldn't be the way to reset all .c files --- that's something we would want to add to the test, I guess).
Thanks.