Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Johannes Sixt writes: >> These days, we do not add random subcommands willy-nilly (I still >> doubt if check-ignore needs to be a separate debugging command, or a >> new mode of operation of ls-files or something), so the approach to >> use a blacklist makes more sense. "help -a" is designed to show >> whatever the users throw in their ~/bin (assuming that is on $PATH) >> under git-whatever name, and we _do_ want to complete "git wh" >> to that custom command, so a whitelist-based solution is unwieldy to >> construct. > > We already have 'git check-attr', but it is obviously not among the > autocompleted commands, otherwise the above test would not have passed. > IMO, 'git check-ignore' falls into the same category as 'git check-attr' > with regard to completion. Exactly. Actually I think what happened was that the submitter didn't have change to contrib/completion/ in earlier private versions, saw the test fail and updated t9902 without thinking. The patch posted has addition to contrib/completion/ that blacklists check-ignore the same way as check-attr, which made the change to t9902 unnecessary but because the update was done without thinking, it wasn't even realized that the test would have passed without the patch to it. Reverting the part that touches t9902 should still pass, I would think. t/t9902-completion.sh | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git c/t/t9902-completion.sh w/t/t9902-completion.sh index cce51ac..92d7eb4 100755 --- c/t/t9902-completion.sh +++ w/t/t9902-completion.sh @@ -213,19 +213,19 @@ test_expect_success 'general options' ' ' test_expect_success 'general options plus command' ' - test_completion "git --version checko" "checkout " && - test_completion "git --paginate checko" "checkout " && - test_completion "git --git-dir=foo checko" "checkout " && - test_completion "git --bare checko" "checkout " && + test_completion "git --version check" "checkout " && + test_completion "git --paginate check" "checkout " && + test_completion "git --git-dir=foo check" "checkout " && + test_completion "git --bare check" "checkout " && test_completion "git --help des" "describe " && - test_completion "git --exec-path=foo checko" "checkout " && - test_completion "git --html-path checko" "checkout " && - test_completion "git --no-pager checko" "checkout " && - test_completion "git --work-tree=foo checko" "checkout " && - test_completion "git --namespace=foo checko" "checkout " && - test_completion "git --paginate checko" "checkout " && - test_completion "git --info-path checko" "checkout " && - test_completion "git --no-replace-objects checko" "checkout " + test_completion "git --exec-path=foo check" "checkout " && + test_completion "git --html-path check" "checkout " && + test_completion "git --no-pager check" "checkout " && + test_completion "git --work-tree=foo check" "checkout " && + test_completion "git --namespace=foo check" "checkout " && + test_completion "git --paginate check" "checkout " && + test_completion "git --info-path check" "checkout " && + test_completion "git --no-replace-objects check" "checkout " ' test_done -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Am 9/26/2012 1:25, schrieb Junio C Hamano: > Johannes Sixt writes: > >> Am 9/20/2012 21:46, schrieb Adam Spiers: >>> test_expect_success 'general options plus command' ' >>> - test_completion "git --version check" "checkout " && >>> - test_completion "git --paginate check" "checkout " && >>> - test_completion "git --git-dir=foo check" "checkout " && >>> - test_completion "git --bare check" "checkout " && >>> + test_completion "git --version checko" "checkout " && >>> + test_completion "git --paginate checko" "checkout " && >>> + test_completion "git --git-dir=foo checko" "checkout " && >>> + test_completion "git --bare checko" "checkout " && >>> ... >> >> I find this worrysome. Is check-ignore, being a debugging aid, so >> important that it must be autocompleted? > > The shell function __git_list_porcelain_commands in contrib/completion/ > starts from "git help -a" and filters plumbing commands and helpers > via a blacklist. At least, check-ignore needs to be added there. > > These days, we do not add random subcommands willy-nilly (I still > doubt if check-ignore needs to be a separate debugging command, or a > new mode of operation of ls-files or something), so the approach to > use a blacklist makes more sense. "help -a" is designed to show > whatever the users throw in their ~/bin (assuming that is on $PATH) > under git-whatever name, and we _do_ want to complete "git wh" > to that custom command, so a whitelist-based solution is unwieldy to > construct. We already have 'git check-attr', but it is obviously not among the autocompleted commands, otherwise the above test would not have passed. IMO, 'git check-ignore' falls into the same category as 'git check-attr' with regard to completion. -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Johannes Sixt writes: > Am 9/20/2012 21:46, schrieb Adam Spiers: >> test_expect_success 'general options plus command' ' >> -test_completion "git --version check" "checkout " && >> -test_completion "git --paginate check" "checkout " && >> -test_completion "git --git-dir=foo check" "checkout " && >> -test_completion "git --bare check" "checkout " && >> +test_completion "git --version checko" "checkout " && >> +test_completion "git --paginate checko" "checkout " && >> +test_completion "git --git-dir=foo checko" "checkout " && >> +test_completion "git --bare checko" "checkout " && >> ... > > I find this worrysome. Is check-ignore, being a debugging aid, so > important that it must be autocompleted? The shell function __git_list_porcelain_commands in contrib/completion/ starts from "git help -a" and filters plumbing commands and helpers via a blacklist. At least, check-ignore needs to be added there. These days, we do not add random subcommands willy-nilly (I still doubt if check-ignore needs to be a separate debugging command, or a new mode of operation of ls-files or something), so the approach to use a blacklist makes more sense. "help -a" is designed to show whatever the users throw in their ~/bin (assuming that is on $PATH) under git-whatever name, and we _do_ want to complete "git wh" to that custom command, so a whitelist-based solution is unwieldy to construct. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Adam Spiers writes:
> +expect_in () {
> + dest="$HOME/expected-$1" text="$2"
> + if test -z "$text"
> + then
> + >"$dest" # avoid newline
> + else
> + echo -e "$text" >"$dest"
This breaks when your shell is not "bash".
> +test_check_ignore () {
> + args="$1" expect_code="${2:-0}" global_args="$3"
> +
> + init_vars &&
> + rm -f "$HOME/stdout" "$HOME/stderr" "$HOME/cmd" &&
> + echo $(which git) $global_args check-ignore $quiet_opt $verbose_opt
> $args \
> + >"$HOME/cmd" &&
Don't use which in scripts. Is this just leftover cruft from debugging?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Michael Haggerty writes: >> +For each pathname given via the command-line or from a file via >> +`--stdin`, this command will list the first exclude pattern found (if >> +any) which explicitly excludes or includes that pathname. Note that >> +within any given exclude file, later patterns take precedence over >> +earlier ones, so any matching pattern which this command outputs may >> +not be the one you would immediately expect. > > Can I tell from the output of "git check-ignore" whether a file is > really ignored? The way I read the paragraph above, the output doesn't > necessarily show the pattern that determines whether a file is *really* > ignored. That makes it sound like the ignore status of the file might > be different than what I would infer from the output. If I am > misunderstanding the situation, then perhaps the explanation in the > above paragraph can be improved. > > On the other hand, if my understanding is correct, then why did you > choose this (seemingly strange) policy? It would seem more useful > either to output the pattern that has the definitive effect on the > file's status, or to output all patterns that match the file. Very good point. I didn't look at this patch at all, but I would guess the patch hooked this into the order in which the existing ignore mechanism computes the match, and "the first I find" Adam says is written from implementation point of view. And that is a wrong way to go about it. The existing document for the ignore mechanism explains things from the end user perspective, i.e. within a file, the last matching pattern determines the fate of the path. And this debugging aid should report which pattern from what source determined the fate of the path, so from the end user perspective, saying "first" is nonsense. It just happens that the implementation optimizes by scanning the list of patterns "backwards" and stops at the "first" hit, which is the last matching pattern that determines the fate of the path. The documentation of this debugging aid should use the same phrasing "git help ignore" uses and say "show the pattern that decides the outcome", and lose that confusing "Note that" that is not helping anybody. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
On 09/20/2012 09:46 PM, Adam Spiers wrote: > This works in a similar manner to git-check-attr. Some code > was reused from add.c by refactoring out into pathspec.c. > > Thanks to Jeff King and Junio C Hamano for the idea: > http://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815 > > Signed-off-by: Adam Spiers > --- > .gitignore | 1 + > Documentation/git-check-ignore.txt | 85 + > Documentation/gitignore.txt| 6 +- > Makefile | 1 + > builtin.h | 1 + > builtin/check-ignore.c | 167 ++ > command-list.txt | 1 + > contrib/completion/git-completion.bash | 1 + > git.c | 1 + > t/t0007-ignores.sh | 587 > + > t/t9902-completion.sh | 24 +- > 11 files changed, 861 insertions(+), 14 deletions(-) > create mode 100644 Documentation/git-check-ignore.txt > create mode 100644 builtin/check-ignore.c > create mode 100755 t/t0007-ignores.sh > > diff --git a/.gitignore b/.gitignore > index a188a82..11cd975 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -22,6 +22,7 @@ > /git-bundle > /git-cat-file > /git-check-attr > +/git-check-ignore > /git-check-ref-format > /git-checkout > /git-checkout-index > diff --git a/Documentation/git-check-ignore.txt > b/Documentation/git-check-ignore.txt > new file mode 100644 > index 000..2de4e17 > --- /dev/null > +++ b/Documentation/git-check-ignore.txt > @@ -0,0 +1,85 @@ > +git-check-ignore(1) > += > + > +NAME > + > +git-check-ignore - Debug gitignore / exclude files > + > + > +SYNOPSIS > + > +[verse] > +'git check-ignore' [options] pathname... > +'git check-ignore' [options] --stdin < > + > +DESCRIPTION > +--- > + > +For each pathname given via the command-line or from a file via > +`--stdin`, this command will list the first exclude pattern found (if > +any) which explicitly excludes or includes that pathname. Note that > +within any given exclude file, later patterns take precedence over > +earlier ones, so any matching pattern which this command outputs may > +not be the one you would immediately expect. Can I tell from the output of "git check-ignore" whether a file is really ignored? The way I read the paragraph above, the output doesn't necessarily show the pattern that determines whether a file is *really* ignored. That makes it sound like the ignore status of the file might be different than what I would infer from the output. If I am misunderstanding the situation, then perhaps the explanation in the above paragraph can be improved. On the other hand, if my understanding is correct, then why did you choose this (seemingly strange) policy? It would seem more useful either to output the pattern that has the definitive effect on the file's status, or to output all patterns that match the file. > +OPTIONS > +--- > +-q, --quiet:: > + Don't output anything, just set exit status. This is only > + valid with a single pathname. > + > +-v, --verbose:: > + Also output details about the matching pattern (if any) > + for each given pathname. > + > +--stdin:: > + Read file names from stdin instead of from the command-line. > + > +-z:: > + The output format is modified to be machine-parseable (see > + below). If `--stdin` is also given, input paths are separated > + with a NUL character instead of a linefeed character. > + > +OUTPUT > +-- > + > +By default, any of the given pathnames which match an ignore pattern > +will be output, one per line. If no pattern matches a given path, > +nothing will be output for that path; this means that path will not be > +ignored. > + > +If `--verbose` is specified, the output is a series of lines of the form: > + > + > + > + is the path of a file being queried, is the > +matching pattern, is the pattern's source file, and > +is the line number of the pattern within that source. If the pattern > +contained a `!` prefix or `/` suffix, it will be preserved in the > +output. will be an absolute path when referring to the file > +configured by `core.excludesfile`, or relative to the repository root > +when referring to `.git/info/exclude` or a per-directory exclude file. > + > +If `-z` is specified, the output is a series of lines of the form: > + > +EXIT STATUS > +--- > [...] I think you forgot to finish the thought about "If -z is specified". Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] Add git-check-ignore sub-command
Am 9/20/2012 21:46, schrieb Adam Spiers: > test_expect_success 'general options plus command' ' > - test_completion "git --version check" "checkout " && > - test_completion "git --paginate check" "checkout " && > - test_completion "git --git-dir=foo check" "checkout " && > - test_completion "git --bare check" "checkout " && > + test_completion "git --version checko" "checkout " && > + test_completion "git --paginate checko" "checkout " && > + test_completion "git --git-dir=foo checko" "checkout " && > + test_completion "git --bare checko" "checkout " && > ... I find this worrysome. Is check-ignore, being a debugging aid, so important that it must be autocompleted? -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
