sean [off-list ref] writes:
So your new patch should also fix that comment to remove the
"(or flags)" portion.
Probably.
Since you comment on the -- marker here I think it should
also appear in the command line above:
'git-grep' [<option>...] [-e] <pattern> [--] [<path>...]
I've thought about this but it is not any more correct than what
we have now (both are technically incorrect). If you do not use
an `-e` and let a non-option terminate the option processing,
double dashes are not removed, so you do not want it there.
Instead it is more useful for them to be told _specificly_ which
git-ls-files options are available and that all others will be
passed to grep. Somthing like:
I like it.
... Your patch fixes the problem case and
there is no reason now to warn the user away from supplying the --
marker in addition to the "-e"; it'll work properly in either case.
Does it? I think if you give -- without -e it will look for a
path that matches -- because we pass our own -- to ls-files.
That's it and the rest looked good. In case you agree with anything
i've said here, find an amended version of your patch below.
Thanks.
When people make an improvement proposal, I'd often prefer to
see a patch that is on top of the patch being discussed, not a
replacement.
On Sat, 21 Jan 2006 11:06:27 -0800
Junio C Hamano [off-list ref] wrote:
I've thought about this but it is not any more correct than what
we have now (both are technically incorrect). If you do not use
an `-e` and let a non-option terminate the option processing,
double dashes are not removed, so you do not want it there.
I think this should be fixed rather than requiring the user
to remember such an obscure detail. It's easy to fix git-grep
to deal with it instead (see below).
Does it? I think if you give -- without -e it will look for a
path that matches -- because we pass our own -- to ls-files.
You're right, in the patch below I added a specific test to handle
this case so the documentation can be simplified and the user is
free to use any combination of -e and --.
When people make an improvement proposal, I'd often prefer to
see a patch that is on top of the patch being discussed, not a
replacement.
Yes, I can see how that would be easier to review. Below
is a patch on top of your original that now includes the tweak
mentioned above:
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 55d3bed..7fd675b 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -8,7 +8,7 @@ git-grep - print lines matching a patter
SYNOPSIS
--------
-'git-grep' [<option>...] [-e] <pattern> [<path>...]
+'git-grep' [<option>...] [-e] <pattern> [--] [<path>...]
DESCRIPTION
-----------
@@ -24,26 +24,18 @@ OPTIONS
<option>...::
Either an option to pass to `grep` or `git-ls-files`.
- Some `grep` options, such as `-C` and `-m`, that take
- parameters are known to `git-grep`. Among options
- applicable to git-ls-files`, `--others` and
- `--exclude=*` (and other variants of exclusion) may be
- of interest. Only `-o` is recognized as an option to
- `git-ls-files` in the short form (e.g. `-d` and `-m` are
- given to `grep`, not to `git-ls-files` as synonym
- for `--deleted` and `--modifed`), so you need to spell
- out `git-ls-files` options in longer form
- e.g. `--deleted`.
+
+ The following are the specific `git-ls-files` options
+ that may be given: `-o`, `--cached`, `--deleted`, `--others`,
+ `--killed`, `--ignored`, `--modified`, `--exclude=*`,
+ `--exclude-from=*`, and `--exclude-per-directory=*`.
+
+ All other options will be passed to `grep`.
<pattern>::
The pattern to look for. The first non option is taken
as the pattern; if your pattern begins with a dash, use
- `-e <pattern>`. When a pattern is found without `-e`, it
- also terminates the option processing and the rest of
- the parameters are used as the `<path>...`, and you do
- not specifically add `--` to protect the path limiter
- that happens to begin with a dash from being mistaken as
- an option.
+ `-e <pattern>`.
<path>...::
Optional paths to limit the set of files to be searched;
diff --git a/git-grep.sh b/git-grep.sh
index 23b1e03..4f06093 100755
--- a/git-grep.sh
+++ b/git-grep.sh
@@ -36,7 +36,7 @@ while : ; do
shift
;;
--)
- # The rest are git-ls-files paths (or flags)
+ # The rest are git-ls-files paths
shift
break
;;
@@ -49,6 +49,7 @@ while : ; do
got_pattern "$1"
shift
fi
+ [ "$1" = -- ] && shift
break
;;
esac