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

* RFC: Using '--no-output-indicator-old' to only show new state
@ 2022-03-10 19:40 Linus Torvalds
  2022-03-10 20:13 ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Linus Torvalds @ 2022-03-10 19:40 UTC (permalink / raw)
  To: Junio Hamano C; +Cc: Git List Mailing

[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]

So after another round of doing

     git diff | grep -v '^-'

to just show what the end result of a patch is, I decided that there
has to be a better way.

Of course, to normal people, that "better way" is probably some GUI
tool that shows diffs as "before/after" in two different frames next
to each other, but I'm a grumpy old man ("Get off my lawn") and I do
everything but read my email in a standard text-only terminal.

So this RFC is that better way for me.

It's a RFC because maybe people think this is stupid, but even more so
becasue I think it should also go with an actual new and more legible
name - which this tech demonstration does *not* do, because I suspect
people have more opinions on the option name than they have on the
trivial technical parts.

IOW, with this patch, you can do

    git show --no-output-indicator-old

and it shows the diff but without the 'old' lines. That option name is
horrible to write, though, and even with command line completion I
think it migth be worth doing as

   git show --new

or something like that.

When would you want this? The case _I_ tend to use that "grep -v '^-'"
thing is when I re-write a function, and a regular diff really is just
very messy and what I really want to see is just the end result to
verify that "yeah, this looks sane".

In case people care, the immediate (current) cause of this was kernel
commit fe673d3f5bf1 ("mm: gup: make fault_in_safe_writeable() use
fixup_user_fault()"), which really is a fairly messy diff, but what
matters is the much simplified end result of the
fault_in_safe_writeable() function.

But in the commit message, I put a random git commit as an example
instead, just to make it easier to see what's up when you're a git
developer, not a kernel developer.

And hey, there are probably smarter things that could be done: this
doesn't work with word-diffs, for example. Maybe people would want an
equivalent "don't show the deleted word" instead. But this is really
really simple.

And maybe there already was some better way to do this.

Comments?

(And for consistency, this obviously also allows
"--no-output-indicator-new", but I've never found _that_ particularly
useful. But GUI diff tools most certainly also tend to show the old
state on one side, so maybe it's would be reasonable to use as a
"compare the old state vs the new state" in terminals side-by-side. I
just tend to care much more about "what is the end result")

               Linus

[-- Attachment #2: 0001-Allow-no-output-indicator-old-new-to-disable-diff-ou.patch --]
[-- Type: text/x-patch, Size: 2660 bytes --]

From 95e926c38267eb7ec8956ea47db55bf8080fb282 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 10 Mar 2022 11:00:43 -0800
Subject: [PATCH 1/2] Allow '--no-output-indicator-{old,new}' to disable diff
 output

This is particularly useful if you want to just see the end result of a
diff, without the original lines that have been removed (or the reverse).

That's a fairly common model in various GUI diff viewers, but it's not
unusual to want to just see "what is the end result of my changes"
without seeing (a) everything that didn't change and (b) the old removed
state.

Example:

    git show --no-output-indicator-old 04bf052eef

to see just the end result of the changes in commit 04bf052eef ("grep:
simplify config parsing and option parsing").

This is a technology presentation, I think it needs a shorter option
name too if people agree that this is useful (this basically replaces my
hacky

    git diff | grep -v '^-'

that I use for the same purpose, but the very long argument name
obviously makes it no more convenient, although the end result is
qualitatively better).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 diff.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 2bd5e0d817..895951b849 100644
--- a/diff.c
+++ b/diff.c
@@ -1254,6 +1254,8 @@ static void emit_line_ws_markup(struct diff_options *o,
 	const char *ws = NULL;
 	int sign = o->output_indicators[sign_index];
 
+	if (!sign)
+		return;
 	if (o->ws_error_highlight & ws_rule) {
 		ws = diff_get_color_opt(o, DIFF_WHITESPACE);
 		if (!*ws)
@@ -4986,6 +4988,10 @@ static int diff_opt_char(const struct option *opt,
 {
 	char *value = opt->value;
 
+	if (unset) {
+		*value = 0;
+		return 0;
+	}
 	BUG_ON_OPT_NEG(unset);
 	if (arg[1])
 		return error(_("%s expects a character, got '%s'"),
@@ -5476,12 +5482,12 @@ static void prep_parse_options(struct diff_options *options)
 			       &options->output_indicators[OUTPUT_INDICATOR_NEW],
 			       N_("<char>"),
 			       N_("specify the character to indicate a new line instead of '+'"),
-			       PARSE_OPT_NONEG, diff_opt_char),
+			       0, diff_opt_char),
 		OPT_CALLBACK_F(0, "output-indicator-old",
 			       &options->output_indicators[OUTPUT_INDICATOR_OLD],
 			       N_("<char>"),
 			       N_("specify the character to indicate an old line instead of '-'"),
-			       PARSE_OPT_NONEG, diff_opt_char),
+			       0, diff_opt_char),
 		OPT_CALLBACK_F(0, "output-indicator-context",
 			       &options->output_indicators[OUTPUT_INDICATOR_CONTEXT],
 			       N_("<char>"),
-- 
2.35.1.459.gabbef95d73


^ permalink raw reply related	[flat|nested] 11+ messages in thread