git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH 0/4] squash fixups in rj/add-p-pager
Date: Thu, 25 Jul 2024 15:42:07 +0200	[thread overview]
Message-ID: <76936fb1-446d-455f-b4e7-6e24dda3c17d@gmail.com> (raw)
In-Reply-To: <88286ad9-eab7-4461-a407-898737faa6a1@gmail.com>

Here is the series that squashes the fixups in rj/add-p-pager.

I don't have a strong preference for this over what's already in
rj/add-p-pager, but let's go through the changes Phillip has suggested,
even if it's just to archive them in the list.  

Thanks.

Rubén Justo (4):
  add-patch: test for 'p' command
  pager: do not close fd 2 unnecessarily
  pager: introduce wait_for_pager
  add-patch: render hunks through the pager

 add-patch.c                | 18 +++++++++++---
 pager.c                    | 48 ++++++++++++++++++++++++++++++++++----
 pager.h                    |  1 +
 t/t3701-add-interactive.sh | 48 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 8 deletions(-)

Range-diff:
1:  5fdfd2f3bd = 1:  9f358c6d69 add-patch: test for 'p' command
2:  506f457e48 = 2:  f45a7ca9b2 pager: do not close fd 2 unnecessarily
3:  b29c59e3d2 ! 3:  9d7a50e531 pager: introduce wait_for_pager
    @@ Commit message
         In the interactive commands (i.e.: add -p) we want to use the pager for
         some output, while maintaining the interaction with the user.
     
    -    Modify the pager machinery so that we can use setup_pager and, once
    +    Modify the pager machinery so that we can use `setup_pager()` and, once
         we've finished sending the desired output for the pager, wait for the
    -    pager termination using a new function wait_for_pager.   Make this
    +    pager termination using a new function `wait_for_pager()`.  Make this
         function reset the pager machinery before returning.
     
    +    One specific point to note is that we avoid forking the pager in
    +    `setup_pager()` if the configured pager is an empty string [*1*] or
    +    simply "cat" [*2*].  In these cases, `setup_pager()` does nothing and
    +    therefore `wait_for_pager()` should not be called.
    +
    +    We could modify `setup_pager()` to return an indication of these
    +    situations, so we could avoid calling `wait_for_pager()`.
    +
    +    However, let's avoid transferring that responsibility to the caller and
    +    instead treat the call to `wait_for_pager()` as a no-op when we know we
    +    haven't forked the pager.
    +
    +       1.- 402461aab1 (pager: do not fork a pager if PAGER is set to empty.,
    +                       2006-04-16)
    +
    +       2.- caef71a535 (Do not fork PAGER=cat, 2006-04-16)
    +
         Signed-off-by: Rubén Justo <rjusto@gmail.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## pager.c ##
     @@ pager.c: int pager_use_color = 1;
    @@ pager.c: static void wait_for_pager_atexit(void)
     +
     +void wait_for_pager(void)
     +{
    ++	if (old_fd1 == -1)
    ++		return;
    ++
     +	finish_pager();
     +	sigchain_pop_common();
     +	unsetenv("GIT_PAGER_IN_USE");
4:  6bc52a5543 ! 4:  6f4990c0d4 add-patch: render hunks through the pager
    @@ Commit message
         this limit.
     
         Signed-off-by: Rubén Justo <rjusto@gmail.com>
    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## add-patch.c ##
     @@
    @@ t/t3701-add-interactive.sh: test_expect_success 'print again the hunk' '
     +	test_when_finished "rm -f huge_file; git reset" &&
     +	printf "\n%2500000s" Y >huge_file &&
     +	git add -N huge_file &&
    -+	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
    ++	)
     +'
     +
      test_expect_success 'split hunk "add -p (edit)"' '
5:  b7637a9f21 < -:  ---------- t3701: avoid one-shot export for shell functions
6:  4b53ff8c0e < -:  ---------- pager: make wait_for_pager a no-op for "cat"

base-commit: a7dae3bdc8b516d36f630b12bb01e853a667e0d9
-- 
2.46.0.rc0.4.g6f4990c0d4

  reply	other threads:[~2024-07-25 13:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-23  0:39 [PATCH v2 0/2] add-p P fixups Rubén Justo
2024-07-23  0:42 ` [PATCH v2 1/2] t3701: avoid one-shot export for shell functions Rubén Justo
2024-07-23  0:42 ` [PATCH v2 2/2] pager: make wait_for_pager a no-op for "cat" Rubén Justo
2024-07-23  0:54 ` [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-23  9:15 ` Phillip Wood
2024-07-23 16:52   ` Junio C Hamano
2024-07-23 22:08   ` Rubén Justo
2024-07-24 15:21     ` phillip.wood123
2024-07-24 16:12       ` Rubén Justo
2024-07-25  9:45         ` Phillip Wood
2024-07-25 12:16           ` Rubén Justo
2024-07-25 13:42             ` Rubén Justo [this message]
2024-07-25 13:44               ` [PATCH 1/4] add-patch: test for 'p' command Rubén Justo
2024-07-25 13:44               ` [PATCH 2/4] pager: do not close fd 2 unnecessarily Rubén Justo
2024-07-25 13:44               ` [PATCH 3/4] pager: introduce wait_for_pager Rubén Justo
2024-07-25 13:44               ` [PATCH 4/4] add-patch: render hunks through the pager Rubén Justo
2024-07-25 15:24           ` [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-25 16:41             ` Re* " Rubén Justo
2024-07-25 16:43               ` [PATCH 1/2] pager: introduce wait_for_pager Rubén Justo
2024-07-25 16:43               ` [PATCH 2/2] add-patch: render hunks through the pager Rubén Justo
2024-07-26 18:36                 ` Junio C Hamano
2024-07-27  1:40                   ` Junio C Hamano
2024-07-27 14:33                     ` Rubén Justo
2024-07-26 18:24               ` Re* [PATCH v2 0/2] add-p P fixups Junio C Hamano
2024-07-26 19:22                 ` Rubén Justo
2024-07-26 19:48                   ` Junio C Hamano
2024-07-26 20:16                     ` Rubén Justo
2024-07-28  9:11                       ` Rubén Justo
2024-07-29 18:45                         ` 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=76936fb1-446d-455f-b4e7-6e24dda3c17d@gmail.com \
    --to=rjusto@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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).