* [RFC 0/2] grep: make output consistent with revision syntax
@ 2017-01-19 15:03 Stefan Hajnoczi
2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw)
To: git; +Cc: gitster, Stefan Hajnoczi
git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax.
This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1)
and expect "git show rev:path/to/file.c" to work. See the individual patches
for examples of command-lines that produce invalid output.
This series is an incomplete attempt at solving the issue. I'm not familiar
enough with the git codebase to propose a better solution. Perhaps someone is
interested in a proper fix?
Stefan Hajnoczi (2):
grep: only add delimiter if there isn't one already
grep: use '/' delimiter for paths
builtin/grep.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
--
2.9.3
^ permalink raw reply [flat|nested] 21+ messages in thread* [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-19 15:03 [RFC 0/2] grep: make output consistent with revision syntax Stefan Hajnoczi @ 2017-01-19 15:03 ` Stefan Hajnoczi 2017-01-19 18:39 ` Junio C Hamano 2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi 2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King 2 siblings, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw) To: git; +Cc: gitster, Stefan Hajnoczi git-grep(1) output does not follow git's own syntax: $ git grep malloc v2.9.3: v2.9.3::Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc $ git show v2.9.3::Makefile fatal: Path ':Makefile' does not exist in 'v2.9.3' This patch avoids emitting the unnecessary ':' delimiter if the name already ends with ':' or '/': $ git grep malloc v2.9.3: v2.9.3:Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc $ git show v2.9.3:Makefile (succeeds) Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- builtin/grep.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index 462e607..3643d8a 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -490,7 +490,11 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, strbuf_init(&base, PATH_MAX + len + 1); if (len) { strbuf_add(&base, name, len); - strbuf_addch(&base, ':'); + + /* Add a delimiter if there isn't one already */ + if (name[len - 1] != '/' && name[len - 1] != ':') { + strbuf_addch(&base, ':'); + } } init_tree_desc(&tree, data, size); hit = grep_tree(opt, pathspec, &tree, &base, base.len, -- 2.9.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi @ 2017-01-19 18:39 ` Junio C Hamano 2017-01-20 13:56 ` Stefan Hajnoczi 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2017-01-19 18:39 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git Stefan Hajnoczi <stefanha@redhat.com> writes: > git-grep(1) output does not follow git's own syntax: > > $ git grep malloc v2.9.3: > v2.9.3::Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc > $ git show v2.9.3::Makefile > fatal: Path ':Makefile' does not exist in 'v2.9.3' > > This patch avoids emitting the unnecessary ':' delimiter if the name > already ends with ':' or '/': > > $ git grep malloc v2.9.3: > v2.9.3:Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc > $ git show v2.9.3:Makefile > (succeeds) I am mildly negative on this one. I suspect that the above example is a made-up one and you might have a more compelling real-world use case in mind, but at least the above does not convince me. The end-user explicitly gave the extra ':' because she wanted to feed the tree object, not a tag that leads to the tree, for some reason. I think the output should respect that and show how she spelled the starting point. IOW, it is not "':' added unnecessarily by Git". It is ':' added by the end-user for whatever reason. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > builtin/grep.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/builtin/grep.c b/builtin/grep.c > index 462e607..3643d8a 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -490,7 +490,11 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, > strbuf_init(&base, PATH_MAX + len + 1); > if (len) { > strbuf_add(&base, name, len); > - strbuf_addch(&base, ':'); > + > + /* Add a delimiter if there isn't one already */ > + if (name[len - 1] != '/' && name[len - 1] != ':') { > + strbuf_addch(&base, ':'); > + } > } > init_tree_desc(&tree, data, size); > hit = grep_tree(opt, pathspec, &tree, &base, base.len, ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-19 18:39 ` Junio C Hamano @ 2017-01-20 13:56 ` Stefan Hajnoczi 2017-01-20 18:16 ` Junio C Hamano 0 siblings, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-20 13:56 UTC (permalink / raw) To: Junio C Hamano; +Cc: git [-- Attachment #1: Type: text/plain, Size: 1751 bytes --] On Thu, Jan 19, 2017 at 10:39:02AM -0800, Junio C Hamano wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > git-grep(1) output does not follow git's own syntax: > > > > $ git grep malloc v2.9.3: > > v2.9.3::Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc > > $ git show v2.9.3::Makefile > > fatal: Path ':Makefile' does not exist in 'v2.9.3' > > > > This patch avoids emitting the unnecessary ':' delimiter if the name > > already ends with ':' or '/': > > > > $ git grep malloc v2.9.3: > > v2.9.3:Makefile: COMPAT_CFLAGS += -Icompat/nedmalloc > > $ git show v2.9.3:Makefile > > (succeeds) > > I am mildly negative on this one. I suspect that the above example > is a made-up one and you might have a more compelling real-world use > case in mind, but at least the above does not convince me. Another trailing delimiter example: $ git grep malloc v2.9.3:t/ v2.9.3:t/:test-lib.sh: setup_malloc_check () { After Patch 1/2: v2.9.3:t/test-lib.sh: setup_malloc_check () { > The end-user explicitly gave the extra ':' because she wanted to > feed the tree object, not a tag that leads to the tree, for some > reason. I think the output should respect that and show how she > spelled the starting point. IOW, it is not "':' added unnecessarily > by Git". It is ':' added by the end-user for whatever reason. v2.9.3::Makefile may convey that the user originally provided v2.9.3: but is that actually useful information? I don't see what the user will do with this information (and they should already know since they provided the command-line). v2.9.3:Makefile can be copy-pasted or extracted by scripts for further git commands. That is useful. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-20 13:56 ` Stefan Hajnoczi @ 2017-01-20 18:16 ` Junio C Hamano 2017-01-23 13:15 ` Stefan Hajnoczi 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2017-01-20 18:16 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git Stefan Hajnoczi <stefanha@redhat.com> writes: > v2.9.3::Makefile may convey that the user originally provided v2.9.3: > but is that actually useful information? You are either asking a wrong question, or asking a wrong person (i.e. Git) the question. The real question is why the user added a colon at the end, when "v2.9.3" alone would have sufficed. What did the user want to get out of giving an extra colon like "v2.9.3:"? One answer I can think of is that it is a degenerate case of [2/2], i.e. if "v2.9.3:t" were an appropriate way to look in the subtree "t" of "v2.9.3", "v2.9.3:" would be the appropriate way to look in the whole tree of "v2.9.3". I understand, from your response to my comment in the thread for [2/2], that the reason why "v2.9.3:t" was used in the example was because you felt uncomfortable with using pathspec. That's superstition. My only piece of advice to folks who feel that way is to learn Git more and get comfortable. You can do neat things like $ git grep -e pattern rev -- t ':!t/helper/' that you cannot do with "rev:t", for example ;-) All examples we saw so far are the ones that the user used the colon syntax when it is more natural to express the command without it. I am hesitant to see the behaviour of the command changed to appease such suboptimal use cases if it requires us to discard a bit of information, when we haven't established it is OK to lose. There may be a case (1) where the colon syntax is the most natural thing to use, AND script reading the output that used that syntax is forced to do unnecessary work because "git grep" parrots the colon literally, instread of being more intelligent about it (i.e. omitting or substituting with slash when the input is a tree object, not a tree-ish, as discussed in the thread). (2) where the colon syntax is the most natural thing, AND script reading the output WANTS to see the distinction in the input (remember, "git grep" can take more than one input tree). We haven't seen either of the above two in the discussion, so the discussion so far is not sufficient to support the change being proposed in this RFC, which requires that it is safe to assume that (1) is the only case where the input is a raw tree (or the input contains a colon) and (2) will never happen. So I am still mildly negative on the whole thing. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-20 18:16 ` Junio C Hamano @ 2017-01-23 13:15 ` Stefan Hajnoczi 2017-01-24 19:07 ` Jakub Narębski 0 siblings, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-23 13:15 UTC (permalink / raw) To: Junio C Hamano; +Cc: git [-- Attachment #1: Type: text/plain, Size: 2866 bytes --] On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > v2.9.3::Makefile may convey that the user originally provided v2.9.3: > > but is that actually useful information? > > You are either asking a wrong question, or asking a wrong person > (i.e. Git) the question. The real question is why the user added a > colon at the end, when "v2.9.3" alone would have sufficed. What did > the user want to get out of giving an extra colon like "v2.9.3:"? > > One answer I can think of is that it is a degenerate case of [2/2], > i.e. if "v2.9.3:t" were an appropriate way to look in the subtree > "t" of "v2.9.3", "v2.9.3:" would be the appropriate way to look in > the whole tree of "v2.9.3". > > I understand, from your response to my comment in the thread for > [2/2], that the reason why "v2.9.3:t" was used in the example was > because you felt uncomfortable with using pathspec. > > That's superstition. > > My only piece of advice to folks who feel that way is to learn Git > more and get comfortable. You can do neat things like > > $ git grep -e pattern rev -- t ':!t/helper/' > > that you cannot do with "rev:t", for example ;-) Neat, thanks for showing the path exclusion syntax. I wasn't aware of it. > All examples we saw so far are the ones that the user used the colon > syntax when it is more natural to express the command without it. I > am hesitant to see the behaviour of the command changed to appease > such suboptimal use cases if it requires us to discard a bit of > information, when we haven't established it is OK to lose. > > There may be a case > > (1) where the colon syntax is the most natural thing to use, AND > script reading the output that used that syntax is forced to do > unnecessary work because "git grep" parrots the colon > literally, instread of being more intelligent about it > (i.e. omitting or substituting with slash when the input is a > tree object, not a tree-ish, as discussed in the thread). > > (2) where the colon syntax is the most natural thing, AND script > reading the output WANTS to see the distinction in the input > (remember, "git grep" can take more than one input tree). > > We haven't seen either of the above two in the discussion, so the > discussion so far is not sufficient to support the change being > proposed in this RFC, which requires that it is safe to assume that > (1) is the only case where the input is a raw tree (or the input > contains a colon) and (2) will never happen. > > So I am still mildly negative on the whole thing. Avoiding the colon syntax avoids the whole issue for my use case. I still think git-grep(1)'s output would be more useful if it used valid git rev:path syntax in all cases. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-23 13:15 ` Stefan Hajnoczi @ 2017-01-24 19:07 ` Jakub Narębski 2017-01-24 20:48 ` Philip Oakley 0 siblings, 1 reply; 21+ messages in thread From: Jakub Narębski @ 2017-01-24 19:07 UTC (permalink / raw) To: Stefan Hajnoczi, Junio C Hamano; +Cc: git W dniu 23.01.2017 o 14:15, Stefan Hajnoczi pisze: > On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote: >> My only piece of advice to folks who feel that way is to learn Git >> more and get comfortable. You can do neat things like >> >> $ git grep -e pattern rev -- t ':!t/helper/' >> >> that you cannot do with "rev:t", for example ;-) > > Neat, thanks for showing the path exclusion syntax. I wasn't aware of > it. That reminds me of mu TODO item: moving extended pathspec information from gitglossary(7) manpage (sic!) to to-be-created gitpathspec(7). -- Jakub Narębski ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 1/2] grep: only add delimiter if there isn't one already 2017-01-24 19:07 ` Jakub Narębski @ 2017-01-24 20:48 ` Philip Oakley 0 siblings, 0 replies; 21+ messages in thread From: Philip Oakley @ 2017-01-24 20:48 UTC (permalink / raw) To: Stefan Hajnoczi, Junio C Hamano, Jakub Narębski; +Cc: git [-- Attachment #1: Type: text/plain, Size: 836 bytes --] From: "Jakub Narębski" <jnareb@gmail.com> >W dniu 23.01.2017 o 14:15, Stefan Hajnoczi pisze: >> On Fri, Jan 20, 2017 at 10:16:31AM -0800, Junio C Hamano wrote: > >>> My only piece of advice to folks who feel that way is to learn Git >>> more and get comfortable. You can do neat things like >>> >>> $ git grep -e pattern rev -- t ':!t/helper/' >>> >>> that you cannot do with "rev:t", for example ;-) >> >> Neat, thanks for showing the path exclusion syntax. I wasn't aware of >> it. > > That reminds me of mu TODO item: moving extended pathspec information > from gitglossary(7) manpage (sic!) to to-be-created gitpathspec(7). > Good to see someone else also had it on a ToDo list.. Attached is my collation of all the different path spec info I found from trawling the man & guide pages to satisfy my ignorance... -- Philip [-- Attachment #2: gitpathspec.txt --] [-- Type: text/plain, Size: 12282 bytes --] gitpathspec(7) ============ NAME ---- gitpathspec - How to specify a path or file to git SYNOPSIS -------- $HOME/.config/git/ignore, $GIT_DIR/info/exclude, .gitignore DESCRIPTION ----------- Pathspecs are used in a range of git functions. .gitignore .gitexclude gitsparse git-add -- pathspec git-checkout -- pathspec (after the double-dash) git grep (active/non-active wild card matching) git log (L#:<> pathspec limiters ?? what does it mean) git rerere (uncontentious) git status (uncontentious) gitk (uncontentious, but see 'log' above) 'git' itself -- --literal-pathspecs Treat pathspecs literally (i.e. no globbing, no pathspec magic). This is equivalent to setting the GIT_LITERAL_PATHSPECS environment variable to 1. --glob-pathspecs Add "glob" magic to all pathspec. This is equivalent to setting the GIT_GLOB_PATHSPECS environment variable to 1. Disabling globbing on individual pathspecs can be done using pathspec magic ":(literal)" --noglob-pathspecs Add "literal" magic to all pathspec. This is equivalent to setting the GIT_NOGLOB_PATHSPECS environment variable to 1. Enabling globbing on individual pathspecs can be done using pathspec magic ":(glob)" --icase-pathspecs Add "icase" magic to all pathspec. This is equivalent to setting the GIT_ICASE_PATHSPECS environment variable to 1. see glossary-content pathspec Pattern used to limit paths in Git commands. Pathspecs are used on the command line of "git ls-files", "git ls-tree", "git add", "git grep", "git diff", "git checkout", and many other commands to limit the scope of operations to some subset of the tree or worktree. See the documentation of each command for whether paths are relative to the current directory or toplevel. The pathspec syntax is as follows: any path matches itself the pathspec up to the last slash represents a directory prefix. The scope of that pathspec is limited to that subtree. the rest of the pathspec is a pattern for the remainder of the pathname. Paths relative to the directory prefix will be matched against that pattern using fnmatch(3); in particular, * and ? can match directory separators. For example, Documentation/*.jpg will match all .jpg files in the Documentation subtree, including Documentation/chapter_1/figure_1.jpg. A pathspec that begins with a colon : has special meaning. In the short form, the leading colon : is followed by zero or more "magic signature" letters (which optionally is terminated by another colon :), and the remainder is the pattern to match against the path. The "magic signature" consists of ASCII symbols that are neither alphanumeric, glob, regex special charaters nor colon. The optional colon that terminates the "magic signature" can be omitted if the pattern begins with a character that does not belong to "magic signature" symbol set and is not a colon. In the long form, the leading colon : is followed by a open parenthesis (, a comma-separated list of zero or more "magic words", and a close parentheses ), and the remainder is the pattern to match against the path. A pathspec with only a colon means "there is no pathspec". This form should not be combined with other pathspec. top The magic word top (magic signature: /) makes the pattern match from the root of the working tree, even when you are running the command from inside a subdirectory. literal Wildcards in the pattern such as * or ? are treated as literal characters. icase Case insensitive match. glob Git treats the pattern as a shell glob suitable for consumption by fnmatch(3) with the FNM_PATHNAME flag: wildcards in the pattern will not match a / in the pathname. For example, "Documentation/*.html" matches "Documentation/git.html" but not "Documentation/ppc/ppc.html" or "tools/perf/Documentation/perf.html". Two consecutive asterisks ("**") in patterns matched against full pathname may have special meaning: A leading "**" followed by a slash means match in all directories. For example, "**/foo" matches file or directory "foo" anywhere, the same as pattern "foo". "**/foo/bar" matches file or directory "bar" anywhere that is directly under directory "foo". A trailing "/**" matches everything inside. For example, "abc/**" matches all files inside directory "abc", relative to the location of the .gitignore file, with infinite depth. A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, "a/**/b" matches "a/b", "a/x/b", "a/x/y/b" and so on. Other consecutive asterisks are considered invalid. Glob magic is incompatible with literal magic. exclude After a path matches any non-exclude pathspec, it will be run through all exclude pathspec (magic signature: !). If it matches, the path is ignored. Which characters to escape(\) *:\/ ?? rooting (/*) of a path (a) $GIT_DIR (b) system root. directory (/) terminator (D/F conflict) Compare with <path>; <paths>; and <file> [glob(7) patterns] (see grep) ` file specifies intentionally untracked files that Git should ignore. Files already tracked by Git are not affected; see the NOTES below for details. a `pathspec` is specified by a pattern. When deciding whether a path matches a `pathspec` pattern, Git normally checks with the following order of precedence, from highest to lowest (within one level of precedence, the last matching pattern decides the outcome): * Patterns read from the command line for those commands that support them. * Patterns read from a `.pathspec` file in the same directory as the path, or in any parent directory, with patterns in the higher level files (up to the toplevel of the work tree) being overridden by those in lower level files down to the directory containing the file. These patterns match relative to the location of the `.pathspec` file. A project normally includes such `.pathspec` files in its repository, containing patterns for files generated as part of the project build. * Patterns read from `$GIT_DIR/info/exclude`. * Patterns read from the file specified by the configuration variable 'core.excludesfile'. Which file to place a pattern in depends on how the pattern is meant to be used. * Patterns which should be version-controlled and distributed to other repositories via clone (i.e., files that all developers will want to ignore) should go into a `.pathspec` file. * Patterns which are specific to a particular repository but which do not need to be shared with other related repositories (e.g., auxiliary files that live inside the repository but are specific to one user's workflow) should go into the `$GIT_DIR/info/exclude` file. * Patterns which a user wants Git to ignore in all situations (e.g., backup or temporary files generated by the user's editor of choice) generally go into a file specified by `core.excludesfile` in the user's `~/.gitconfig`. Its default value is $XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore is used instead. The underlying Git plumbing tools, such as 'git ls-files' and 'git read-tree', read `pathspec` patterns specified by command-line options, or from files specified by command-line options. Higher-level Git tools, such as 'git status' and 'git add', use patterns from the sources specified above. PATTERN FORMAT -------------- - A blank line matches no files, so it can serve as a separator for readability. - A line starting with # serves as a comment. Put a backslash ("`\`") in front of the first hash for patterns that begin with a hash. - An optional prefix "`!`" which negates the pattern; any matching file excluded by a previous pattern will become included again. It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn't list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined. Put a backslash ("`\`") in front of the first "`!`" for patterns that begin with a literal "`!`", for example, "`\!important!.txt`". - If the pattern ends with a slash, it is removed for the purpose of the following description, but it would only find a match with a directory. In other words, `foo/` will match a directory `foo` and paths underneath it, but will not match a regular file or a symbolic link `foo` (this is consistent with the way how pathspec works in general in Git). - If the pattern does not contain a slash '/', Git treats it as a shell glob pattern and checks for a match against the pathname relative to the location of the `.pathspec` file (relative to the toplevel of the work tree if not from a `.pathspec` file). - Otherwise, Git treats the pattern as a shell glob suitable for consumption by fnmatch(3) with the FNM_PATHNAME flag: wildcards in the pattern will not match a / in the pathname. For example, "Documentation/{asterisk}.html" matches "Documentation/git.html" but not "Documentation/ppc/ppc.html" or "tools/perf/Documentation/perf.html". - A leading slash matches the beginning of the pathname. For example, "/{asterisk}.c" matches "cat-file.c" but not "mozilla-sha1/sha1.c". Two consecutive asterisks ("`**`") in patterns matched against full pathname may have special meaning: - A leading "`**`" followed by a slash means match in all directories. For example, "`**/foo`" matches file or directory "`foo`" anywhere, the same as pattern "`foo`". "`**/foo/bar`" matches file or directory "`bar`" anywhere that is directly under directory "`foo`". - A trailing "`/**`" matches everything inside. For example, "`abc/**`" matches all files inside directory "`abc`", relative to the location of the `.pathspec` file, with infinite depth. - A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, "`a/**/b`" matches "`a/b`", "`a/x/b`", "`a/x/y/b`" and so on. - Other consecutive asterisks are considered invalid. NOTES ----- The purpose of pathspec files is to ensure that certain files not tracked by Git remain untracked. To ignore uncommitted changes in a file that is already tracked, use 'git update-index {litdd}assume-unchanged'. To stop tracking a file that is currently tracked, use 'git rm --cached'. EXAMPLES -------- -------------------------------------------------------------- $ git status [...] # Untracked files: [...] # Documentation/foo.html # Documentation/pathspec.html # file.o # lib.a # src/internal.o [...] $ cat .git/info/exclude # ignore objects and archives, anywhere in the tree. *.[oa] $ cat Documentation/.pathspec # ignore generated html files, *.html # except foo.html which is maintained by hand !foo.html $ git status [...] # Untracked files: [...] # Documentation/foo.html [...] -------------------------------------------------------------- Another example: -------------------------------------------------------------- $ cat .pathspec vmlinux* $ ls arch/foo/kernel/vm* arch/foo/kernel/vmlinux.lds.S $ echo '!/vmlinux*' >arch/foo/kernel/.pathspec -------------------------------------------------------------- The second .pathspec prevents Git from ignoring `arch/foo/kernel/vmlinux.lds.S`. Example to exclude everything except a specific directory `foo/bar` (note the `/*` - without the slash, the wildcard would also exclude everything within `foo/bar`): -------------------------------------------------------------- $ cat .pathspec # exclude everything except directory foo/bar /* !/foo /foo/* !/foo/bar -------------------------------------------------------------- SEE ALSO -------- linkgit:git-rm[1], linkgit:git-update-index[1], linkgit:gitrepository-layout[5], linkgit:git-check-ignore[1] GIT --- Part of the linkgit:git[1] suite ^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 2/2] grep: use '/' delimiter for paths 2017-01-19 15:03 [RFC 0/2] grep: make output consistent with revision syntax Stefan Hajnoczi 2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi @ 2017-01-19 15:03 ` Stefan Hajnoczi 2017-01-19 18:29 ` Brandon Williams 2017-01-19 18:54 ` Junio C Hamano 2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King 2 siblings, 2 replies; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-19 15:03 UTC (permalink / raw) To: git; +Cc: gitster, Stefan Hajnoczi If the tree contains a sub-directory then git-grep(1) output contains a colon character instead of a path separator: $ git grep malloc v2.9.3:t v2.9.3:t:test-lib.sh: setup_malloc_check () { $ git show v2.9.3:t:test-lib.sh fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3' This patch attempts to use the correct delimiter: $ git grep malloc v2.9.3:t v2.9.3:t/test-lib.sh: setup_malloc_check () { $ git show v2.9.3:t/test-lib.sh (success) This patch does not cope with @{1979-02-26 18:30:00} syntax and treats it as a path because it contains colons. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- builtin/grep.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index 3643d8a..06f8b47 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -493,7 +493,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, /* Add a delimiter if there isn't one already */ if (name[len - 1] != '/' && name[len - 1] != ':') { - strbuf_addch(&base, ':'); + /* rev: or rev:path/ */ + strbuf_addch(&base, strchr(name, ':') ? '/' : ':'); } } init_tree_desc(&tree, data, size); -- 2.9.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi @ 2017-01-19 18:29 ` Brandon Williams 2017-01-20 14:17 ` Stefan Hajnoczi 2017-01-19 18:54 ` Junio C Hamano 1 sibling, 1 reply; 21+ messages in thread From: Brandon Williams @ 2017-01-19 18:29 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git, gitster On 01/19, Stefan Hajnoczi wrote: > If the tree contains a sub-directory then git-grep(1) output contains a > colon character instead of a path separator: > > $ git grep malloc v2.9.3:t > v2.9.3:t:test-lib.sh: setup_malloc_check () { > $ git show v2.9.3:t:test-lib.sh > fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3' > > This patch attempts to use the correct delimiter: > > $ git grep malloc v2.9.3:t > v2.9.3:t/test-lib.sh: setup_malloc_check () { > $ git show v2.9.3:t/test-lib.sh > (success) > > This patch does not cope with @{1979-02-26 18:30:00} syntax and treats > it as a path because it contains colons. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > builtin/grep.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/builtin/grep.c b/builtin/grep.c > index 3643d8a..06f8b47 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -493,7 +493,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, > > /* Add a delimiter if there isn't one already */ > if (name[len - 1] != '/' && name[len - 1] != ':') { > - strbuf_addch(&base, ':'); > + /* rev: or rev:path/ */ > + strbuf_addch(&base, strchr(name, ':') ? '/' : ':'); As Jeff mentioned it may be better to base which character gets appended by checking the obj->type field like this maybe: diff --git a/builtin/grep.c b/builtin/grep.c index 69dab5dc5..9dfe11dc7 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -495,7 +495,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, /* Add a delimiter if there isn't one already */ if (name[len - 1] != '/' && name[len - 1] != ':') { /* rev: or rev:path/ */ - strbuf_addch(&base, strchr(name, ':') ? '/' : ':'); + char del = obj->type == OBJ_COMMIT ? ':' : '/'; + strbuf_addch(&base, del); } } init_tree_desc(&tree, data, size); -- Brandon Williams ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-19 18:29 ` Brandon Williams @ 2017-01-20 14:17 ` Stefan Hajnoczi 0 siblings, 0 replies; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-20 14:17 UTC (permalink / raw) To: Brandon Williams; +Cc: git, gitster [-- Attachment #1: Type: text/plain, Size: 2172 bytes --] On Thu, Jan 19, 2017 at 10:29:34AM -0800, Brandon Williams wrote: > On 01/19, Stefan Hajnoczi wrote: > > If the tree contains a sub-directory then git-grep(1) output contains a > > colon character instead of a path separator: > > > > $ git grep malloc v2.9.3:t > > v2.9.3:t:test-lib.sh: setup_malloc_check () { > > $ git show v2.9.3:t:test-lib.sh > > fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3' > > > > This patch attempts to use the correct delimiter: > > > > $ git grep malloc v2.9.3:t > > v2.9.3:t/test-lib.sh: setup_malloc_check () { > > $ git show v2.9.3:t/test-lib.sh > > (success) > > > > This patch does not cope with @{1979-02-26 18:30:00} syntax and treats > > it as a path because it contains colons. > > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > builtin/grep.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/builtin/grep.c b/builtin/grep.c > > index 3643d8a..06f8b47 100644 > > --- a/builtin/grep.c > > +++ b/builtin/grep.c > > @@ -493,7 +493,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, > > > > /* Add a delimiter if there isn't one already */ > > if (name[len - 1] != '/' && name[len - 1] != ':') { > > - strbuf_addch(&base, ':'); > > + /* rev: or rev:path/ */ > > + strbuf_addch(&base, strchr(name, ':') ? '/' : ':'); > > As Jeff mentioned it may be better to base which character gets appended > by checking the obj->type field like this maybe: > > diff --git a/builtin/grep.c b/builtin/grep.c > index 69dab5dc5..9dfe11dc7 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -495,7 +495,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec, > /* Add a delimiter if there isn't one already */ > if (name[len - 1] != '/' && name[len - 1] != ':') { > /* rev: or rev:path/ */ > - strbuf_addch(&base, strchr(name, ':') ? '/' : ':'); > + char del = obj->type == OBJ_COMMIT ? ':' : '/'; > + strbuf_addch(&base, del); Nice, that also solves the false positive with @{1979-02-26 18:30:00}. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi 2017-01-19 18:29 ` Brandon Williams @ 2017-01-19 18:54 ` Junio C Hamano 2017-01-20 14:12 ` Stefan Hajnoczi 1 sibling, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2017-01-19 18:54 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git Stefan Hajnoczi <stefanha@redhat.com> writes: > If the tree contains a sub-directory then git-grep(1) output contains a > colon character instead of a path separator: > > $ git grep malloc v2.9.3:t > v2.9.3:t:test-lib.sh: setup_malloc_check () { > $ git show v2.9.3:t:test-lib.sh > fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3' I am slightly less negative on this compared to 1/2, but not by a big margin. The user wanted to feed a subtree to the command, instead of doing the more natural $ git grep -e malloc v2.9.3 -- t So again, "contains a colon character" is not coming from what Git does, but the user gave Git "a colon character" when she didn't have to. Having said that, if we wanted to start ignoring what the end-user said in the initial input like 1/2 and 2/2 does (i.e. "this specific tree object" as an input), I think the approach to solve for 1/2 and 2/2 should be the same. I think we should decide to do a slash instead of a colon, not because v2.9.3: has colon at the end and v2.9.3:t has colon in it, but because both of these are both bare tree objects. The patches presented does not seem to base their decision on the actual object type but on the textual input, which seems wrong. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-19 18:54 ` Junio C Hamano @ 2017-01-20 14:12 ` Stefan Hajnoczi 2017-01-20 14:19 ` Jeff King 0 siblings, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-20 14:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: git [-- Attachment #1: Type: text/plain, Size: 2219 bytes --] On Thu, Jan 19, 2017 at 10:54:02AM -0800, Junio C Hamano wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > > If the tree contains a sub-directory then git-grep(1) output contains a > > colon character instead of a path separator: > > > > $ git grep malloc v2.9.3:t > > v2.9.3:t:test-lib.sh: setup_malloc_check () { > > $ git show v2.9.3:t:test-lib.sh > > fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3' > > I am slightly less negative on this compared to 1/2, but not by a > big margin. The user wanted to feed a subtree to the command, > instead of doing the more natural > > $ git grep -e malloc v2.9.3 -- t I find <rev>:<path> vs <rev> -- <path> confusing: | <rev>:<path> | <rev> -- <path> ----------+----------------------+--------------------- git grep | OK | OK ----------+----------------------+--------------------- git show | OK | <path> ignored ----------+----------------------+--------------------- git log | no output | OK ----------+----------------------+--------------------- Neither syntax always does what I expect. If git show <rev> -- <path> honored <path> then I could use that syntax consistently. Sorry for going on a tangent. Does it seem reasonable to handle <path> in git-show(1) as a UI convenience? > So again, "contains a colon character" is not coming from what Git > does, but the user gave Git "a colon character" when she didn't have > to. > > Having said that, if we wanted to start ignoring what the end-user > said in the initial input like 1/2 and 2/2 does (i.e. "this specific > tree object" as an input), I think the approach to solve for 1/2 and > 2/2 should be the same. I think we should decide to do a slash > instead of a colon, not because v2.9.3: has colon at the end and > v2.9.3:t has colon in it, but because both of these are both bare > tree objects. The patches presented does not seem to base their > decision on the actual object type but on the textual input, which > seems wrong. Yes, reparsing the name is ugly and I hoped to get feedback with this RFC. Thanks for the quick review! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-20 14:12 ` Stefan Hajnoczi @ 2017-01-20 14:19 ` Jeff King 2017-01-20 22:56 ` Junio C Hamano 0 siblings, 1 reply; 21+ messages in thread From: Jeff King @ 2017-01-20 14:19 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Junio C Hamano, git On Fri, Jan 20, 2017 at 02:12:12PM +0000, Stefan Hajnoczi wrote: > I find <rev>:<path> vs <rev> -- <path> confusing: > > | <rev>:<path> | <rev> -- <path> > ----------+----------------------+--------------------- > git grep | OK | OK > ----------+----------------------+--------------------- > git show | OK | <path> ignored > ----------+----------------------+--------------------- > git log | no output | OK > ----------+----------------------+--------------------- > > Neither syntax always does what I expect. If git show <rev> -- <path> > honored <path> then I could use that syntax consistently. > > Sorry for going on a tangent. Does it seem reasonable to handle <path> > in git-show(1) as a UI convenience? It's not ignored; just as with git-log, it's a pathspec to limit the diff. E.g.: $ git show --name-status v2.9.3 ... M Documentation/RelNotes/2.9.3.txt M Documentation/git.txt M GIT-VERSION-GEN $ git show --name-status v2.9.3 -- Documentation M Documentation/RelNotes/2.9.3.txt M Documentation/git.txt That's typically less useful than it is with log (where limiting the diff also kicks in history simplification and omits some commits entirely). But it does do something. -Peff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-20 14:19 ` Jeff King @ 2017-01-20 22:56 ` Junio C Hamano 2017-01-23 13:29 ` Stefan Hajnoczi 0 siblings, 1 reply; 21+ messages in thread From: Junio C Hamano @ 2017-01-20 22:56 UTC (permalink / raw) To: Jeff King; +Cc: Stefan Hajnoczi, git Jeff King <peff@peff.net> writes: > It's not ignored; just as with git-log, it's a pathspec to limit the > diff. E.g.: > > $ git show --name-status v2.9.3 > ... > M Documentation/RelNotes/2.9.3.txt > M Documentation/git.txt > M GIT-VERSION-GEN > > $ git show --name-status v2.9.3 -- Documentation > M Documentation/RelNotes/2.9.3.txt > M Documentation/git.txt > > That's typically less useful than it is with log (where limiting the > diff also kicks in history simplification and omits some commits > entirely). But it does do something. I think Stefan is missing the fact that the argument to "git show <tree-ish>:<path>" actually is naming a blob that sits at the <path> in the <tree-ish>. In other words, "show" is acting as a glorified "git -p cat-file blob", in that use. The use of "git show" you are demonstrating is still about showing the commit object, whose behaviour is defined to show the log message and the diff relative to its sole parent, limited to the paths that match the pathspec. It is perfectly fine and desirable that "git show <commit>:<path>" and "git show <commit> -- <path>" behaves differently. These are two completely different features. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-20 22:56 ` Junio C Hamano @ 2017-01-23 13:29 ` Stefan Hajnoczi 2017-01-24 17:18 ` Phil Hord 0 siblings, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-23 13:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jeff King, git [-- Attachment #1: Type: text/plain, Size: 1531 bytes --] On Fri, Jan 20, 2017 at 02:56:26PM -0800, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > It's not ignored; just as with git-log, it's a pathspec to limit the > > diff. E.g.: > > > > $ git show --name-status v2.9.3 > > ... > > M Documentation/RelNotes/2.9.3.txt > > M Documentation/git.txt > > M GIT-VERSION-GEN > > > > $ git show --name-status v2.9.3 -- Documentation > > M Documentation/RelNotes/2.9.3.txt > > M Documentation/git.txt > > > > That's typically less useful than it is with log (where limiting the > > diff also kicks in history simplification and omits some commits > > entirely). But it does do something. > > I think Stefan is missing the fact that the argument to "git show > <tree-ish>:<path>" actually is naming a blob that sits at the <path> > in the <tree-ish>. In other words, "show" is acting as a glorified > "git -p cat-file blob", in that use. > > The use of "git show" you are demonstrating is still about showing > the commit object, whose behaviour is defined to show the log > message and the diff relative to its sole parent, limited to the > paths that match the pathspec. > > It is perfectly fine and desirable that "git show <commit>:<path>" > and "git show <commit> -- <path>" behaves differently. These are > two completely different features. Thanks for explaining guys. It all makes logical sense. I just hope I remember the distinctions in that table for everyday git use. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 2/2] grep: use '/' delimiter for paths 2017-01-23 13:29 ` Stefan Hajnoczi @ 2017-01-24 17:18 ` Phil Hord 0 siblings, 0 replies; 21+ messages in thread From: Phil Hord @ 2017-01-24 17:18 UTC (permalink / raw) To: Stefan Hajnoczi, Junio C Hamano; +Cc: Jeff King, Git On Tue, Jan 24, 2017 at 1:54 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > The use of "git show" you are demonstrating is still about showing > > the commit object, whose behaviour is defined to show the log > > message and the diff relative to its sole parent, limited to the > > paths that match the pathspec. > > > > It is perfectly fine and desirable that "git show <commit>:<path>" > > and "git show <commit> -- <path>" behaves differently. These are > > two completely different features. > > Thanks for explaining guys. It all makes logical sense. I just hope I > remember the distinctions in that table for everyday git use. Familiar itch; previous discussion here: https://public-inbox.org/git/1377528372-31206-1-git-send-email-hordp@cisco.com/ Phil ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/2] grep: make output consistent with revision syntax 2017-01-19 15:03 [RFC 0/2] grep: make output consistent with revision syntax Stefan Hajnoczi 2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi 2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi @ 2017-01-19 16:59 ` Jeff King 2017-01-19 18:26 ` Brandon Williams 2017-01-20 14:18 ` Stefan Hajnoczi 2 siblings, 2 replies; 21+ messages in thread From: Jeff King @ 2017-01-19 16:59 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git, gitster On Thu, Jan 19, 2017 at 03:03:45PM +0000, Stefan Hajnoczi wrote: > git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax. > > This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1) > and expect "git show rev:path/to/file.c" to work. See the individual patches > for examples of command-lines that produce invalid output. I think this is a good goal. I couldn't immediately think of any cases where your patches would misbehave, but my initial thought was that the "/" versus ":" distinction is about whether the initial object is a tree or a commit. You do still have to special case the root tree (so "v2.9.3:" does not get any delimiter). I think "ends in a colon" is actually a reasonable way of determining that. > This series is an incomplete attempt at solving the issue. I'm not familiar > enough with the git codebase to propose a better solution. Perhaps someone is > interested in a proper fix? Are there cases you know that aren't covered by your patches? -Peff ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/2] grep: make output consistent with revision syntax 2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King @ 2017-01-19 18:26 ` Brandon Williams 2017-01-20 14:18 ` Stefan Hajnoczi 1 sibling, 0 replies; 21+ messages in thread From: Brandon Williams @ 2017-01-19 18:26 UTC (permalink / raw) To: Jeff King; +Cc: Stefan Hajnoczi, git, gitster On 01/19, Jeff King wrote: > On Thu, Jan 19, 2017 at 03:03:45PM +0000, Stefan Hajnoczi wrote: > > > git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax. > > > > This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1) > > and expect "git show rev:path/to/file.c" to work. See the individual patches > > for examples of command-lines that produce invalid output. > > I think this is a good goal. I agree. > I couldn't immediately think of any cases where your patches would > misbehave, but my initial thought was that the "/" versus ":" > distinction is about whether the initial object is a tree or a commit. I think this is also the case, I couldn't think of another case where this decision wasn't based on if the object is a tree or a commit. Interestingly enough I don't think we have any tests that exist that test the formatting of grep's output when given a tree object since the test suite still passes with these changes in. Which means this fix should probably include a couple tests to ensure there's no regression in the future. -- Brandon Williams ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/2] grep: make output consistent with revision syntax 2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King 2017-01-19 18:26 ` Brandon Williams @ 2017-01-20 14:18 ` Stefan Hajnoczi 2017-01-20 14:32 ` Jeff King 1 sibling, 1 reply; 21+ messages in thread From: Stefan Hajnoczi @ 2017-01-20 14:18 UTC (permalink / raw) To: Jeff King; +Cc: git, gitster [-- Attachment #1: Type: text/plain, Size: 1377 bytes --] On Thu, Jan 19, 2017 at 11:59:59AM -0500, Jeff King wrote: > On Thu, Jan 19, 2017 at 03:03:45PM +0000, Stefan Hajnoczi wrote: > > > git-grep(1)'s output is not consistent with git-rev-parse(1) revision syntax. > > > > This means you cannot take "rev:path/to/file.c: foo();" output from git-grep(1) > > and expect "git show rev:path/to/file.c" to work. See the individual patches > > for examples of command-lines that produce invalid output. > > I think this is a good goal. > > I couldn't immediately think of any cases where your patches would > misbehave, but my initial thought was that the "/" versus ":" > distinction is about whether the initial object is a tree or a commit. > > You do still have to special case the root tree (so "v2.9.3:" does not > get any delimiter). I think "ends in a colon" is actually a reasonable > way of determining that. > > > This series is an incomplete attempt at solving the issue. I'm not familiar > > enough with the git codebase to propose a better solution. Perhaps someone is > > interested in a proper fix? > > Are there cases you know that aren't covered by your patches? From Patch 2/2: This patch does not cope with @{1979-02-26 18:30:00} syntax and treats it as a path because it contains colons. If we use obj->type instead of re-parsing the name then that problem is solved. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 455 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC 0/2] grep: make output consistent with revision syntax 2017-01-20 14:18 ` Stefan Hajnoczi @ 2017-01-20 14:32 ` Jeff King 0 siblings, 0 replies; 21+ messages in thread From: Jeff King @ 2017-01-20 14:32 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: git, gitster On Fri, Jan 20, 2017 at 02:18:32PM +0000, Stefan Hajnoczi wrote: > > Are there cases you know that aren't covered by your patches? > > From Patch 2/2: > > This patch does not cope with @{1979-02-26 18:30:00} syntax and treats > it as a path because it contains colons. > > If we use obj->type instead of re-parsing the name then that problem is > solved. Ah, right. I somehow totally missed that, and blindly assumed your colon search was only at the end. But of course that wouldn't work at all (it would miss "v2.9.3:t"). So yes, definitely it should be checking the object type. -Peff ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2017-01-24 20:50 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-19 15:03 [RFC 0/2] grep: make output consistent with revision syntax Stefan Hajnoczi 2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi 2017-01-19 18:39 ` Junio C Hamano 2017-01-20 13:56 ` Stefan Hajnoczi 2017-01-20 18:16 ` Junio C Hamano 2017-01-23 13:15 ` Stefan Hajnoczi 2017-01-24 19:07 ` Jakub Narębski 2017-01-24 20:48 ` Philip Oakley 2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi 2017-01-19 18:29 ` Brandon Williams 2017-01-20 14:17 ` Stefan Hajnoczi 2017-01-19 18:54 ` Junio C Hamano 2017-01-20 14:12 ` Stefan Hajnoczi 2017-01-20 14:19 ` Jeff King 2017-01-20 22:56 ` Junio C Hamano 2017-01-23 13:29 ` Stefan Hajnoczi 2017-01-24 17:18 ` Phil Hord 2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King 2017-01-19 18:26 ` Brandon Williams 2017-01-20 14:18 ` Stefan Hajnoczi 2017-01-20 14:32 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).