Thread (5 messages) 5 messages, 2 authors, 2023-09-25

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

From: Taylor Blau <hidden>
Date: 2023-09-25 22:47:23

On Mon, Sep 25, 2023 at 02:48:35PM +0200, Patrick Steinhardt wrote:
quoted
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help