Hi Junio,
On Tue, 11 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> 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?
Oh right. Copy/paste fail.
> > + 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...
It does, doesn't it? I really would have loved to start resetting right
away, but if the list were not sorted and traversed at the same time as
the tree-ish, the performance would just be suboptimal.
I think that is an important point and I adjusted the commit message
accordingly.
> > + 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).
True. I adjust the flags accordingly now.
Thanks,
Dscho