Re: RFC: Using '--no-output-indicator-old' to only show new state

[Linus Torvalds <torvalds@linux-foundation.org>]

On Fri, Mar 11, 2022 at 12:43 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> If it's not too much trouble inline patches like those you send to the
> LKML would be preferred :)

Heh. When I send patches to LKML inline I end up doing whitespace
damage to them.

Part of the problem is that gmail - which is what I use for normal
email - makes it quite hard to send patches with the legacy tools I
have when you have set your account up to have all the security
measures (2FA etc).

In fact, I think these days gmail actively blocks any clients it
doesn't trust, even if you were to do the special app key thing (which
I have occasionally done).

Despite that, I find gmail very convenient for my workflow, but  the
two issues I have:

 (a) I'd like to have "inline attachments" for the web interface so
that I could attach a text file that would *not* get whitespace
damaged

 (b) the mobile client should have a "no html" mode (for when I'm on
the road and reply to lkml)

And I know a lot of people inside of google, but I gave up trying to
get the message through to the gmail people (everybody I knew said
"it's a different group and we have our own issues with them")

Yes, yes, I could use other tools - and other email setups - for
sending out patches, but realistically these days I never actually
send out patches any more. These kinds of very occasional git patches
are the exception, not the rule.

So, for example, I could trivially use "torvalds@kernel.org" to send
out patches, but I've tried to avoid having multiple active email
accounts and confusing people with different names etc.

My lkml patches are invariably examples (ie "how about we do it this
way") and then I actually mostly *prefer* that they be
whitespace-damaged so that people won't apply them mindlessly. In
fact, I often explicitly indent the patch to make it really obvious
that "yes, this is a patch, but it's meant to be thought about, not
used mindlessly".

> FWIW the reason... [...]
> ..you needed to do that is because we pass PARSE_OPT_KEEP_UNKNOWN
> parse_options() there, which turns off our abbreviation discovery logic,
> i.e. where we'll take a --foo, --foob, --fooba if we have a --foobar
> option defined.

Yeah, I had gotten to the PARSE_OPT_KEEP_UNKNOWN part, and then
decided I didn't understand why the option parsing code did that, and
I wasn't going to touch it.

I saw your patch to fix it, and it looked sane to me. Thanks.

> We really should just teach those callsites to "grab" the revisions.c
> options and do the parse in one pass,

I was assuming the two-pass thing to avoid doing any callbacks before
deciding the option is unambiguous.

Not that I see why that would be a big deal as long as you just error
out anyway, but maybe some users end up ignoring errors?

You know that code better than I do in any case.

> Of course that also makes --new and --old work (even --ne and --ol), but
> we generally accept that in other places, so... :)

I do wonder if abbreviation should have some limit (ie single-letter
abbreviation sounds very suspect even if they are unique, and even
two-letter ones sound odd), but it probably doesn't matter.

> I see this interacts nicely with the -I option, at least in my testing,
> i.e. (with my change applied) this will ignore the first hunk of your
> patch:
>
>     --new -p -I'sign|return'

So it wasn't really intentional - I saw it _purely_ as a "filter
output" thing, not as a "filter the patch generation".

In fact, I like the part where it often shows patch hunks that have no
actual changes, because the only changes in that hunk were removals.
To me, that's a feature - it shows the end result of the patch in that
place.

But it also means:

> What this doesn't interact "well" with, or perhaps it's what we actually
> want is e.g. -G:

Yeah, exactly because I saw it as purely a "filter the final output of
the patch" thing, it means that things that filter on the *contents*
of the patch aren't affected.

So that was my mental model, but yes, if your mental model is that
"-G" acts on the output, then it's a mis-feature.

IOW, to me, this would be a bigger decision about what kinds of
semantics you want.

Now, the *real* downside of "--new" to me is that it really doesn't
work very well with --word-diff. That's where I think it would really
make sense to still work just fine. Particularly with
"--word-diff=color" I think "--new" (and "--old") would make a lot of
sense, but my patch very much did *not* change the word-diff output
machinery to take the output_indicators into account.

A big reason for that is that while I occasionally use --word-diff
(and find it very useful when I do), I've never touched that code, so
it's entirely unfamiliar to me.

                      Linus