git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: phillip.wood123@gmail.com
To: Junio C Hamano <gitster@pobox.com>
Cc: "Rubén Justo" <rjusto@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Dragan Simic" <dsimic@manjaro.org>, "Jeff King" <peff@peff.net>,
	"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v3 4/4] add-patch: render hunks through the pager
Date: Thu, 18 Jul 2024 10:56:19 +0100	[thread overview]
Message-ID: <6317c4a0-06b9-4e2c-9b23-10cb96950617@gmail.com> (raw)
In-Reply-To: <xmqqy15zzror.fsf@gitster.g>

On 17/07/2024 21:31, Junio C Hamano wrote:
> phillip.wood123@gmail.com writes:
> 
>>> -	test_write_lines P q | GIT_PAGER="head -n 1" test_terminal git add -p
>>> +	test_write_lines P q |
>>> +	(
>>> +		GIT_PAGER="head -n 1" &&
>>> +		export GIT_PAGER &&
>>> +		test_terminal git add -p >actual
>>> +	)
>>
>> That's surprising, why does running git in a sub-shell stop it from
>> segfaulting?
> 
> Another difference besides the sub-shell is where the output from
> test_terminal goes.  If the above change fixes the issue for Rubén,
> I wonder if it still works if ">actual" gets removed.

Yes that would be interesting to know. I do think we should be checking 
the output here to make sure the test is doing what we think it is - all 
we know at the moment is whether git exits successfully or not. If we 
receive SIGPIPE then fputs() will see EPIPE which will set ferror() 
which we should be clearing and checking that the prompt prints 
correctly afterwards. I think it would be helpful to add

tail -n3 actual >actual.trimmed &&
cat >expect <<\EOF &&
	\ No newline at end of file
	(1/1) Stage addition [y,n,q,a,d,e,p,?]? @@ -0,0 +1,2 @@
	(1/1) Stage addition [y,n,q,a,d,e,p,?]?
	EOF
test_cmp expect actual.trimmed

to the test. That all still leaves me wondering about the segfault though

Best Wishes

Phillip

  reply	other threads:[~2024-07-18  9:56 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-12  0:57 [PATCH 0/4] use the pager in 'add -p' Rubén Justo
2024-07-12  1:00 ` [PATCH 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-12  1:00 ` [PATCH 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-12  1:00 ` [PATCH 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-12 13:17   ` Phillip Wood
2024-07-12  1:00 ` [PATCH 4/4] add-patch: render hunks through the pager Rubén Justo
2024-07-12  8:58   ` Dragan Simic
2024-07-12 13:26   ` Phillip Wood
2024-07-12 16:24     ` Rubén Justo
2024-07-13  3:23       ` Rubén Justo
2024-07-13  9:12       ` Junio C Hamano
2024-07-13 13:17       ` phillip.wood123
2024-07-13 23:13         ` Rubén Justo
2024-07-12  8:56 ` [PATCH 0/4] use the pager in 'add -p' Dragan Simic
2024-07-13 16:26 ` [PATCH v2 " Rubén Justo
2024-07-13 16:29   ` [PATCH v2 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-13 16:29   ` [PATCH v2 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-13 16:29   ` [PATCH v2 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-13 16:30   ` [PATCH v2 4/4] add-patch: render hunks through the pager Rubén Justo
2024-07-13 17:08   ` [PATCH v2 0/4] use the pager in 'add -p' Junio C Hamano
2024-07-13 23:21     ` Rubén Justo
2024-07-14  1:18       ` Junio C Hamano
2024-07-14 16:00   ` [PATCH v3 " Rubén Justo
2024-07-14 16:04     ` [PATCH v3 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-14 16:04     ` [PATCH v3 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-14 16:04     ` [PATCH v3 4/4] add-patch: render hunks through the pager Rubén Justo
2024-07-15 14:10       ` Phillip Wood
2024-07-15 23:54       ` Junio C Hamano
2024-07-17 17:20         ` Rubén Justo
2024-07-17 19:39           ` phillip.wood123
2024-07-17 20:03             ` Junio C Hamano
2024-07-17 20:09               ` Eric Sunshine
2024-07-17 20:21                 ` Junio C Hamano
2024-07-20 22:37                 ` Rubén Justo
2024-07-22  7:18                   ` Eric Sunshine
2024-07-22 14:53                     ` Rubén Justo
2024-07-18  9:48               ` phillip.wood123
2024-07-17 20:31             ` Junio C Hamano
2024-07-18  9:56               ` phillip.wood123 [this message]
2024-07-20 22:39               ` Rubén Justo
2024-07-20 22:29             ` Rubén Justo
2024-07-22 10:18               ` Phillip Wood
2024-07-22 16:45                 ` Rubén Justo
2024-07-22 17:22               ` Junio C Hamano
2024-07-22 19:06                 ` Rubén Justo
2024-07-22 19:27                   ` Junio C Hamano
2024-07-22 21:06                     ` Re* " Junio C Hamano
2024-07-22 22:00                       ` Rubén Justo
2024-07-22 23:12                       ` Kyle Lippincott
2024-07-22 23:28                         ` Junio C Hamano
2024-07-23  2:31                         ` Junio C Hamano
2024-07-22 21:25                   ` Junio C Hamano
2024-07-22 23:20                     ` Rubén Justo
2024-07-22 23:24                       ` [PATCH 1/2] t3701: avoid one-shot export for shell functions Rubén Justo
2024-07-22 23:44                         ` Junio C Hamano
2024-07-22 23:57                           ` Junio C Hamano
2024-07-22 23:24                       ` [PATCH 2/2] pager: make wait_for_pager a no-op for "cat" Rubén Justo
2024-07-22 23:54                         ` Junio C Hamano
2024-07-18  9:58           ` [PATCH v3 4/4] add-patch: render hunks through the pager phillip.wood123
2024-07-20 22:45             ` Rubén Justo
2024-07-14 16:04     ` [PATCH v3 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-15 14:13       ` Phillip Wood
2024-07-15 20:04         ` Rubén Justo
2024-07-17 14:58           ` phillip.wood123
2024-07-15 20:16     ` [PATCH v4 0/4] add-patch: render hunks through the pager Rubén Justo
2024-07-15 20:20       ` [PATCH v4 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-15 20:21       ` [PATCH v4 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-15 20:21       ` [PATCH v4 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-15 20:22       ` [PATCH v4 4/4] add-patch: render hunks through the pager Rubén Justo

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=6317c4a0-06b9-4e2c-9b23-10cb96950617@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=rjusto@gmail.com \
    /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).