On Wed, Jul 20, 2011 at 22:57, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Taken in total isolation, this patch does allow a use case where we did
> not allow, but when considered in a larger picture of what "grep" is used
> for and how different use cases the command should support, a few random
> thoughts come to mind:
>
> - Like "diff --no-index", "grep --no-index" is about a directory that is
> not managed by git at all with random collection of files. Do we even
> want to be using any "exclude" in such a use case? Wouldn't it actually
> be a bug that we pay attention to standard-exlcludes in the current
> code, as .gitignore files scattered in such a directory should _not_
> mean anything, as it is not a git working tree at all?
>
> - Even in a git managed directory, you _could_ use "grep --no-index" to
> find hits from both tracked and untracked files. In this particular use
> case, it makes some sense to pay attention to "exclude", as that would
> catch what _could_ be committed, and paths that would be excluded won't
> be part of that set (unless you use "add -f"). But wouldn't that use
> case better be covered by a switch that is different from --no-index
> (which means "These are not managed by git at all")? It is still about
> files in a git working tree, it is just that the user wants us to pay
> attention also to untracked files, e.g. "grep --untracked-too"?
>
> So I think the patch identified a good problem to solve, but it might be a
> wrong solution that encourages a use of a wrong option (i.e. --no-index)
> only because we do not have the right one (i.e. "I am in the working tree,
> but I want untracked ones also considered.").
>
> What do people think if we did this a bit differently?
>
> - Since 3081623 (grep --no-index: allow use of "git grep" outside a git
> repository, 2010-01-15) and 59332d1 (Resurrect "git grep --no-index",
> 2010-02-06), "grep --no-index" incorrectly paid attention to the
> exclude patterns. We shouldn't have, and we'd fix that bug.
I considered this when I noticed this, but feared the
backward-incompatibility. If you call it a bug, than we can change for
the better.
>
> - It might be useful to be able to "git grep" both tracked and untracked
> (i.e. new files you may want to "git add") paths, but there is no good
> way to do so. Introduce a new option --untracked-too (or more suitable
> name --- I am bad at naming and not married to this one) to allow
> this. This mode always takes "exclude" into account.
My proposal would be to name it --include-untracked.
>
> Opinions?
>
> Regarding the patch:
>
>> + /* --no-exclude-standard needs --no-index */
>> + if (use_index && !exclude_standard)
>> + die(_("--no-exclude-standard does not make sense without --no-index."));
>
> For that matter,
>
> $ git grep --no-exclude-standard --exclude-standard --cached -e foo
>
> should be an error, no?
>
It should be. But I think that unveils one of the shortcomings of the
(any) option parser: You wont get notified when an option was given,
regardless of its value. To handle the above I would have to use
OPTION_CALLBACK to set an addition flag exc_given (like it is done in
git-ls-files) and test against this. The same problem is possible for
a number option, if you want to know whether the option was actually
given on the command line, one need to invent an invalid value (which
isn't always possible) and use this as the initializer or use
OPTION_CALLBACK again.
Bert
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html