public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Dave Williams <dave@opensourcesolutions•co.uk>
Cc: Git List <git@vger•kernel.org>, Adam Spiers <git@adamspiers•org>,
	Duy Nguyen <pclouds@gmail•com>,
	Eric Sunshine <sunshine@sunshineco•com>
Subject: Re: [PATCH V3] check-ignore: Add option to ignore index contents
Date: Thu, 05 Sep 2013 15:27:36 -0700	[thread overview]
Message-ID: <xmqqbo46hpk7.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: 20130905160801.GA22710@opensourcesolutions.co.uk

Dave Williams <dave@opensourcesolutions•co.uk> writes:

> check-ignore currently shows how .gitignore rules would treat untracked
> paths. Tracked paths do not generate useful output.  This prevents
> debugging of why a path became tracked unexpectedly unless that path is
> first removed from the index with `git rm --cached <path>`.
>
> This option (-i, --no-index) simply by-passes the check for the path
> being in the index and hence allows tracked paths to be checked too.

Now the long option name is "--no-index", it makes me wonder if "-i"
is a good synonym for it, and the longer I stare at it, the more
certain I become convinced that it is a bad choice.

Do we even need a short-and-sweet one-letter option for this?  I'd
prefer starting with only the long option.

I came up with a squashable tweak to remove "-i" on top of this
patch; will tentatively queue this patch with it to 'pu'.

> In particlar Junio queried my approach in
> builtin/git-check-ignores.c that bypassed the functions that check
> a path is in the index as well as avoiding reading the index in
> the first place.

Thanks.

I think this version is cleaner without those "if (!no_index)" used
for special casing in the codeflow.  An empty index is a valid
state, in which the in-core index starts when any git program
begins.  Not reading the index should be the only thing necessary to
mimick the state in which nothing has been added to the index, and
if that is not the case, we have found a bug in the existing code.

> Regarding the test script I have tidied up the variables containing the
> separate option switches so they dont contain leading spaces, instead I
> have added spaces as needed when formatting the command line.  This
> not only improves my patch but also the existing code which was a little
> inconsistent in this respect.

Yeah, that's very much appreciated.

> Finally I have rebased from the latest commmit on master to pick up
> unrelated recent changes made to builtin/check-ignores.c and updated my
> code to be consistent with this.
>
> Hopefully I have put these patch notes in the right place now! Let me
> know if not.
>
> Dave

Nicely done.

next prev parent reply	other threads:[~2013-09-05 22:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-05 16:08 [PATCH V3] check-ignore: Add option to ignore index contents Dave Williams
2013-09-05 22:27 ` Junio C Hamano [this message]
2013-09-06  5:59   ` Dave Williams
2013-09-06 16:30     ` Junio C Hamano

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=xmqqbo46hpk7.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=dave@opensourcesolutions$(echo .)co.uk \
    --cc=git@adamspiers$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox