git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] revision: make pseudo-opt flags read via stdin behave consistently

From
Taylor Blau <me@ttaylorr.com>
Date
Sep 25, 2023, 22:47 UTC
Message-ID
<ZRIN9gJsRgG4qj5r@nand.local>
In-Reply-To
<ZRGBo1TsgSYrMt01@tanuki>
On Mon, Sep 25, 2023 at 02:48:35PM +0200, Patrick Steinhardt wrote:
Show 22 quoted lines
> > > diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> > > index a4a0cb93b2..9bf13bac53 100644
> > > --- a/Documentation/rev-list-options.txt
> > > +++ b/Documentation/rev-list-options.txt
> > > @@ -151,6 +151,8 @@ endif::git-log[]
> > >  --not::
> > >  	Reverses the meaning of the '{caret}' prefix (or lack thereof)
> > >  	for all following revision specifiers, up to the next `--not`.
> > > +	When used on the command line before --stdin, the revisions passed
> > > +	through stdin will not be affected by it.
> >
> > Hmmph. This seems to change the behavior introduced by 42cabc341c4. Am I
> > reading this correctly that tips on stdin with '--not --stdin' would not
> > be marked as UNINTERESTING?
> >
> > (Reading this back, there are a lot of double-negatives in what I just
> > wrote making it confusing for me at least. What I'm getting at here is
> > that I think `--not --stdin` should mark tips given via stdin as
> > UNINTERESTING).
>
> It does not change the behaviour, it only documents the current state
> such that it's at least spelled out somewhere.

Sorry, I must have been confused when writing this :-<. Looking at it again, I agree that the current behavior is that "--not --stdin" treats any tips given over stdin as INTERESTING, so this documentation is consistent with that.

Show 16 quoted lines
> > I wonder if we could instead do something like:
> >
> >   - inherit the current set of psuedo-opts when processing input over
> >     `--stdin`
> >   - allow pseudo-opts within the context of `--stdin` arbitrarily
> >   - prevent those psuedo-opts applied while processing `--stdin` from
> >     leaking over to subsequent command-line arguments.
> >
> > Here's one approach for doing that, where we make a copy of the current
> > set of flags when calling `read_revisions_from_stdin()` instead of
> > passing a pointer to the global state.
>
> That was indeed my first approach. But this change would break behaviour
> that was introduced with 42cabc341c4 (Teach rev-list an option to read
> revs from the standard input., 2006-09-05) almost 17 years ago. If we
> change it now then this is very likely to cause problems somewhere.

Per above, you're absolutely right. I think that the patch that you proposed here LGTM.

Thanks, Taylor

Previous: Patrick SteinhardtNext: Junio C Hamano
Message 4 of 7 in “revision: make pseudo-opt flags read via stdin behave consistently”
  1. revision: make pseudo-opt flags read via stdin behave consistentlyPatrick Steinhardt, Sep 21, 2023
  2. Taylor BlauSep 21, 2023
  3. Patrick SteinhardtSep 25, 2023
  4. Taylor BlauSep 25, 2023
  5. Junio C HamanoSep 21, 2023
  6. Patrick SteinhardtSep 25, 2023
  7. revision: make pseudo-opt flags read via stdin behave consistentlyPatrick Steinhardt, Sep 25, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.