git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH/RFC 0/9] Setup cleanup, chapter one
Date: Mon, 12 Apr 2010 21:11:54 -0500	[thread overview]
Message-ID: <20100413021153.GA3978@progeny.tock> (raw)

These patches are designed to make people who want the pager.<cmd>
configuration to be more reliable a little happier.  More importantly,
they bring the setup procedure closer to doing the Right Thing™.

More precisely:

The pager configuration procedure does not always do as much as one
would like:

A. Unless one runs check_pager_config() first, the pager.<cmd>
   configuration is ignored.  The setup procedure does not run
   check_pager_config() for commands that do not use RUN_SETUP.

B. Unless one runs setup_repository_gently() first, functions that
   check configuration expect to find configuration in ./.git/config.
   This is wrong for two reasons: some commands should ignore the
   per-repository configuration, and most commands should be walking
   parent parent directories to find an appropriate .git or foo.git
   dir.  The upshot is that the per-repository configuration is
   ignored for pager.<cmd> and core.pager when git is run from the
   toplevel of a repository.

These patches are a modest attempt to start to ameliorate these
problems.  Patches 1, 2, and 4 introduce new tests to make sure
we are not making things worse.

Patch 3 delays startup of the pager.  By the time we actually need
to start a pager for commands with the RUN_SETUP option, the .git
directory will already have been found, addressing problem (B) for
those commands.

Patches 5 and 6 are something of a trojan horse: they introduce
infrastructure that I expect to be more widely useful after this
modest patch series.  That infrastructure is a collection of global
variables representing the state of the current repository, for
built-in commands only (for now).  The main benefit now is that it
lets us pass more data to builtins without changing the builtin API.

Patches 7 gives builtins the option to search for a repository
early even if they do not require a repository to work.  The benefits
of patch 3 will accrue to such commands just as though they used
RUN_SETUP.  Patch 8 addresses problem (A) for builtins using this new
RUN_SETUP_GENTLY option.

And patch 9 is an example command taking advantage of the opportunity.
I don’t imagine anyone was eagerly awaiting the ability to make their
‘git config’ automatically paginate.  The secondary benefits
(especially UI consistency) are in my opinion more important.

This series would not be possible without the hard work of Nguyễn Thái
Ngọc Duy.  It consists mostly of patches by him.  A footnote [1] contains
some context if you would like it.

Thoughts?

Jonathan Nieder (4):
  t7006: GIT_DIR/config should be honored in subdirs of toplevel
  t7006: test pager configuration for several git commands
  t7006: test pager.<cmd> configuration
  builtin: check pager.<cmd> configuration if RUN_SETUP_GENTLY is used

Nguyễn Thái Ngọc Duy (5):
  builtins: do not commit pager choice early
  builtin: introduce startup_info struct
  builtin: remember whether repository was found
  builtin: Support RUN_SETUP_GENTLY to set up repository early if found
  config: run setup before commiting pager choice

 builtin/config.c |    6 +-
 cache.h          |    6 ++
 environment.c    |    1 +
 git.c            |   23 +++++--
 setup.c          |   12 ++++-
 t/t7006-pager.sh |  168 +++++++++++++++++++++++++++++++++++++++++-------------
 6 files changed, 165 insertions(+), 51 deletions(-)

[1] I warned that something like this was coming at
<http://thread.gmane.org/gmane.comp.version-control.git/144090>.

I will send a rebased version of the rest of the nd/setup series to
the list as a follow-up.  But beware: after perhaps introducing all
kinds of bugs through repeated rebasing, I didn’t give the later
patches of the series another glance.

             reply	other threads:[~2010-04-13  2:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-13  2:11 Jonathan Nieder [this message]
2010-04-13  2:13 ` [PATCH 1/9] t7006: GIT_DIR/config should be honored in subdirs of toplevel Jonathan Nieder
2010-04-14 20:50   ` Junio C Hamano
2010-04-15  0:38     ` [PATCH] t7006: guard cleanup with test_expect_success Jonathan Nieder
2010-04-15  0:56       ` Junio C Hamano
2010-04-15  1:27         ` Jonathan Nieder
2010-04-13  2:17 ` [PATCH 2/9] t7006: test pager configuration for several git commands Jonathan Nieder
2010-04-13  6:34   ` Johannes Sixt
2010-04-13 22:07     ` Jonathan Nieder
2010-04-14  1:26   ` [PATCH 2/9 v2] " Jonathan Nieder
2010-04-13  2:24 ` [PATCH 3/9] builtins: do not commit pager choice early Jonathan Nieder
2010-04-14  2:17   ` [PATCH 3/9 v2] " Jonathan Nieder
2010-04-13  2:25 ` [PATCH 4/9] t7006: test pager.<cmd> configuration Jonathan Nieder
2010-04-14  2:19   ` [PATCH 4/9 v2] " Jonathan Nieder
2010-04-13  2:27 ` [PATCH 5/9] builtin: introduce startup_info struct Jonathan Nieder
2010-04-13  2:28 ` [PATCH 6/9] builtin: remember whether repository was found Jonathan Nieder
2010-04-13  2:29 ` [PATCH 7/9] builtin: Support RUN_SETUP_GENTLY to set up repository early if found Jonathan Nieder
2010-04-13  2:30 ` [PATCH 8/9] builtin: check pager.<cmd> configuration if RUN_SETUP_GENTLY is used Jonathan Nieder
2010-04-13  5:29   ` Nguyen Thai Ngoc Duy
2010-04-14  4:38     ` Jonathan Nieder
2010-04-13 10:12   ` Nguyen Thai Ngoc Duy
2010-04-14  5:06     ` Jonathan Nieder
2010-04-15  8:33       ` Nguyen Thai Ngoc Duy
     [not found]         ` <20100415084925.GA14660@progeny.tock>
2010-04-15 17:43           ` Nguyen Thai Ngoc Duy
2010-04-13  2:31 ` [PATCH 9/9] config: run setup before commiting pager choice Jonathan Nieder
2010-04-14  2:23   ` [PATCH 9/9 v2] " Jonathan Nieder
2010-04-13  3:08 ` [RFC/PATCH 00/46] nd/setup remainder for convenient reference Jonathan Nieder
2010-04-14  7:59   ` Nguyen Thai Ngoc Duy
2010-04-14 20:54 ` [PATCH/RFC 0/9] Setup cleanup, chapter one Junio C Hamano
2010-04-15  0:05   ` Jonathan Nieder

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=20100413021153.GA3978@progeny.tock \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.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).