Re: [PATCH 2/2] reset: support the --stdin option

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +	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?

> +		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.