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

["Martin Ågren" <martin.agren@gmail.com>]

Hi Matthew,

On Wed, 20 Nov 2024 at 11:41, Matthew Bystrin <dev.mbstr@gmail.com> wrote:
>
> Calling commands using editor in terminal with `--paginate` option will
> break things. For example `git --paginate config --edit`. Add extra
> check to ignore paginate flag in case command have DELAY_PAGER_CONFIG
> set.
>
> Relates: cd878a206e8c (t7006: add tests for how git config paginates)
> Signed-off-by: Matthew Bystrin <dev.mbstr@gmail.com>
> ---

> Some time ago I've sent RFC patch [1], which was not quite ready, and I didn't
> receive any feedback. Now I'm sending a more complete version of it. Fixing
> mentioned behaviour of `note` command can be done in separate patch series.

Thanks for reposting, and welcome to the list.

> +       if (use_pager == 1 && (p->option & DELAY_PAGER_CONFIG))
> +               use_pager = 0;
>         if (run_setup && startup_info->have_repository)

This flag goes back to c409824cc2a ("git.c: let builtins opt for
handling `pager.foo` themselves", 2017-08-02). The original observation
was that setting `pager.tag` to true was more or less foolish -- it
would help `git tag --list`, sure, but would also break `git tag -a`
quite badly along the way since the latter wants to open an editor.

That's why c409824cc2a allowed builtins to basically say "delay pager
config handling -- I'll respect it for some options/modes, but not for
others". Now this proposed patch wants to extend that handling beyond
"delay pager *config*" to even go "for some options/modes, I'm going to
completely ignore `--paginate`." ("Delay pager *option*"?)

Actually, no, it's not so much ignoring as *forcing*. Since you force it
to 0, doesn't that mean that `--paginate` ends up basically being
`--no-pager`? So `git --paginate branch` is now `git --no-pager branch`?
That doesn't seem right. An optionless `git branch` would have
paginated, so adding `--paginate` shouldn't change anything.

But even if we force it to -1 instead (for "maybe"), I'm not sure I
understand why such an undoing of user intention is wanted. If I run
`git --paginate tag -a ...`, maybe that's just self-inflicted harm, but
are we certain that for all the `git foo --bar` which won't respect
`pager.foo`, that it's also completely crazy to provide `--paginate`?

Martin