On Mon, Apr 23 2018, Taylor Blau wrote: > For your consideration: https://github.com/ttaylorr/git/compare/tb/grep-colno Looks good to me aside from two minor issues I noticed: * In "grep.c: display column number of first match" you use a comment style we don't normally use, i.e. /**\n not /*\n. See "Multi-line comments" in Documentation/CodingGuidelines. * You're not updating contrib/git-jump/README to document the new output format. It just refers to the old format, but now depending on if you use "grep" or not it'll use this new thing. It also makes sense to update the example added in 007d06aa57 ("contrib/git-jump: allow to configure the grep command", 2017-11-20) which seems to have added jump.grepCmd as a workaround for not having this. But also, after just looking at this the second time around; Is there a reason we shouldn't just call this --column, not --column-number? I realize the former works because of the lazyness of our getopt parsing (also --colu though..). I think when we add features to git-grep we should be as close to GNU grep as possible (e.g. not add this -m alias meaning something different as in your v1), but if GNU grep doesn't have something go with the trend of other grep tools, as noted at https://beyondgrep.com/feature-comparison/ (and I found another one that has this: https://github.com/beyondgrep/website/pull/83), so there's already 3 prominent grep tools that call this just --column. I think we should just go with that. Also, as a bonus question, since you're poking at this column code anyway, interested in implementing -o (--only-matching)? That would be super-useful (see https://public-inbox.org/git/87in9ucsbb.fsf@xxxxxxxxxxxxxxxxxxx/) :)
