* [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]
@ 2010-06-05 0:51 Jonathan Nieder
2010-06-05 0:53 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Jonathan Nieder
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: Jonathan Nieder @ 2010-06-05 0:51 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Junio C Hamano
This series teaches ‘git grep’ to accept
git grep -Ovi some_function_name
to find all the call sites for some_function_name so they can be
adjusted. Dscho’s cover letter[1] explains it better.
And in fact, Dscho did all the work here; this iteration just rebases it
on master and adds some tests.
Happy hacking,
Jonathan
Johannes Schindelin (2):
grep: Add the option '--open-files-in-pager'
grep -O: allow optional argument specifying the pager (or editor)
Documentation/git-grep.txt | 8 +
builtin/grep.c | 81 +++++++-
git.c | 2 +-
t/lib-pager.sh | 15 ++
t/t7002-grep.sh | 530 --------------------------------------------
t/t7006-pager.sh | 16 +-
t/t7810-grep.sh | 530 ++++++++++++++++++++++++++++++++++++++++++++
t/t7811-grep-open.sh | 157 +++++++++++++
8 files changed, 792 insertions(+), 547 deletions(-)
create mode 100644 t/lib-pager.sh
delete mode 100755 t/t7002-grep.sh
create mode 100755 t/t7810-grep.sh
create mode 100644 t/t7811-grep-open.sh
[1] http://thread.gmane.org/gmane.comp.version-control.git/143219
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-06-05 0:51 [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Jonathan Nieder @ 2010-06-05 0:53 ` Jonathan Nieder 2010-06-05 1:40 ` Jonathan Nieder 2010-06-05 0:55 ` [PATCH 2/2] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2010-06-05 0:53 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Junio C Hamano From: Johannes Schindelin <johannes.schindelin@gmx.de> This adds an option to open the matching files in the pager, and if the pager happens to be "less" (or "vi") and there is only one grep pattern, it also jumps to the first match right away. The short option was chose as '-O' to avoid clashes with GNU grep's options (as suggested by Junio). So, 'git grep -O abc' is a short form for 'less +/abc $(grep -l abc)' except that it works also with spaces in file names, and it does not start the pager if there was no matching file. [jn: rebased and added tests; with error handling fix from Junio squashed in] Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Documentation/git-grep.txt | 8 ++ builtin/grep.c | 83 ++++++++++++++++++- git.c | 2 +- t/lib-pager.sh | 15 ++++ t/t7006-pager.sh | 16 +--- t/{t7002-grep.sh => t7810-grep.sh} | 0 t/t7811-grep-open.sh | 154 ++++++++++++++++++++++++++++++++++++ 7 files changed, 261 insertions(+), 17 deletions(-) create mode 100644 t/lib-pager.sh rename t/{t7002-grep.sh => t7810-grep.sh} (100%) create mode 100644 t/t7811-grep-open.sh diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 4b32322..8fdd8e1 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -14,6 +14,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] + [-O | --open-files-in-pager] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -104,6 +105,13 @@ OPTIONS For better compatibility with 'git diff', `--name-only` is a synonym for `--files-with-matches`. +-O:: +--open-files-in-pager:: + Open the matching files in the pager (not the output of 'grep'). + If the pager happens to be "less" or "vi", and the user + specified only one pattern, the first file is positioned at + the first match automatically. + -z:: --null:: Output \0 instead of the character that normally follows a diff --git a/builtin/grep.c b/builtin/grep.c index b194ea3..bc6c2ea 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -11,6 +11,8 @@ #include "tree-walk.h" #include "builtin.h" #include "parse-options.h" +#include "string-list.h" +#include "run-command.h" #include "userdiff.h" #include "grep.h" #include "quote.h" @@ -556,7 +558,34 @@ static int grep_file(struct grep_opt *opt, const char *filename) } } -static int grep_cache(struct grep_opt *opt, const char **paths, int cached) +static void append_path(struct grep_opt *opt, const void *data, size_t len) +{ + struct string_list *path_list = opt->output_priv; + + if (len == 1 && *(const char *)data == '\0') + return; + string_list_append(xstrndup(data, len), path_list); +} + +static void run_pager(struct grep_opt *opt, const char *prefix) +{ + struct string_list *path_list = opt->output_priv; + const char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1)); + int i, status; + + for (i = 0; i < path_list->nr; i++) + argv[i] = path_list->items[i].string; + argv[path_list->nr] = NULL; + + if (prefix && chdir(prefix)) + error("Failed to chdir: %s", prefix); + status = run_command_v_opt(argv, RUN_USING_SHELL); + if (status) + exit(status); +} + +static int grep_cache(struct grep_opt *opt, const char **paths, + int cached, int show_in_pager, const char *prefix) { int hit = 0; int nr; @@ -590,6 +619,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached) if (hit && opt->status_only) break; } + if (hit && show_in_pager) + run_pager(opt, prefix); free_grep_patterns(opt); return hit; } @@ -675,7 +706,8 @@ static int grep_object(struct grep_opt *opt, const char **paths, die("unable to grep from object of type %s", typename(obj->type)); } -static int grep_directory(struct grep_opt *opt, const char **paths) +static int grep_directory(struct grep_opt *opt, const char **paths, + int show_in_pager, const char *prefix) { struct dir_struct dir; int i, hit = 0; @@ -689,6 +721,8 @@ static int grep_directory(struct grep_opt *opt, const char **paths) if (hit && opt->status_only) break; } + if (hit && show_in_pager) + run_pager(opt, prefix); free_grep_patterns(opt); return hit; } @@ -782,9 +816,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix) int cached = 0; int seen_dashdash = 0; int external_grep_allowed__ignored; + int show_in_pager = 0; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; const char **paths = NULL; + struct string_list path_list = { NULL, 0, 0, 0 }; int i; int dummy; int nongit = 0, use_index = 1; @@ -868,6 +904,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) OPT_BOOLEAN(0, "all-match", &opt.all_match, "show only matches from files that match all patterns"), OPT_GROUP(""), + OPT_BOOLEAN('O', "open-files-in-pager", &show_in_pager, + "show matching files in the pager"), OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored, "allow calling of grep(1) (ignored by this build)"), { OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage", @@ -943,6 +981,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix) argc--; } + if (show_in_pager) { + const char *pager = git_pager(1); + if (!pager) { + show_in_pager = 0; + } else { + opt.name_only = 1; + opt.null_following_name = 1; + opt.output_priv = &path_list; + opt.output = append_path; + string_list_append(pager, &path_list); + use_threads = 0; + } + } + if (!opt.pattern_list) die("no pattern given."); if (!opt.fixed && opt.ignore_case) @@ -999,13 +1051,36 @@ int cmd_grep(int argc, const char **argv, const char *prefix) paths[1] = NULL; } + if (show_in_pager && (cached || list.nr)) + die("--open-files-in-pager only works on the worktree"); + + if (show_in_pager && opt.pattern_list && !opt.pattern_list->next) { + const char *pager = path_list.items[0].string; + int len = strlen(pager); + + if (len > 4 && is_dir_sep(pager[len - 5])) + pager += len - 4; + + if (!strcmp("less", pager) || !strcmp("vi", pager)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "+/%s%s", + strcmp("less", pager) ? "" : "*", + opt.pattern_list->pattern); + string_list_append(buf.buf, &path_list); + strbuf_detach(&buf, NULL); + } + } + + if (!show_in_pager) + setup_pager(); + if (!use_index) { int hit; if (cached) die("--cached cannot be used with --no-index."); if (list.nr) die("--no-index cannot be used with revs."); - hit = grep_directory(&opt, paths); + hit = grep_directory(&opt, paths, show_in_pager, prefix); if (use_threads) hit |= wait_all(); return !hit; @@ -1016,7 +1091,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (!cached) setup_work_tree(); - hit = grep_cache(&opt, paths, cached); + hit = grep_cache(&opt, paths, cached, show_in_pager, prefix); if (use_threads) hit |= wait_all(); return !hit; diff --git a/git.c b/git.c index 99f0363..265fa09 100644 --- a/git.c +++ b/git.c @@ -329,7 +329,7 @@ static void handle_internal_command(int argc, const char **argv) { "fsck-objects", cmd_fsck, RUN_SETUP }, { "gc", cmd_gc, RUN_SETUP }, { "get-tar-commit-id", cmd_get_tar_commit_id }, - { "grep", cmd_grep, USE_PAGER }, + { "grep", cmd_grep }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, { "index-pack", cmd_index_pack }, diff --git a/t/lib-pager.sh b/t/lib-pager.sh new file mode 100644 index 0000000..f8c6025 --- /dev/null +++ b/t/lib-pager.sh @@ -0,0 +1,15 @@ +#!/bin/sh + +test_expect_success 'determine default pager' ' + test_might_fail git config --unset core.pager && + less=$( + unset PAGER GIT_PAGER; + git var GIT_PAGER + ) && + test -n "$less" +' + +if expr "$less" : '^[a-z][a-z]*$' >/dev/null +then + test_set_prereq SIMPLEPAGER +fi diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 3bc7a2a..fc993fc 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -3,6 +3,7 @@ test_description='Test automatic use of a pager.' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-pager.sh cleanup_fail() { echo >&2 cleanup failed @@ -158,21 +159,12 @@ test_expect_success 'color when writing to a file intended for a pager' ' colorful colorful.log ' -test_expect_success 'determine default pager' ' - unset PAGER GIT_PAGER && - test_might_fail git config --unset core.pager || - cleanup_fail && - - less=$(git var GIT_PAGER) && - test -n "$less" -' - -if expr "$less" : '^[a-z][a-z]*$' >/dev/null && test_have_prereq TTY +if test_have_prereq SIMPLEPAGER && test_have_prereq TTY then - test_set_prereq SIMPLEPAGER + test_set_prereq SIMPLEPAGERTTY fi -test_expect_success SIMPLEPAGER 'default pager is used by default' ' +test_expect_success SIMPLEPAGERTTY 'default pager is used by default' ' unset PAGER GIT_PAGER && test_might_fail git config --unset core.pager && rm -f default_pager_used || diff --git a/t/t7002-grep.sh b/t/t7810-grep.sh similarity index 100% rename from t/t7002-grep.sh rename to t/t7810-grep.sh diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh new file mode 100644 index 0000000..72e4023 --- /dev/null +++ b/t/t7811-grep-open.sh @@ -0,0 +1,154 @@ +#!/bin/sh + +test_description='git grep --open-files-in-pager +' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-pager.sh +unset PAGER GIT_PAGER + +test_expect_success 'setup' ' + test_commit initial grep.h " +enum grep_pat_token { + GREP_PATTERN, + GREP_PATTERN_HEAD, + GREP_PATTERN_BODY, + GREP_AND, + GREP_OPEN_PAREN, + GREP_CLOSE_PAREN, + GREP_NOT, + GREP_OR, +};" && + + test_commit add-user revision.c " + } + if (seen_dashdash) + read_pathspec_from_stdin(revs, &sb, prune); + strbuf_release(&sb); +} + +static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what) +{ + append_grep_pattern(&revs->grep_filter, ptn, \"command line\", 0, what); +" && + + mkdir subdir && + test_commit subdir subdir/grep.c "enum grep_pat_token" && + + test_commit uninteresting unrelated "hello, world" && + + echo GREP_PATTERN >untracked +' + +test_expect_success SIMPLEPAGER 'git grep -O' ' + cat >$less <<-\EOF && + #!/bin/sh + printf "%s\n" "$@" >pager-args + EOF + chmod +x $less && + cat >expect.less <<-\EOF && + +/*GREP_PATTERN + grep.h + EOF + echo grep.h >expect.notless && + >empty && + + PATH=.:$PATH git grep -O GREP_PATTERN >out && + { + test_cmp expect.less pager-args || + test_cmp expect.notless pager-args + } && + test_cmp empty out +' + +test_expect_success 'git grep -O --cached' ' + test_must_fail git grep --cached -O GREP_PATTERN >out 2>msg && + grep open-files-in-pager msg +' + +test_expect_success 'git grep -O --no-index' ' + rm -f expect.less pager-args out && + cat >expect <<-\EOF && + grep.h + untracked + EOF + >empty && + + ( + GIT_PAGER='\''printf "%s\n" >pager-args'\'' && + export GIT_PAGER && + git grep --no-index -O GREP_PATTERN >out + ) && + test_cmp expect pager-args && + test_cmp empty out +' + +test_expect_success 'setup: fake "less"' ' + cat >less <<-\EOF + #!/bin/sh + printf "%s\n" "$@" >actual + EOF +' + +test_expect_success 'git grep -O jumps to line in less' ' + cat >expect <<-\EOF && + +/*GREP_PATTERN + grep.h + EOF + >empty && + + GIT_PAGER=./less git grep -O GREP_PATTERN >out + test_cmp expect actual && + test_cmp empty out +' + +test_expect_success 'modified file' ' + rm -f actual && + cat >less <<-\EOF && + #!/bin/sh + printf "%s\n" "$@" >actual + EOF + chmod +x $less && + cat >expect <<-\EOF && + +/*enum grep_pat_token + grep.h + revision.c + subdir/grep.c + unrelated + EOF + >empty && + + echo "enum grep_pat_token" >unrelated && + test_when_finished "git checkout HEAD unrelated" && + GIT_PAGER=./less git grep -F -O "enum grep_pat_token" >out && + test_cmp expect actual && + test_cmp empty out +' + +test_expect_success 'run from subdir' ' + rm -f actual && + echo grep.c >expect && + >empty && + + ( + cd subdir && + export GIT_PAGER && + GIT_PAGER='\''printf "%s\n" >../args'\'' && + git grep -O "enum grep_pat_token" >../out + GIT_PAGER="pwd >../dir; :" && + git grep -O "enum grep_pat_token" >../out2 + ) && + case $(cat dir) in + *subdir) + : good + ;; + *) + false + ;; + esac && + test_cmp expect args && + test_cmp empty out && + test_cmp empty out2 +' + +test_done -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-06-05 0:53 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Jonathan Nieder @ 2010-06-05 1:40 ` Jonathan Nieder 0 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-05 1:40 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Junio C Hamano Jonathan Nieder wrote: > +test_expect_success 'git grep -O jumps to line in less' ' > + cat >expect <<-\EOF && > + +/*GREP_PATTERN > + grep.h > + EOF > + >empty && > + > + GIT_PAGER=./less git grep -O GREP_PATTERN >out > + test_cmp expect actual && Ugh. We need a static analyzer for shell scripts. :) Short of that, here’s a fixup to squash in. Sorry for the trouble. diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh index 72e4023..fcfc56e 100644 --- a/t/t7811-grep-open.sh +++ b/t/t7811-grep-open.sh @@ -97,7 +97,7 @@ test_expect_success 'git grep -O jumps to line in less' ' EOF >empty && - GIT_PAGER=./less git grep -O GREP_PATTERN >out + GIT_PAGER=./less git grep -O GREP_PATTERN >out && test_cmp expect actual && test_cmp empty out ' @@ -134,7 +134,7 @@ test_expect_success 'run from subdir' ' cd subdir && export GIT_PAGER && GIT_PAGER='\''printf "%s\n" >../args'\'' && - git grep -O "enum grep_pat_token" >../out + git grep -O "enum grep_pat_token" >../out && GIT_PAGER="pwd >../dir; :" && git grep -O "enum grep_pat_token" >../out2 ) && -- ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] grep -O: allow optional argument specifying the pager (or editor) 2010-06-05 0:51 [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Jonathan Nieder 2010-06-05 0:53 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Jonathan Nieder @ 2010-06-05 0:55 ` Jonathan Nieder 2010-06-05 16:11 ` [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin 2010-06-12 11:38 ` [PATCH v2 0/2] " Paolo Bonzini 3 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-05 0:55 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Junio C Hamano From: Johannes Schindelin <johannes.schindelin@gmx.de> Suppose you want to edit all files that contain a specific search term. Of course, you can do something totally trivial such as git grep -z -e <term> | xargs -0r vi +/<term> but maybe you are happy that the same will be achieved by git grep -Ovi <term> now. [jn: rebased and added tests] Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Things I didn’t think hard about: - the exit status - the special-casing of "less" and "vi" Maybe the latter could be improved by also accepting "more" and "vim", which both will accept the +/<term> option. Thanks for reading. Documentation/git-grep.txt | 6 +++--- builtin/grep.c | 32 +++++++++++++++----------------- t/t7811-grep-open.sh | 11 +++++++---- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 8fdd8e1..d89ec32 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -14,7 +14,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] - [-O | --open-files-in-pager] + [(-O | --open-files-in-pager) [<pager>]] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -105,8 +105,8 @@ OPTIONS For better compatibility with 'git diff', `--name-only` is a synonym for `--files-with-matches`. --O:: ---open-files-in-pager:: +-O [<pager>]:: +--open-files-in-pager [<pager>]:: Open the matching files in the pager (not the output of 'grep'). If the pager happens to be "less" or "vi", and the user specified only one pattern, the first file is positioned at diff --git a/builtin/grep.c b/builtin/grep.c index bc6c2ea..f9551a7 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -584,8 +584,8 @@ static void run_pager(struct grep_opt *opt, const char *prefix) exit(status); } -static int grep_cache(struct grep_opt *opt, const char **paths, - int cached, int show_in_pager, const char *prefix) +static int grep_cache(struct grep_opt *opt, const char **paths, int cached, + const char *show_in_pager, const char *prefix) { int hit = 0; int nr; @@ -707,7 +707,7 @@ static int grep_object(struct grep_opt *opt, const char **paths, } static int grep_directory(struct grep_opt *opt, const char **paths, - int show_in_pager, const char *prefix) + const char *show_in_pager, const char *prefix) { struct dir_struct dir; int i, hit = 0; @@ -816,7 +816,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) int cached = 0; int seen_dashdash = 0; int external_grep_allowed__ignored; - int show_in_pager = 0; + const char *show_in_pager = NULL, *default_pager = "dummy"; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; const char **paths = NULL; @@ -904,8 +904,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) OPT_BOOLEAN(0, "all-match", &opt.all_match, "show only matches from files that match all patterns"), OPT_GROUP(""), - OPT_BOOLEAN('O', "open-files-in-pager", &show_in_pager, - "show matching files in the pager"), + { OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager, + "pager", "show matching files in the pager", + PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager }, OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored, "allow calling of grep(1) (ignored by this build)"), { OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage", @@ -981,18 +982,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix) argc--; } + if (show_in_pager == default_pager) + show_in_pager = git_pager(1); if (show_in_pager) { - const char *pager = git_pager(1); - if (!pager) { - show_in_pager = 0; - } else { - opt.name_only = 1; - opt.null_following_name = 1; - opt.output_priv = &path_list; - opt.output = append_path; - string_list_append(pager, &path_list); - use_threads = 0; - } + opt.name_only = 1; + opt.null_following_name = 1; + opt.output_priv = &path_list; + opt.output = append_path; + string_list_append(show_in_pager, &path_list); + use_threads = 0; } if (!opt.pattern_list) diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh index 72e4023..fdf45b8 100644 --- a/t/t7811-grep-open.sh +++ b/t/t7811-grep-open.sh @@ -97,9 +97,13 @@ test_expect_success 'git grep -O jumps to line in less' ' EOF >empty && - GIT_PAGER=./less git grep -O GREP_PATTERN >out + GIT_PAGER=./less git grep -O GREP_PATTERN >out1 test_cmp expect actual && - test_cmp empty out + test_cmp empty out1 && + + git grep -O./less GREP_PATTERN >out2 && + test_cmp expect actual && + test_cmp empty out2 ' test_expect_success 'modified file' ' @@ -135,8 +139,7 @@ test_expect_success 'run from subdir' ' export GIT_PAGER && GIT_PAGER='\''printf "%s\n" >../args'\'' && git grep -O "enum grep_pat_token" >../out - GIT_PAGER="pwd >../dir; :" && - git grep -O "enum grep_pat_token" >../out2 + git grep -O"pwd >../dir; :" "enum grep_pat_token" >../out2 ) && case $(cat dir) in *subdir) -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-05 0:51 [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Jonathan Nieder 2010-06-05 0:53 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Jonathan Nieder 2010-06-05 0:55 ` [PATCH 2/2] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder @ 2010-06-05 16:11 ` Johannes Schindelin 2010-06-12 7:55 ` Jonathan Nieder 2010-06-12 11:38 ` [PATCH v2 0/2] " Paolo Bonzini 3 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2010-06-05 16:11 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Junio C Hamano [-- Attachment #1: Type: TEXT/PLAIN, Size: 694 bytes --] Hi, On Fri, 4 Jun 2010, Jonathan Nieder wrote: > This series teaches ‘git grep’ to accept > > git grep -Ovi some_function_name > > to find all the call sites for some_function_name so they can be > adjusted. Dscho’s cover letter[1] explains it better. > > And in fact, Dscho did all the work here; this iteration just rebases it > on master and adds some tests. > > Happy hacking, > Jonathan > > Johannes Schindelin (2): > grep: Add the option '--open-files-in-pager' > grep -O: allow optional argument specifying the pager (or editor) Last time I tried, it also needed the patch "Unify code paths of threaded greps". Don't know if that one made it in already. Ciao, Dscho ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-05 16:11 ` [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin @ 2010-06-12 7:55 ` Jonathan Nieder 2010-06-12 9:46 ` Johannes Schindelin 0 siblings, 1 reply; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 7:55 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano Johannes Schindelin wrote: > On Fri, 4 Jun 2010, Jonathan Nieder wrote: >> Johannes Schindelin (2): >> grep: Add the option '--open-files-in-pager' >> grep -O: allow optional argument specifying the pager (or editor) > > Last time I tried, it also needed the patch "Unify code paths of threaded > greps". Don't know if that one made it in already. Thanks for the pointer, but I can’t seem to find any such patch. Maybe it was squashed into Fredrik’s patch (5b594f4)? Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-12 7:55 ` Jonathan Nieder @ 2010-06-12 9:46 ` Johannes Schindelin 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder 0 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2010-06-12 9:46 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Junio C Hamano [-- Attachment #1: Type: TEXT/PLAIN, Size: 853 bytes --] Hi, On Sat, 12 Jun 2010, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > On Fri, 4 Jun 2010, Jonathan Nieder wrote: > > >> Johannes Schindelin (2): > >> grep: Add the option '--open-files-in-pager' > >> grep -O: allow optional argument specifying the pager (or editor) > > > > Last time I tried, it also needed the patch "Unify code paths of threaded > > greps". Don't know if that one made it in already. > > Thanks for the pointer, but I can’t seem to find any such patch. Maybe > it was squashed into Fredrik’s patch (5b594f4)? Nope, the patch is still there, even after a rebase. In any case, the stuff is available in 4msysgit.git, and working (I was sick and tired of maintaining two different forks, and it was costing me too much time and work anyway, so I made 4msysgit.git's devel my main branch). Thanks, Johannes ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-12 9:46 ` Johannes Schindelin @ 2010-06-12 16:29 ` Jonathan Nieder 2010-06-12 16:31 ` [PATCH 1/4] grep: refactor grep_objects loop into its own function Jonathan Nieder ` (4 more replies) 0 siblings, 5 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 16:29 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano Johannes Schindelin wrote: > Nope, the patch is still there, even after a rebase. Ah, I see. Looks like a good patch (d00d655, Unify code paths of threaded greps, 2010-02-07) though in principle a separate topic, since grep -O already sets use_threads = 0. I think it is good to base grep-O on it because it allows cleaner code. Here’s a rebased version. Johannes Schindelin (3): Unify code paths of threaded greps grep: Add the option '--open-files-in-pager' grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder (1): grep: refactor grep_objects loop into its own function Documentation/git-grep.txt | 8 ++ builtin/grep.c | 121 ++++++++++++++++++++++------ git.c | 2 +- t/lib-pager.sh | 15 ++++ t/t7006-pager.sh | 16 +--- t/{t7002-grep.sh => t7810-grep.sh} | 0 t/t7811-grep-open.sh | 157 ++++++++++++++++++++++++++++++++++++ 7 files changed, 280 insertions(+), 39 deletions(-) create mode 100644 t/lib-pager.sh rename t/{t7002-grep.sh => t7810-grep.sh} (100%) create mode 100755 t/t7811-grep-open.sh ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] grep: refactor grep_objects loop into its own function 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder @ 2010-06-12 16:31 ` Jonathan Nieder 2010-06-12 16:32 ` [PATCH 2/4] Unify code paths of threaded greps Jonathan Nieder ` (3 subsequent siblings) 4 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 16:31 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano Simplify cmd_grep by splitting off the loop that finds matches in a list of trees. So now the main part of cmd_grep looks like: if (!use_index) { int hit = grep_directory(&opt, paths); if (use_threads) hit |= wait_all(); return !hit; } if (!list.nr) { if (!cached) setup_work_tree(); int hit = grep_cache(&opt, paths, cached); if (use_threads) hit |= wait_all; return !hit; } hit = grep_objects(&opt, path, &list); if (use_threads) hit |= wait_all(); return !hit; and is ripe for further refactoring. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- builtin/grep.c | 30 ++++++++++++++++++++---------- 1 files changed, 20 insertions(+), 10 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index b194ea3..5b8879f 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -675,6 +675,25 @@ static int grep_object(struct grep_opt *opt, const char **paths, die("unable to grep from object of type %s", typename(obj->type)); } +static int grep_objects(struct grep_opt *opt, const char **paths, + const struct object_array *list) +{ + unsigned int i; + int hit = 0; + const unsigned int nr = list->nr; + + for (i = 0; i < nr; i++) { + struct object *real_obj; + real_obj = deref_tag(list->objects[i].item, NULL, 0); + if (grep_object(opt, paths, real_obj, list->objects[i].name)) { + hit = 1; + if (opt->status_only) + break; + } + } + return hit; +} + static int grep_directory(struct grep_opt *opt, const char **paths) { struct dir_struct dir; @@ -1024,16 +1043,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (cached) die("both --cached and trees are given."); - - for (i = 0; i < list.nr; i++) { - struct object *real_obj; - real_obj = deref_tag(list.objects[i].item, NULL, 0); - if (grep_object(&opt, paths, real_obj, list.objects[i].name)) { - hit = 1; - if (opt.status_only) - break; - } - } + hit = grep_objects(&opt, paths, &list); if (use_threads) hit |= wait_all(); -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] Unify code paths of threaded greps 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder 2010-06-12 16:31 ` [PATCH 1/4] grep: refactor grep_objects loop into its own function Jonathan Nieder @ 2010-06-12 16:32 ` Jonathan Nieder 2010-06-12 16:36 ` [PATCH 3/4] grep: Add the option '--open-files-in-pager' Jonathan Nieder ` (2 subsequent siblings) 4 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 16:32 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano From: Johannes Schindelin <johannes.schindelin@gmx.de> There were three awfully similar code paths ending the threaded grep. It is better to avoid duplicated code, though. This change might very well prevent a race, where the grep patterns were free()d before waiting that all threads finished. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- builtin/grep.c | 22 +++++----------------- 1 files changed, 5 insertions(+), 17 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 5b8879f..2111212 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -590,7 +590,6 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached) if (hit && opt->status_only) break; } - free_grep_patterns(opt); return hit; } @@ -708,7 +707,6 @@ static int grep_directory(struct grep_opt *opt, const char **paths) if (hit && opt->status_only) break; } - free_grep_patterns(opt); return hit; } @@ -1019,32 +1017,22 @@ int cmd_grep(int argc, const char **argv, const char *prefix) } if (!use_index) { - int hit; if (cached) die("--cached cannot be used with --no-index."); if (list.nr) die("--no-index cannot be used with revs."); hit = grep_directory(&opt, paths); - if (use_threads) - hit |= wait_all(); - return !hit; - } - - if (!list.nr) { - int hit; + } else if (!list.nr) { if (!cached) setup_work_tree(); hit = grep_cache(&opt, paths, cached); - if (use_threads) - hit |= wait_all(); - return !hit; + } else { + if (cached) + die("both --cached and trees are given."); + hit = grep_objects(&opt, paths, &list); } - if (cached) - die("both --cached and trees are given."); - hit = grep_objects(&opt, paths, &list); - if (use_threads) hit |= wait_all(); free_grep_patterns(&opt); -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] grep: Add the option '--open-files-in-pager' 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder 2010-06-12 16:31 ` [PATCH 1/4] grep: refactor grep_objects loop into its own function Jonathan Nieder 2010-06-12 16:32 ` [PATCH 2/4] Unify code paths of threaded greps Jonathan Nieder @ 2010-06-12 16:36 ` Jonathan Nieder 2010-06-12 16:39 ` [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder 2010-06-12 21:36 ` [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin 4 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 16:36 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano, Paolo Bonzini From: Johannes Schindelin <johannes.schindelin@gmx.de> This adds an option to open the matching files in the pager, and if the pager happens to be "less" (or "vi") and there is only one grep pattern, it also jumps to the first match right away. The short option was chose as '-O' to avoid clashes with GNU grep's options (as suggested by Junio). So, 'git grep -O abc' is a short form for 'less +/abc $(grep -l abc)' except that it works also with spaces in file names, and it does not start the pager if there was no matching file. [jn: rebased and added tests; with error handling fix from Junio squashed in] Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Changes from previous version: - fix leaked argv - die on chdir failure - chmod +x t/t7811-grep-open.sh There was some discussion about changing the option’s name, but it might be better to instead split it into two options, along these lines: -O | --view-files --viewer= I have left it as is for now. Documentation/git-grep.txt | 8 ++ builtin/grep.c | 73 +++++++++++++++++ git.c | 2 +- t/lib-pager.sh | 15 ++++ t/t7006-pager.sh | 16 +--- t/{t7002-grep.sh => t7810-grep.sh} | 0 t/t7811-grep-open.sh | 154 ++++++++++++++++++++++++++++++++++++ 7 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 t/lib-pager.sh rename t/{t7002-grep.sh => t7810-grep.sh} (100%) create mode 100755 t/t7811-grep-open.sh diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 4b32322..8fdd8e1 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -14,6 +14,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] + [-O | --open-files-in-pager] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -104,6 +105,13 @@ OPTIONS For better compatibility with 'git diff', `--name-only` is a synonym for `--files-with-matches`. +-O:: +--open-files-in-pager:: + Open the matching files in the pager (not the output of 'grep'). + If the pager happens to be "less" or "vi", and the user + specified only one pattern, the first file is positioned at + the first match automatically. + -z:: --null:: Output \0 instead of the character that normally follows a diff --git a/builtin/grep.c b/builtin/grep.c index 2111212..1e8b946 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -11,6 +11,8 @@ #include "tree-walk.h" #include "builtin.h" #include "parse-options.h" +#include "string-list.h" +#include "run-command.h" #include "userdiff.h" #include "grep.h" #include "quote.h" @@ -556,6 +558,33 @@ static int grep_file(struct grep_opt *opt, const char *filename) } } +static void append_path(struct grep_opt *opt, const void *data, size_t len) +{ + struct string_list *path_list = opt->output_priv; + + if (len == 1 && *(const char *)data == '\0') + return; + string_list_append(xstrndup(data, len), path_list); +} + +static void run_pager(struct grep_opt *opt, const char *prefix) +{ + struct string_list *path_list = opt->output_priv; + const char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1)); + int i, status; + + for (i = 0; i < path_list->nr; i++) + argv[i] = path_list->items[i].string; + argv[path_list->nr] = NULL; + + if (prefix && chdir(prefix)) + die("Failed to chdir: %s", prefix); + status = run_command_v_opt(argv, RUN_USING_SHELL); + if (status) + exit(status); + free(argv); +} + static int grep_cache(struct grep_opt *opt, const char **paths, int cached) { int hit = 0; @@ -799,9 +828,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix) int cached = 0; int seen_dashdash = 0; int external_grep_allowed__ignored; + int show_in_pager = 0; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; const char **paths = NULL; + struct string_list path_list = { NULL, 0, 0, 0 }; int i; int dummy; int nongit = 0, use_index = 1; @@ -885,6 +916,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) OPT_BOOLEAN(0, "all-match", &opt.all_match, "show only matches from files that match all patterns"), OPT_GROUP(""), + OPT_BOOLEAN('O', "open-files-in-pager", &show_in_pager, + "show matching files in the pager"), OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored, "allow calling of grep(1) (ignored by this build)"), { OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage", @@ -960,6 +993,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix) argc--; } + if (show_in_pager) { + const char *pager = git_pager(1); + if (!pager) { + show_in_pager = 0; + } else { + opt.name_only = 1; + opt.null_following_name = 1; + opt.output_priv = &path_list; + opt.output = append_path; + string_list_append(pager, &path_list); + use_threads = 0; + } + } + if (!opt.pattern_list) die("no pattern given."); if (!opt.fixed && opt.ignore_case) @@ -1016,6 +1063,30 @@ int cmd_grep(int argc, const char **argv, const char *prefix) paths[1] = NULL; } + if (show_in_pager && (cached || list.nr)) + die("--open-files-in-pager only works on the worktree"); + + if (show_in_pager && opt.pattern_list && !opt.pattern_list->next) { + const char *pager = path_list.items[0].string; + int len = strlen(pager); + + if (len > 4 && is_dir_sep(pager[len - 5])) + pager += len - 4; + + if (!strcmp("less", pager) || !strcmp("vi", pager)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "+/%s%s", + strcmp("less", pager) ? "" : "*", + opt.pattern_list->pattern); + string_list_append(buf.buf, &path_list); + strbuf_detach(&buf, NULL); + } + } + + if (!show_in_pager) + setup_pager(); + + if (!use_index) { if (cached) die("--cached cannot be used with --no-index."); @@ -1035,6 +1106,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (use_threads) hit |= wait_all(); + if (hit && show_in_pager) + run_pager(&opt, prefix); free_grep_patterns(&opt); return !hit; } diff --git a/git.c b/git.c index 99f0363..265fa09 100644 --- a/git.c +++ b/git.c @@ -329,7 +329,7 @@ static void handle_internal_command(int argc, const char **argv) { "fsck-objects", cmd_fsck, RUN_SETUP }, { "gc", cmd_gc, RUN_SETUP }, { "get-tar-commit-id", cmd_get_tar_commit_id }, - { "grep", cmd_grep, USE_PAGER }, + { "grep", cmd_grep }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, { "index-pack", cmd_index_pack }, diff --git a/t/lib-pager.sh b/t/lib-pager.sh new file mode 100644 index 0000000..f8c6025 --- /dev/null +++ b/t/lib-pager.sh @@ -0,0 +1,15 @@ +#!/bin/sh + +test_expect_success 'determine default pager' ' + test_might_fail git config --unset core.pager && + less=$( + unset PAGER GIT_PAGER; + git var GIT_PAGER + ) && + test -n "$less" +' + +if expr "$less" : '^[a-z][a-z]*$' >/dev/null +then + test_set_prereq SIMPLEPAGER +fi diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index 3bc7a2a..fc993fc 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -3,6 +3,7 @@ test_description='Test automatic use of a pager.' . ./test-lib.sh +. "$TEST_DIRECTORY"/lib-pager.sh cleanup_fail() { echo >&2 cleanup failed @@ -158,21 +159,12 @@ test_expect_success 'color when writing to a file intended for a pager' ' colorful colorful.log ' -test_expect_success 'determine default pager' ' - unset PAGER GIT_PAGER && - test_might_fail git config --unset core.pager || - cleanup_fail && - - less=$(git var GIT_PAGER) && - test -n "$less" -' - -if expr "$less" : '^[a-z][a-z]*$' >/dev/null && test_have_prereq TTY +if test_have_prereq SIMPLEPAGER && test_have_prereq TTY then - test_set_prereq SIMPLEPAGER + test_set_prereq SIMPLEPAGERTTY fi -test_expect_success SIMPLEPAGER 'default pager is used by default' ' +test_expect_success SIMPLEPAGERTTY 'default pager is used by default' ' unset PAGER GIT_PAGER && test_might_fail git config --unset core.pager && rm -f default_pager_used || diff --git a/t/t7002-grep.sh b/t/t7810-grep.sh similarity index 100% rename from t/t7002-grep.sh rename to t/t7810-grep.sh diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh new file mode 100755 index 0000000..fcfc56e --- /dev/null +++ b/t/t7811-grep-open.sh @@ -0,0 +1,154 @@ +#!/bin/sh + +test_description='git grep --open-files-in-pager +' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-pager.sh +unset PAGER GIT_PAGER + +test_expect_success 'setup' ' + test_commit initial grep.h " +enum grep_pat_token { + GREP_PATTERN, + GREP_PATTERN_HEAD, + GREP_PATTERN_BODY, + GREP_AND, + GREP_OPEN_PAREN, + GREP_CLOSE_PAREN, + GREP_NOT, + GREP_OR, +};" && + + test_commit add-user revision.c " + } + if (seen_dashdash) + read_pathspec_from_stdin(revs, &sb, prune); + strbuf_release(&sb); +} + +static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what) +{ + append_grep_pattern(&revs->grep_filter, ptn, \"command line\", 0, what); +" && + + mkdir subdir && + test_commit subdir subdir/grep.c "enum grep_pat_token" && + + test_commit uninteresting unrelated "hello, world" && + + echo GREP_PATTERN >untracked +' + +test_expect_success SIMPLEPAGER 'git grep -O' ' + cat >$less <<-\EOF && + #!/bin/sh + printf "%s\n" "$@" >pager-args + EOF + chmod +x $less && + cat >expect.less <<-\EOF && + +/*GREP_PATTERN + grep.h + EOF + echo grep.h >expect.notless && + >empty && + + PATH=.:$PATH git grep -O GREP_PATTERN >out && + { + test_cmp expect.less pager-args || + test_cmp expect.notless pager-args + } && + test_cmp empty out +' + +test_expect_success 'git grep -O --cached' ' + test_must_fail git grep --cached -O GREP_PATTERN >out 2>msg && + grep open-files-in-pager msg +' + +test_expect_success 'git grep -O --no-index' ' + rm -f expect.less pager-args out && + cat >expect <<-\EOF && + grep.h + untracked + EOF + >empty && + + ( + GIT_PAGER='\''printf "%s\n" >pager-args'\'' && + export GIT_PAGER && + git grep --no-index -O GREP_PATTERN >out + ) && + test_cmp expect pager-args && + test_cmp empty out +' + +test_expect_success 'setup: fake "less"' ' + cat >less <<-\EOF + #!/bin/sh + printf "%s\n" "$@" >actual + EOF +' + +test_expect_success 'git grep -O jumps to line in less' ' + cat >expect <<-\EOF && + +/*GREP_PATTERN + grep.h + EOF + >empty && + + GIT_PAGER=./less git grep -O GREP_PATTERN >out && + test_cmp expect actual && + test_cmp empty out +' + +test_expect_success 'modified file' ' + rm -f actual && + cat >less <<-\EOF && + #!/bin/sh + printf "%s\n" "$@" >actual + EOF + chmod +x $less && + cat >expect <<-\EOF && + +/*enum grep_pat_token + grep.h + revision.c + subdir/grep.c + unrelated + EOF + >empty && + + echo "enum grep_pat_token" >unrelated && + test_when_finished "git checkout HEAD unrelated" && + GIT_PAGER=./less git grep -F -O "enum grep_pat_token" >out && + test_cmp expect actual && + test_cmp empty out +' + +test_expect_success 'run from subdir' ' + rm -f actual && + echo grep.c >expect && + >empty && + + ( + cd subdir && + export GIT_PAGER && + GIT_PAGER='\''printf "%s\n" >../args'\'' && + git grep -O "enum grep_pat_token" >../out && + GIT_PAGER="pwd >../dir; :" && + git grep -O "enum grep_pat_token" >../out2 + ) && + case $(cat dir) in + *subdir) + : good + ;; + *) + false + ;; + esac && + test_cmp expect args && + test_cmp empty out && + test_cmp empty out2 +' + +test_done -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder ` (2 preceding siblings ...) 2010-06-12 16:36 ` [PATCH 3/4] grep: Add the option '--open-files-in-pager' Jonathan Nieder @ 2010-06-12 16:39 ` Jonathan Nieder 2010-06-12 17:16 ` Paolo Bonzini 2010-06-13 16:35 ` Junio C Hamano 2010-06-12 21:36 ` [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin 4 siblings, 2 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 16:39 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano, Paolo Bonzini From: Johannes Schindelin <johannes.schindelin@gmx.de> Suppose you want to edit all files that contain a specific search term. Of course, you can do something totally trivial such as git grep -z -e <term> | xargs -0r vi +/<term> but maybe you are happy that the same will be achieved by git grep -Ovi <term> now. [jn: rebased and added tests] Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- As before. Thanks for reading. Documentation/git-grep.txt | 6 +++--- builtin/grep.c | 26 ++++++++++++-------------- t/t7811-grep-open.sh | 9 ++++++--- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index 8fdd8e1..d89ec32 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -14,7 +14,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] - [-O | --open-files-in-pager] + [(-O | --open-files-in-pager) [<pager>]] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -105,8 +105,8 @@ OPTIONS For better compatibility with 'git diff', `--name-only` is a synonym for `--files-with-matches`. --O:: ---open-files-in-pager:: +-O [<pager>]:: +--open-files-in-pager [<pager>]:: Open the matching files in the pager (not the output of 'grep'). If the pager happens to be "less" or "vi", and the user specified only one pattern, the first file is positioned at diff --git a/builtin/grep.c b/builtin/grep.c index 1e8b946..f32fbbc 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -828,7 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) int cached = 0; int seen_dashdash = 0; int external_grep_allowed__ignored; - int show_in_pager = 0; + const char *show_in_pager = NULL, *default_pager = "dummy"; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; const char **paths = NULL; @@ -916,8 +916,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) OPT_BOOLEAN(0, "all-match", &opt.all_match, "show only matches from files that match all patterns"), OPT_GROUP(""), - OPT_BOOLEAN('O', "open-files-in-pager", &show_in_pager, - "show matching files in the pager"), + { OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager, + "pager", "show matching files in the pager", + PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager }, OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored, "allow calling of grep(1) (ignored by this build)"), { OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage", @@ -993,18 +994,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix) argc--; } + if (show_in_pager == default_pager) + show_in_pager = git_pager(1); if (show_in_pager) { - const char *pager = git_pager(1); - if (!pager) { - show_in_pager = 0; - } else { - opt.name_only = 1; - opt.null_following_name = 1; - opt.output_priv = &path_list; - opt.output = append_path; - string_list_append(pager, &path_list); - use_threads = 0; - } + opt.name_only = 1; + opt.null_following_name = 1; + opt.output_priv = &path_list; + opt.output = append_path; + string_list_append(show_in_pager, &path_list); + use_threads = 0; } if (!opt.pattern_list) diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh index fcfc56e..8db4fc8 100755 --- a/t/t7811-grep-open.sh +++ b/t/t7811-grep-open.sh @@ -99,7 +99,11 @@ test_expect_success 'git grep -O jumps to line in less' ' GIT_PAGER=./less git grep -O GREP_PATTERN >out && test_cmp expect actual && - test_cmp empty out + test_cmp empty out && + + git grep -O./less GREP_PATTERN >out2 && + test_cmp expect actual && + test_cmp empty out2 ' test_expect_success 'modified file' ' @@ -135,8 +139,7 @@ test_expect_success 'run from subdir' ' export GIT_PAGER && GIT_PAGER='\''printf "%s\n" >../args'\'' && git grep -O "enum grep_pat_token" >../out && - GIT_PAGER="pwd >../dir; :" && - git grep -O "enum grep_pat_token" >../out2 + git grep -O"pwd >../dir; :" "enum grep_pat_token" >../out2 ) && case $(cat dir) in *subdir) -- 1.7.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) 2010-06-12 16:39 ` [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder @ 2010-06-12 17:16 ` Paolo Bonzini 2010-06-13 16:35 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Paolo Bonzini @ 2010-06-12 17:16 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Johannes Schindelin, git, Junio C Hamano On 06/12/2010 06:39 PM, Jonathan Nieder wrote: > From: Johannes Schindelin<johannes.schindelin@gmx.de> > > Suppose you want to edit all files that contain a specific search term. > Of course, you can do something totally trivial such as > > git grep -z -e<term> | xargs -0r vi +/<term> > > but maybe you are happy that the same will be achieved by > > git grep -Ovi<term> > > now. Ah, I had missed that the argument was optional. I think that optional arguments are slightly frowned upon for POSIX utilities, because they're a bit different. In fact, I think the only one is -i in sed (and BSD sed instead makes it mandatory...). Personally I have no problem with making -O in GNU grep optional too, I'll see what the other maintainers say. Acked-by: Paolo Bonzini <bonzini@gnu.org> Paolo ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) 2010-06-12 16:39 ` [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder 2010-06-12 17:16 ` Paolo Bonzini @ 2010-06-13 16:35 ` Junio C Hamano 2010-06-13 17:04 ` Paolo Bonzini 2010-06-14 6:31 ` Jonathan Nieder 1 sibling, 2 replies; 24+ messages in thread From: Junio C Hamano @ 2010-06-13 16:35 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Johannes Schindelin, git, Paolo Bonzini Jonathan Nieder <jrnieder@gmail.com> writes: > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > index 8fdd8e1..d89ec32 100644 > --- a/Documentation/git-grep.txt > +++ b/Documentation/git-grep.txt > @@ -14,7 +14,7 @@ SYNOPSIS > [-E | --extended-regexp] [-G | --basic-regexp] > [-F | --fixed-strings] [-n] > [-l | --files-with-matches] [-L | --files-without-match] > - [-O | --open-files-in-pager] > + [(-O | --open-files-in-pager) [<pager>]] > [-z | --null] > [-c | --count] [--all-match] [-q | --quiet] > [--max-depth <depth>] Hmm, does "git grep -e Heh -O frotz" look for Heh and show in the frotz pager, or does it look for Heh in paths under frotz/ directory and show hits in the default pager? > diff --git a/builtin/grep.c b/builtin/grep.c > index 1e8b946..f32fbbc 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -828,7 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) > int cached = 0; > int seen_dashdash = 0; > int external_grep_allowed__ignored; > - int show_in_pager = 0; > + const char *show_in_pager = NULL, *default_pager = "dummy"; If there were another instance of constant string "dummy" elsewhere in the program, is a clever compiler-linker combo allowed to optimize memory use by allocating one instance of such a string and pointing default_pager pointer to it? IOW, if the patch were: > + const char *show_in_pager = NULL, *default_pager = "dummy"; > + const char *another_dummy = "dummy"; could another_dummy and default_pager start out with the same value? This is just out of curiosity and does not affect correctness of the code, but I am wondering... ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) 2010-06-13 16:35 ` Junio C Hamano @ 2010-06-13 17:04 ` Paolo Bonzini 2010-06-14 6:31 ` Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: Paolo Bonzini @ 2010-06-13 17:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Johannes Schindelin, git >> - [-O | --open-files-in-pager] >> + [(-O | --open-files-in-pager) [<pager>]] > > Hmm, does "git grep -e Heh -O frotz" look for Heh and show in the frotz > pager, or does it look for Heh in paths under frotz/ directory and show > hits in the default pager? The latter. >> + const char *show_in_pager = NULL, *default_pager = "dummy"; > > If there were another instance of constant string "dummy" elsewhere in the > program, is a clever compiler-linker combo allowed to optimize memory use > by allocating one instance of such a string and pointing default_pager > pointer to it? IOW, if the patch were: > >> + const char *show_in_pager = NULL, *default_pager = "dummy"; >> + const char *another_dummy = "dummy"; > > could another_dummy and default_pager start out with the same value? In the same file, the compiler will do it already today. In another file, no (except if it does link-time optimization of course). Paolo ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) 2010-06-13 16:35 ` Junio C Hamano 2010-06-13 17:04 ` Paolo Bonzini @ 2010-06-14 6:31 ` Jonathan Nieder 1 sibling, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-14 6:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git, Paolo Bonzini Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: >> - [-O | --open-files-in-pager] >> + [(-O | --open-files-in-pager) [<pager>]] [...] > > Hmm, does "git grep -e Heh -O frotz" look for Heh and show in the frotz > pager, or does it look for Heh in paths under frotz/ directory and show > hits in the default pager? As Paolo mentioned, it is the latter. Patch for squashing is below. >> + const char *show_in_pager = NULL, *default_pager = "dummy"; > > If there were another instance of constant string "dummy" elsewhere in the > program, is a clever compiler-linker combo allowed to optimize memory use > by allocating one instance of such a string and pointing default_pager > pointer to it? For a moment, you had me worried: would an (insane) compiler be allowed to intern strings that appear as arguments, making argv[i] actually compare equal to default_pager? Luckily, the answer is no, because the strings pointed to in argv are guaranteed to be modifiable (see: Execution environments → Hosted environment → Program startup). -- 8< -- Subject: grep -O: do not advertize non-"sticked" form in documentation To avoid ambiguity in option parsing, the -O option will only take an argument if it is stuck to the option like -Ovi or --open-files-in-pager=vim. The un-sticked form -O vi means to search paths under 'vi' and show hits in the default pager. Noticed-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Documentation/git-grep.txt | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index d89ec32..0e12fe4 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -14,7 +14,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] - [(-O | --open-files-in-pager) [<pager>]] + [-O[<pager>] | --open-files-in-pager[=<pager>]] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -106,7 +106,7 @@ OPTIONS synonym for `--files-with-matches`. --O [<pager>]:: ---open-files-in-pager [<pager>]:: +-O[<pager>]:: +--open-files-in-pager=[<pager>]:: Open the matching files in the pager (not the output of 'grep'). If the pager happens to be "less" or "vi", and the user specified only one pattern, the first file is positioned at -- 1.7.1.246.g398e5.dirty ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder ` (3 preceding siblings ...) 2010-06-12 16:39 ` [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder @ 2010-06-12 21:36 ` Johannes Schindelin 2010-06-12 22:12 ` Jonathan Nieder 4 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2010-06-12 21:36 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Junio C Hamano Hi, On Sat, 12 Jun 2010, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > > Nope, the patch is still there, even after a rebase. > > Ah, I see. Looks like a good patch (d00d655, Unify code paths of > threaded greps, 2010-02-07) though in principle a separate topic, since > grep -O already sets use_threads = 0. No. It applies without the patch. But in my tests, it just does not work on multi-core machines without the patch. Ciao, Dscho ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-12 21:36 ` [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin @ 2010-06-12 22:12 ` Jonathan Nieder 0 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 22:12 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano Johannes Schindelin wrote: > But in my tests, it just does not work > on multi-core machines without the patch. Did you try the v2 series I sent earlier? I tested it on a machine with two multi-core CPUs. Now that I’ve read your patch, though, I don’t want to go back. The code duplication in the v2 series was ugly and did not leave the door open to easily adding threading support later, because it ran the pager _before_ waiting for pending threads. And besides, the "Unify code paths" patch contains an important fix for grep without -O. Thanks for both patches. Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-05 0:51 [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Jonathan Nieder ` (2 preceding siblings ...) 2010-06-05 16:11 ` [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin @ 2010-06-12 11:38 ` Paolo Bonzini 2010-06-12 14:50 ` Jonathan Nieder 3 siblings, 1 reply; 24+ messages in thread From: Paolo Bonzini @ 2010-06-12 11:38 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Johannes Schindelin, Junio C Hamano On 06/05/2010 02:51 AM, Jonathan Nieder wrote: > This series teaches ‘git grep’ to accept > > git grep -Ovi some_function_name > > to find all the call sites for some_function_name so they can be > adjusted. Dscho’s cover letter[1] explains it better. > > And in fact, Dscho did all the work here; this iteration just rebases it > on master and adds some tests. > > Happy hacking, > Jonathan > > Johannes Schindelin (2): > grep: Add the option '--open-files-in-pager' Just a nit, why not just "--open-files-in"? It is not necessarily a pager, as you cover letter shows. Of course the shorter version works, but I'm asking because I would like to add this option to GNU grep and I would definitely prefer to avoid the "pager" reference. Paolo ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] 2010-06-12 11:38 ` [PATCH v2 0/2] " Paolo Bonzini @ 2010-06-12 14:50 ` Jonathan Nieder 0 siblings, 0 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-06-12 14:50 UTC (permalink / raw) To: Paolo Bonzini; +Cc: git, Johannes Schindelin, Junio C Hamano Paolo Bonzini wrote: > Just a nit, why not just "--open-files-in"? I assume it was to convey the default. The effect of "git grep -e foo --open-files-in-pager" is more obvious than "git grep -e foo --open-files-in". But I do see your point. I have no preferences about what the long option is named. Care to prepare a follow-up patch? > I'm asking because I would like to add this option to GNU > grep That would be nice indeed. Thanks, Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] @ 2010-03-26 10:48 Johannes Schindelin 2010-03-26 10:49 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Johannes Schindelin 0 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2010-03-26 10:48 UTC (permalink / raw) To: git, gitster This supports opening the results of a 'git grep' directly in a pager (where the pager can be 'vi', too). This series is purely about convenience, everything the option does can be done with a regular script or command line. But I saw so many people doing their own scripts for that, and in many cases, they are subtly broken (e.g. git grep -z <expr> | xargs -0r vi +/<expr> would work as long as you do not have to check the exit status of git grep from another script) that I finally decided to go for it and send this patch pair. My most common use case for this is to do something like git grep -Ovi SomeJustRenamedFile to edit all files I might have forgotten to change after a git mv. (Actually, to be honest, my use case involves -Pvi, but I will have to retrain my hands.) Johannes Schindelin (2): grep: Add the option '--open-files-in-pager' grep -P: allow optional argument specifying the pager (or editor) Documentation/git-grep.txt | 8 +++++ builtin-grep.c | 74 ++++++++++++++++++++++++++++++++++++++++++++ git.c | 2 +- 3 files changed, 83 insertions(+), 1 deletions(-) ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-03-26 10:48 [PATCH " Johannes Schindelin @ 2010-03-26 10:49 ` Johannes Schindelin 2010-03-28 4:09 ` Jonathan Nieder 0 siblings, 1 reply; 24+ messages in thread From: Johannes Schindelin @ 2010-03-26 10:49 UTC (permalink / raw) To: git, gitster This adds an option to open the matching files in the pager, and if the pager happens to be "less" (or "vi") and there is only one grep pattern, it also jumps to the first match right away. The short option was chose as '-O' to avoid clashes with GNU grep's options (as suggested by Junio). So, 'git grep -O abc' is a short form for 'less +/abc $(grep -l abc)' except that it works also with spaces in file names, and it does not start the pager if there was no matching file. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/git-grep.txt | 8 +++++ builtin-grep.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ git.c | 2 +- 3 files changed, 80 insertions(+), 1 deletions(-) diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt index e019e76..ab3bb77 100644 --- a/Documentation/git-grep.txt +++ b/Documentation/git-grep.txt @@ -15,6 +15,7 @@ SYNOPSIS [-E | --extended-regexp] [-G | --basic-regexp] [-F | --fixed-strings] [-n] [-l | --files-with-matches] [-L | --files-without-match] + [-O | --open-files-in-pager] [-z | --null] [-c | --count] [--all-match] [-q | --quiet] [--max-depth <depth>] @@ -101,6 +102,13 @@ OPTIONS For better compatibility with 'git diff', --name-only is a synonym for --files-with-matches. +-O:: +--open-files-in-pager:: + Open the matching files in the pager (not the output of 'grep'). + If the pager happens to be "less" or "vi", and the user + specified only one pattern, the first file is positioned at + the first match automatically. + -z:: --null:: Output \0 instead of the character that normally follows a diff --git a/builtin-grep.c b/builtin-grep.c index 5d83d9b..233c772 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -14,6 +14,7 @@ #include "userdiff.h" #include "grep.h" #include "quote.h" +#include "string-list.h" #ifndef NO_PTHREADS #include "thread-utils.h" @@ -517,6 +518,31 @@ static int grep_file(struct grep_opt *opt, const char *filename) } } +static void append_path(struct grep_opt *opt, const void *data, size_t len) +{ + struct string_list *path_list = opt->output_priv; + + if (len == 1 && *(char *)data == '\0') + return; + string_list_append(xstrndup(data, len), path_list); +} + +static void run_pager(struct grep_opt *opt, const char *prefix) +{ + struct string_list *path_list = opt->output_priv; + char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1)); + int i; + + for (i = 0; i < path_list->nr; i++) + argv[i] = path_list->items[i].string; + argv[path_list->nr] = NULL; + + if (prefix) + chdir(prefix); + execvp(argv[0], argv); + error("Could not run pager %s: %s", argv[0], strerror(errno)); +} + static int grep_cache(struct grep_opt *opt, const char **paths, int cached) { int hit = 0; @@ -727,9 +753,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix) int cached = 0; int seen_dashdash = 0; int external_grep_allowed__ignored; + int show_in_pager = 0; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; const char **paths = NULL; + struct string_list path_list = { NULL, 0, 0, 0 }; int i; int dummy; struct option options[] = { @@ -810,6 +838,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) OPT_BOOLEAN(0, "all-match", &opt.all_match, "show only matches from files that match all patterns"), OPT_GROUP(""), + OPT_BOOLEAN('O', "open-files-in-pager", &show_in_pager, + "show matching files in the pager"), OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed__ignored, "allow calling of grep(1) (ignored by this build)"), { OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage", @@ -862,6 +892,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix) argc--; } + if (show_in_pager) { + const char *pager = getenv("GIT_PAGER"); + if (!pager) + pager = getenv("PAGER"); + if (!pager) + pager = "less"; + opt.name_only = 1; + opt.null_following_name = 1; + opt.output_priv = &path_list; + opt.output = append_path; + string_list_append(pager, &path_list); + use_threads = 0; + } + if (!opt.pattern_list) die("no pattern given."); if (!opt.fixed && opt.ignore_case) @@ -921,6 +965,29 @@ int cmd_grep(int argc, const char **argv, const char *prefix) paths[1] = NULL; } + if (show_in_pager && (cached || list.nr)) + die ("--open-files-in-pager only works on the worktree"); + + if (show_in_pager && opt.pattern_list && !opt.pattern_list->next) { + const char *pager = path_list.items[0].string; + int len = strlen(pager); + + if (len > 4 && is_dir_sep(pager[len - 5])) + pager += len - 4; + + if (!strcmp("less", pager) || !strcmp("vi", pager)) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "+/%s%s", + strcmp("less", pager) ? "" : "*", + opt.pattern_list->pattern); + string_list_append(buf.buf, &path_list); + strbuf_detach(&buf, NULL); + } + } + + if (!show_in_pager) + setup_pager(); + if (!list.nr) { if (!cached) setup_work_tree(); @@ -943,6 +1010,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (use_threads) hit |= wait_all(); + + if (hit && show_in_pager) + run_pager(&opt, prefix); + free_grep_patterns(&opt); return !hit; } diff --git a/git.c b/git.c index 4c3028c..4def718 100644 --- a/git.c +++ b/git.c @@ -317,7 +317,7 @@ static void handle_internal_command(int argc, const char **argv) { "fsck-objects", cmd_fsck, RUN_SETUP }, { "gc", cmd_gc, RUN_SETUP }, { "get-tar-commit-id", cmd_get_tar_commit_id }, - { "grep", cmd_grep, RUN_SETUP | USE_PAGER }, + { "grep", cmd_grep, RUN_SETUP }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, { "index-pack", cmd_index_pack }, -- 1.6.4.313.g3d9e3 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-03-26 10:49 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Johannes Schindelin @ 2010-03-28 4:09 ` Jonathan Nieder 2010-03-29 17:13 ` Johannes Schindelin 2010-03-29 18:17 ` Junio C Hamano 0 siblings, 2 replies; 24+ messages in thread From: Jonathan Nieder @ 2010-03-28 4:09 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, gitster Hi, Johannes Schindelin wrote: > This adds an option to open the matching files in the pager, and if the > pager happens to be "less" (or "vi") and there is only one grep pattern, > it also jumps to the first match right away. Sounds interesting. > diff --git a/builtin-grep.c b/builtin-grep.c > index 5d83d9b..233c772 100644 I couldn’t get this to apply to current master or maint, and searching for blob 5d83d9b to find a more suitable merge base revealed that I don’t actually have that object in my repo. Is this patch available already applied in a public git repo somewhere? Hopeful, Jonathan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-03-28 4:09 ` Jonathan Nieder @ 2010-03-29 17:13 ` Johannes Schindelin 2010-03-29 18:17 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Johannes Schindelin @ 2010-03-29 17:13 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, gitster [-- Attachment #1: Type: TEXT/PLAIN, Size: 516 bytes --] Hi, On Sat, 27 Mar 2010, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > > diff --git a/builtin-grep.c b/builtin-grep.c > > index 5d83d9b..233c772 100644 > > I couldn’t get this to apply to current master or maint, and searching > for blob 5d83d9b to find a more suitable merge base revealed that I > don’t actually have that object in my repo. Is this patch available > already applied in a public git repo somewhere? Yes, it is in 4msysgit.git's "grep-P" branch (still misnamed). Ciao, Dscho ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] grep: Add the option '--open-files-in-pager' 2010-03-28 4:09 ` Jonathan Nieder 2010-03-29 17:13 ` Johannes Schindelin @ 2010-03-29 18:17 ` Junio C Hamano 1 sibling, 0 replies; 24+ messages in thread From: Junio C Hamano @ 2010-03-29 18:17 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Johannes Schindelin, git Jonathan Nieder <jrnieder@gmail.com> writes: > I couldn’t get this to apply to current master or maint, and searching for > blob 5d83d9b to find a more suitable merge base revealed that I don’t > actually have that object in my repo. Is this patch available already > applied in a public git repo somewhere? This seems to be based on a fork with a not-very-recent forkpoint. I managed to create a topic branch and merged it to 'pu'. It probably wants to be adjusted to --no-index mode, as that is also about finding strings in files in the filesystem, but I chose not to do that myself. For this topic to go forward I think it needs a rebase to a more recent 'master'. ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2010-06-14 6:31 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-06-05 0:51 [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Jonathan Nieder 2010-06-05 0:53 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Jonathan Nieder 2010-06-05 1:40 ` Jonathan Nieder 2010-06-05 0:55 ` [PATCH 2/2] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder 2010-06-05 16:11 ` [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin 2010-06-12 7:55 ` Jonathan Nieder 2010-06-12 9:46 ` Johannes Schindelin 2010-06-12 16:29 ` [PATCH v3 0/4] " Jonathan Nieder 2010-06-12 16:31 ` [PATCH 1/4] grep: refactor grep_objects loop into its own function Jonathan Nieder 2010-06-12 16:32 ` [PATCH 2/4] Unify code paths of threaded greps Jonathan Nieder 2010-06-12 16:36 ` [PATCH 3/4] grep: Add the option '--open-files-in-pager' Jonathan Nieder 2010-06-12 16:39 ` [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor) Jonathan Nieder 2010-06-12 17:16 ` Paolo Bonzini 2010-06-13 16:35 ` Junio C Hamano 2010-06-13 17:04 ` Paolo Bonzini 2010-06-14 6:31 ` Jonathan Nieder 2010-06-12 21:36 ` [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>] Johannes Schindelin 2010-06-12 22:12 ` Jonathan Nieder 2010-06-12 11:38 ` [PATCH v2 0/2] " Paolo Bonzini 2010-06-12 14:50 ` Jonathan Nieder -- strict thread matches above, loose matches on Subject: below -- 2010-03-26 10:48 [PATCH " Johannes Schindelin 2010-03-26 10:49 ` [PATCH 1/2] grep: Add the option '--open-files-in-pager' Johannes Schindelin 2010-03-28 4:09 ` Jonathan Nieder 2010-03-29 17:13 ` Johannes Schindelin 2010-03-29 18:17 ` Junio C Hamano
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).