Re: [PATCH 2/2] reset: support the --stdin option
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Oct 12, 2016, 12:57 UTC
- Message-ID
- <alpine.DEB.2.20.1610121449190.3492@virtualbox>
- In-Reply-To
- <xmqqeg3mai1b.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Tue, 11 Oct 2016, Junio C Hamano wrote:
Show 20 quoted lines
> 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?Oh right. Copy/paste fail.
Show 18 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...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.
Show 11 quoted lines
> > + 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