Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 25.12.2010 02:20, schrieb Zapped:
> Signed-off-by: Zapped <zapped@mail.ru>
> ---
>  contrib/completion/git-completion.bash |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d3037fc..50fc385 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -280,7 +280,8 @@ __git_ps1 ()
>  		elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
>  			if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
>  				if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
> -					git diff --no-ext-diff --quiet --exit-code || w="*"
> +					is=$(git config diff.ignoreSubmodules)
> +					git diff --no-ext-diff --quiet --exit-code --ignore-submodules=$is || w="*"
>  					if git rev-parse --quiet --verify HEAD >/dev/null; then
>  						git diff-index --cached --quiet HEAD -- || i="+"
>  					else

Thanks for resubmitting this as an inline patch for review (although
it would have been easier for me if the commit message would have
described the problem you tried to fix a bit more in detail ;-).

After testing this patch it looks like it has a few issues:

1) it will break any per-submodule configuration done via
   the 'submodule.<name>.ignore' setting in .git/config or
   .gitmodules, as using the --ignore-submodules option
   overrides those while only setting 'diff.ignoreSubmodules'
   should not do that.

2) If diff.ignoreSubmodules is unset it leads to an error
   every time the prompt is displayed:
   'fatal: bad --ignore-submodules argument:'

3) And for me it didn't change the behavior at all:

   - The '*' in the prompt vanishes as I set diff.ignoreSubmodules
     as expected with or without your patch.
     Am I missing something here?

   - The real problem here is that the '+' never goes away even
     when 'diff.ignoreSubmodules' is set to 'all'. This is due
     to the fact that 'diff.ignoreSubmodules' is only honored by
     "git diff", but not by "git diff-index".

So the real issue here seems to be the "git diff-index" call, which
doesn't honor the 'diff.ignoreSubmodules' setting. In commit 37aea37
Dscho (CCed) introduced this configuration setting while explicitly
stating that it only affects porcelain. As the other config options
always influence porcelain and plumbing, it looks like we would want
to have this option honored by plumbing too, no?

So are there any reasons for the plumbing diff commands not to honor
the diff.ignoreSubmodules setting?