git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Y5 via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Y5 <y5c4l3@proton.me>,
	y5c4l3 <y5c4l3@proton.me>
Subject: [PATCH] diff: setup pager only before diff contents truly ready
Date: Sat, 19 Oct 2024 20:39:50 +0000	[thread overview]
Message-ID: <pull.1817.git.git.1729370390416.gitgitgadget@gmail.com> (raw)

From: y5c4l3 <y5c4l3@proton.me>

git-diff setups pager at an early stage in cmd_diff; running diff with
invalid options like git diff --invalid will unexpectedly starts a
pager, which causes behavior inconsistency.

The pager setup routine should be moved right before the real diff
contents, in case there is any argv error.

Signed-off-by: y5c4l3 <y5c4l3@proton.me>
---
    diff: setup pager only before diff contents truly ready

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1817%2Fy5c4l3%2Fdiff-invalid-argv-remove-pager-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1817/y5c4l3/diff-invalid-argv-remove-pager-v1
Pull-Request: https://github.com/git/git/pull/1817

 builtin/diff.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index dca52d4221e..03340173700 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -105,6 +105,7 @@ static void builtin_diff_b_f(struct rev_info *revs,
 		     1, 0,
 		     blob[0]->path ? blob[0]->path : path,
 		     path);
+	setup_diff_pager(&revs->diffopt);
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 }
@@ -129,6 +130,7 @@ static void builtin_diff_blobs(struct rev_info *revs,
 		     &blob[0]->item->oid, &blob[1]->item->oid,
 		     1, 1,
 		     blob_path(blob[0]), blob_path(blob[1]));
+	setup_diff_pager(&revs->diffopt);
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
 }
@@ -164,6 +166,7 @@ static void builtin_diff_index(struct rev_info *revs,
 	} else if (repo_read_index(the_repository) < 0) {
 		die_errno("repo_read_cache");
 	}
+	setup_diff_pager(&revs->diffopt);
 	run_diff_index(revs, option);
 }
 
@@ -201,6 +204,7 @@ static void builtin_diff_tree(struct rev_info *revs,
 		oid[swap] = &ent0->item->oid;
 		oid[1 - swap] = &ent1->item->oid;
 	}
+	setup_diff_pager(&revs->diffopt);
 	diff_tree_oid(oid[0], oid[1], "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 }
@@ -227,6 +231,7 @@ static void builtin_diff_combined(struct rev_info *revs,
 		if (i != first_non_parent)
 			oid_array_append(&parents, &ent[i].item->oid);
 	}
+	setup_diff_pager(&revs->diffopt);
 	diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);
 	oid_array_clear(&parents);
 }
@@ -283,6 +288,7 @@ static void builtin_diff_files(struct rev_info *revs, int argc, const char **arg
 				    0) < 0) {
 		die_errno("repo_read_index_preload");
 	}
+	setup_diff_pager(&revs->diffopt);
 	run_diff_files(revs, options);
 }
 
@@ -523,8 +529,6 @@ int cmd_diff(int argc,
 	rev.diffopt.flags.recursive = 1;
 	rev.diffopt.rotate_to_strict = 1;
 
-	setup_diff_pager(&rev.diffopt);
-
 	/*
 	 * Do we have --cached and not have a pending object, then
 	 * default to HEAD by hand.  Eek.

base-commit: 15030f9556f545b167b1879b877a5d780252dc16
-- 
gitgitgadget

             reply	other threads:[~2024-10-19 20:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-19 20:39 Y5 via GitGitGadget [this message]
2024-10-19 20:57 ` [PATCH] diff: setup pager only before diff contents truly ready Kristoffer Haugsbakk
2024-10-21  0:17   ` Philip Yung
2024-10-19 21:19 ` Jeff King
2024-10-21  0:11   ` Philip Yung
2024-10-21 19:00     ` Jeff King
2024-10-21 19:38       ` Taylor Blau
2024-11-18  0:55         ` Junio C Hamano
2024-11-25 11:31           ` Jeff King
2024-12-03  5:33             ` Junio C Hamano
2024-12-03 21:24               ` Jeff King
2024-12-04  0:18                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1817.git.git.1729370390416.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=y5c4l3@proton.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).