From: "Frans Klaver" <fransklaver@gmail.com>
To: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>
Cc: "Alexey Shumkin" <Alex.Crezoff@gmail.com>, git@vger.kernel.org
Subject: Re: BUG. Git config pager when --edit
Date: Mon, 07 Nov 2011 21:45:02 +0100 [thread overview]
Message-ID: <op.v4lfxck60aolir@keputer.lokaal> (raw)
In-Reply-To: <20111107171800.GA3621@sigill.intra.peff.net>
On Mon, 07 Nov 2011 18:18:00 +0100, Jeff King <peff@peff.net> wrote:
>> I was actually hoping that you won't go that route, but the route to
>> push
>> further to decide/spawn pager as late as possible. Clearly no sane
>> person
>> would want to run --edit subcommand under pager and "pager.config =
>> less"
>> should just be ignored in such a case.
>
> The problem with that is that it dumps the responsibility for running
> the pager to every subcommand. For builtins, we can have a flag that
> says "respect the pager.log config" or "foo will handle this itself;
> don't respect pager.tag".
>
> But what about externals? If "pager.stash" does nothing in git.c, and
> leaves it to "git-stash.sh" to start the pager if and when it's
> appropriate, then what about my personal "git-foo" that I drop into my
> PATH? Now I can't use "config.foo" without carrying code to do so in my
> external command.
>
> Maybe that's an OK tradeoff. But it's more of a pain for existing
> scripts, and it's not backwards compatible. What do you think?
For both cases there's something to say. In any new design I might dump
the responsibility on the external, but I would prefer to keep the
decision logic centralized. But as I understand, removing the
responsibility from git.c is going to require a whole bunch of other
changes to get the pager functional again in the scripts. So if there is a
somewhat decent way to be sure about whether or not to use the pager (i.e.
no editing) in git.c, why not keep it there? If, on the other hand, the
code is going to turn out to be a big hack, I'd say move it out.
Frans
next prev parent reply other threads:[~2011-11-07 20:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-07 13:26 BUG. Git config pager when --edit Alexey Shumkin
2011-11-07 13:43 ` Frans Klaver
2011-11-07 16:42 ` Jeff King
2011-11-07 17:02 ` Junio C Hamano
2011-11-07 17:18 ` Jeff King
2011-11-07 20:45 ` Frans Klaver [this message]
2011-11-07 14:54 ` how to merge sub directory or file? Emily
2011-11-07 15:37 ` Konstantin Khomoutov
2011-11-08 7:08 ` Emily Ren
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=op.v4lfxck60aolir@keputer.lokaal \
--to=fransklaver@gmail.com \
--cc=Alex.Crezoff@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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).