Re: [PATCH 9/9] Add git-check-ignores

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 2, 2012, 20:38 UTC
Message-ID: <7v1uikgnnp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <CAOkDyE9wPUOwJpeKQ5wSCoufqyqE9zwRuBuNvDGEZ-z8452DwA@mail.gmail.com>

Adam Spiers <git@adamspiers.org> writes:

Show 8 quoted lines

>>> +SYNOPSIS
>>> +--------
>>> +[verse]
>>> +'git check-ignore' pathname...
>>> +'git check-ignore' --stdin [-z] < <list-of-paths>
>>
>> Also --quiet option, where check-ignore returns 0 if the given path is
>> ignored, 1 otherwise?

I agree that multiple paths are problematic.

We could error out if multiple paths are given with --quiet until we figure out what the useful result would be in such a case, and still give a useful answer to callers that feed a single path, though.

That may encourage suboptimal coding to casual Porcelain writers, i.e. it would allow

	for path in $paths
        do
		if git check-ignore -q "$path"
                then
			do something to "$path"
		fi
	done

even though we would rather want to encourage

	git check-ignore --name-only $paths |
        while read path
        do
		do something to "$path"
	done

But from lay-scriptors' point of view, being able to easily write a script (even though it may be inefficient) to do the job at hand is far better than having to give up writing one because the tool does not allow easy-and-stupid scripting, so it is not exactly a huge downside.

>>  - If many paths are given, then perhaps we could print ignored paths
>> (no extra info).
>
> How is this different to git ls-files -i -o ?

I personally think the parts of ls-files that deal with paths not in the index outlived its usefulness ;-) and users deserve to be given a better UI.

Show 5 quoted lines

>>  - Going to the next level, we could print path and the the location
>> of the final exclude/include rule (file and line number).
>
> That's the current behaviour, and I believe it covers the most common
> use case.

Yes; I have a reservation on your output format, though.

>>  - For debugging, given one path, we print all the rules that are
>> applied to it, which may help understand how/why it goes wrong.

I do not think that would be terribly useful. Maybe for people who are learning how dir.c internally works, but not for people who are trying to improve the set of .gitignore files in their project.

Show 6 quoted lines

> I thought about that, but in the end I decided it probably didn't make
> sense, because none of the exclude matching routines match against the
> index - they all match against the working tree and core.excludesfile.
> This would also require changing the matching logic to honor the index,
> but I didn't see the benefit in doing that, since all operations which
> involve excludes (add, status, etc.) relate to a work tree.

The mechanism primarily is to see if a path in the working tree is a cruft or a valuable still to be added; I am OK with NEED_WORK_TREE; when we have a useful case to run this in a bare repository, we can lift it.

As with the "what to do with multiple paths and -q", it is better to start with feature set to cover only the known or easily anticipated use cases, rejecting the cases for which good semantics are not thought out.

An alternative would be a code that operates sanely only for known or anticipated cases and do random things with irrational semantics in other cases, and people start relying on the irrational behaviour without realizing their input and the behaviour they are seeing are not something that the feature is designed to for, but whatever the code with loose precondition checking happens to do. We do not want to repeat that kind of mistake, which is hard to fix in future versions.