Re: [PATCH] git: fix paginate handling for commands with DELAY_PAGER_CONFIG

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 25, 2024, 02:21 UTC
Message-ID: <xmqqa5do834m.fsf@gitster.g>
In-Reply-To: <D5SWT5JKU6S9.2LH8H7ECRZXXM@gmail.com>

"Matthew Bystrin" <dev.mbstr@gmail.com> writes:

Show 27 quoted lines

> The root of the 'problem' is related with editor, not with commands. So maybe it
> is a good way to deal with it in editor code? I've quickly come up with
> something like this:
>
> diff --git a/editor.c b/editor.c
> index 6b9ce81d5f..04a1f71694 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -13,6 +13,7 @@
>  #include "strvec.h"
>  #include "run-command.h"
>  #include "sigchain.h"
> +#include "pager.h"
>  
>  #ifndef DEFAULT_EDITOR
>  #define DEFAULT_EDITOR "vi"
> @@ -60,6 +61,9 @@ const char *git_sequence_editor(void)
>  static int launch_specified_editor(const char *editor, const char *path,
>  				   struct strbuf *buffer, const char *const *env)
>  {
> +	if (pager_in_use())
> +		wait_for_pager();
> +
>  	if (!editor)
>  		return error("Terminal is dumb, but EDITOR unset");
>
> Brief testing shows what it works, but more complex approach may be needed.

I am not sure what you mean by "it works", and I do not really understand what you meant by "break things" in the original claim in the proposed commit log message, either.

    Calling commands using editor in terminal with `--paginate` option will
    break things. For example `git --paginate config --edit`.

The last sentence is incomplete and it is unclear which part of what `git -p config -e' does (or does not do) is considered "breaks things".

I cannot tell if the simplified change above (which is far nicer than sprinkling code to tweak use_pager variable everywhere) is doing the right thing without knowing what definition of "working" (and hence "breaking") you are using when you report that the above change "works".

Stepping back a bit, whatever code change your next version contains, please make sure that the proposed log message that explains your change tells the readers what you see, and what you would want to see in the ideal world (with a fix), and what the differences between these two are, instead of just saying "currently it breaks, so let's fix it".

Thanks.