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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 22 Nov 2024 at 19:13, Matthew Bystrin <dev.mbstr@xxxxxxxxx> wrote:
>
> > The whole thing started with
> >
> >     Calling commands using editor in terminal with `--paginate`
> >     option will break things. For example `git --paginate config
> >     --edit`.
> >
> > which many of us may respond with "it hurts? do not do it then", so
> > I agree with you that a fallout would be worse than the problem the
> > change is trying to "fix".
>
> I see the point and totally agree with it.
>
> 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. What
> do you think about that? Should I continue work on that despite the fact it does
> not really hurts? If yes, is it better to create new patch or send as an update
> of the current?

So the high-level idea here is "if we're going to open an editor, let's
first shut down any pager".

This helper is relatively recent, from e8bd8883fe (pager: introduce
wait_for_pager, 2024-07-25) and fc87b2f7c1 (add-patch: render hunks
through the pager, 2024-07-25). The context there seems to be that a
long-running git process wants to open the pager, maybe even more than
once. So we need a way to shut down the pager we just opened -- it's
done its thing.

What gives me a bit of pause here is that we're now approaching this
from a quite different direction: a git process wants to shut down the
pager not because it just opened it and the pager has done its thing,
but because we want to open an editor and think that any pager might
interfere.

If `git add -p` is a long-running process that wants to repeatedly
launch a pager, what could the opposite look like? A long-running
process that wants to repeatedly launch an editor? While paging?

Maybe `git rebase -i` could be one such example. TBH, running its output
through a pager might not look super-pretty today due to it producing
temporary output that it then erases, but at least it works. The editor
might be graphical (or a script?) that won't actually suffer from the
pager running. Killing the pager might be unwanted. So I don't know.

What is the original problem here? Is some kind of tooling issuing a
`git -p tag -a` where it's not possible to teach it to drop the `-p`?

Martin




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux