public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web•de>
To: Junio C Hamano <gitster@pobox•com>
Cc: Zapped <zapped@mail•ru>,
	git@vger•kernel.org,
	Johannes Schindelin <Johannes.Schindelin@gmx•de>
Subject: Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable
Date: Mon, 27 Dec 2010 12:14:25 +0100	[thread overview]
Message-ID: <4D187511.3090104@web.de> (raw)
In-Reply-To: <7vd3ooz6qd.fsf@alter.siamese.dyndns.org>

Am 26.12.2010 20:14, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web•de> writes:
> 
>> So are there any reasons for the plumbing diff commands not to honor
>> the diff.ignoreSubmodules setting?

Thank you for explaining those reasons in detail.

> One class of plumbing users is scripts that is about automation and
> mechanization that want to control what they do precisely (think cron
> jobs) without getting affected by the user preference stored in the
> repository configuration.  This class could either:
> 
>  (1) state what they want explicitly from the command line; or
>  (2) rely on built-in defaults not changing underneath them.
> 
> The behaviour of diff recursively inspecting submodule dirtiness has an
> unfortunate history, in that the behaviour changed over time, and in each
> step when we made a change, we thought we were making an unquestionable
> improvement.  Originally we only said "submodule HEAD is different from
> what we have in the index/superproject HEAD".  Later we added different
> kind of dirtiness like untracked files or modified contents in submodules,
> decided perhaps mistakenly that majority of users do want to see them as
> dirtiness and made that the default and allowed them to be ignored by an
> explicit request.  At that point, in order not to break existing scripts
> (mostly of the "mechanization" class, written back when there was no such
> extra dirtyness hence with no "explicit refusal" route available to them
> without rewriting), hence "no configuration should affect plumbing
> randomly" policy.

Good point. But unfortunately the diff plumbing commands are affected by
the "submodule.<name>.ignore" ignore settings I introduced in August in
aee9c7d6 and 302ad7a9. Maybe we should revert the part of these patches
that changed the plumbing commands?

> On the other hand, you may write user facing Porcelain in scripts and run
> plumbing from there.  This class of plumbing users could either:
> 
>  (1) inspect the config itself, interpret the customization and pass
>      an explicit command line flag; or
> 
>  (2) allow the plumbing honor the end user configuration stored in the
>      repository or user configuration files.
> 
> It is argurably more convenient for these users if the plumbing blindly
> honored the configurations, as it would have allowed the latter
> implementation.  That way, we can be more lazy when writing our scripts,
> and ignore having to worry about new kinds of customization added to
> underlying git after a script is written---but new kinds of customization
> may break your script's expectation of what will and what will not be made
> customizable, and you would end up giving an explicit "do not use that
> feature" in some cases, so the being able to be lazy is not necessarily
> always a win.

Agreed.

> Things may have been a bit different if the original feature change to
> inspect submodules deeper, command line flags to control that behaviour
> and configuration to default the flags came at the same time, but
> unfortunately they happend over time.  I think we have been slowly getting
> better at this, but in the case of this particular feature, the original
> introduction of --ignore-submodules was in May 2008, deeper submodule
> inspection and the richer --ignore-submodules=<kind> option came much
> later in June 2010, and the configuration was invented later in August
> 2010, which would mean that allowing the plumbing to honor configuration
> would have broken scripts written in the 2 years and 3 months period.
> 
> And no, this does not call for a blanket "do / do not honor configuration"
> option to plumbing commands.  A more selective "do / do not honor these
> configuration variables" option might be an option, though.

What about a new "--ignore-submodules=config" option to tell the plumbing
that it should honor the config?


And it looks like the PS1 problem that started this discussion is a
valid example for mixed usage of porcelain and plumbing commands. In a
first attempt to fix the problem by using "git diff --cached" instead
of "git diff-index --cached" I noticed that those two commands give
different results when new submodules were created and had been added
to the index. "git diff --cached" ignores them while "git diff-index
--cached" shows them. Anything I am missing here?

  parent reply	other threads:[~2010-12-27 11:15 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-25  1:20 [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal Zapped
2010-12-25  1:20 ` [PATCH 2/3] Fixes bug: git-svn: svn.pathnameencoding is not respected with dcommit/set-tree Zapped
2011-01-04 17:18   ` Thomas Rast
2011-01-04 23:20     ` Eric Wong
2011-02-03 20:28       ` Alexey Shumkin
2011-01-05 11:44     ` Re[2]: " Алексей Шумкин
2010-12-25  1:20 ` [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable Zapped
2010-12-25 12:33   ` Jens Lehmann
2010-12-25 13:08     ` Johannes Schindelin
2010-12-26 19:14     ` Junio C Hamano
2010-12-26 22:42       ` Re[2]: " Алексей Шумкин
2010-12-27 11:14       ` Jens Lehmann [this message]
2010-12-27 22:06         ` Johannes Schindelin
2010-12-27 22:43           ` Casey Dahlin
2010-12-26 22:25     ` Re[2]: " Алексей Крезов
2010-12-28 12:14     ` Алексей Шумкин
2011-01-04 17:13 ` [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal Thomas Rast
2011-01-05 11:53   ` Re[2]: " Алексей Шумкин
2011-01-05 14:23     ` Thomas Rast
2011-01-05 14:31       ` Thomas Rast

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=4D187511.3090104@web.de \
    --to=jens.lehmann@web$(echo .)de \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=zapped@mail$(echo .)ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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