{"thread":{"id":"24013","subject":"[PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","startedAt":"2010-06-05T00:51:16Z","lastAt":"2010-06-14T06:31:25Z","messageCount":20,"participants":["Jonathan Nieder","Johannes Schindelin","Paolo Bonzini","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"143006","messageId":"20100605005116.GA8774@progeny.tock","threadId":"24013","inReplyTo":null,"subject":"[PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-05T00:51:16Z","receivedAt":"2010-06-05T00:51:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This series teaches ‘git grep’ to accept\n\n\tgit grep -Ovi some_function_name\n\nto find all the call sites for some_function_name so they can be\nadjusted.  Dscho’s cover letter[1] explains it better.\n\nAnd in fact, Dscho did all the work here; this iteration just rebases it\non master and adds some tests.\n\nHappy hacking,\nJonathan\n\nJohannes Schindelin (2):\n  grep: Add the option '--open-files-in-pager'\n  grep -O: allow optional argument specifying the pager (or editor)\n\n Documentation/git-grep.txt |    8 +\n builtin/grep.c             |   81 +++++++-\n git.c                      |    2 +-\n t/lib-pager.sh             |   15 ++\n t/t7002-grep.sh            |  530 --------------------------------------------\n t/t7006-pager.sh           |   16 +-\n t/t7810-grep.sh            |  530 ++++++++++++++++++++++++++++++++++++++++++++\n t/t7811-grep-open.sh       |  157 +++++++++++++\n 8 files changed, 792 insertions(+), 547 deletions(-)\n create mode 100644 t/lib-pager.sh\n delete mode 100755 t/t7002-grep.sh\n create mode 100755 t/t7810-grep.sh\n create mode 100644 t/t7811-grep-open.sh\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/143219\n"},{"id":"143007","messageId":"20100605005322.GB8774@progeny.tock","threadId":"24013","inReplyTo":"20100605005116.GA8774@progeny.tock","subject":"[PATCH 1/2] grep: Add the option '--open-files-in-pager'","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-05T00:53:22Z","receivedAt":"2010-06-05T00:53:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis adds an option to open the matching files in the pager, and if the\npager happens to be \"less\" (or \"vi\") and there is only one grep pattern,\nit also jumps to the first match right away.\n\nThe short option was chose as '-O' to avoid clashes with GNU grep's\noptions (as suggested by Junio).\n\nSo, 'git grep -O abc' is a short form for 'less +/abc $(grep -l abc)'\nexcept that it works also with spaces in file names, and it does not\nstart the pager if there was no matching file.\n\n[jn: rebased and added tests; with error handling fix from Junio\nsquashed in]\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/git-grep.txt         |    8 ++\n builtin/grep.c                     |   83 ++++++++++++++++++-\n git.c                              |    2 +-\n t/lib-pager.sh                     |   15 ++++\n t/t7006-pager.sh                   |   16 +---\n t/{t7002-grep.sh => t7810-grep.sh} |    0\n t/t7811-grep-open.sh               |  154 ++++++++++++++++++++++++++++++++++++\n 7 files changed, 261 insertions(+), 17 deletions(-)\n create mode 100644 t/lib-pager.sh\n rename t/{t7002-grep.sh => t7810-grep.sh} (100%)\n create mode 100644 t/t7811-grep-open.sh\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4b32322..8fdd8e1 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -14,6 +14,7 @@ SYNOPSIS\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-F | --fixed-strings] [-n]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n+\t   [-O | --open-files-in-pager]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match] [-q | --quiet]\n \t   [--max-depth <depth>]\n@@ -104,6 +105,13 @@ OPTIONS\n \tFor better compatibility with 'git diff', `--name-only` is a\n \tsynonym for `--files-with-matches`.\n \n+-O::\n+--open-files-in-pager::\n+\tOpen the matching files in the pager (not the output of 'grep').\n+\tIf the pager happens to be \"less\" or \"vi\", and the user\n+\tspecified only one pattern, the first file is positioned at\n+\tthe first match automatically.\n+\n -z::\n --null::\n \tOutput \\0 instead of the character that normally follows a\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex b194ea3..bc6c2ea 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -11,6 +11,8 @@\n #include \"tree-walk.h\"\n #include \"builtin.h\"\n #include \"parse-options.h\"\n+#include \"string-list.h\"\n+#include \"run-command.h\"\n #include \"userdiff.h\"\n #include \"grep.h\"\n #include \"quote.h\"\n@@ -556,7 +558,34 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \t}\n }\n \n-static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n+static void append_path(struct grep_opt *opt, const void *data, size_t len)\n+{\n+\tstruct string_list *path_list = opt->output_priv;\n+\n+\tif (len == 1 && *(const char *)data == '\\0')\n+\t\treturn;\n+\tstring_list_append(xstrndup(data, len), path_list);\n+}\n+\n+static void run_pager(struct grep_opt *opt, const char *prefix)\n+{\n+\tstruct string_list *path_list = opt->output_priv;\n+\tconst char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1));\n+\tint i, status;\n+\n+\tfor (i = 0; i < path_list->nr; i++)\n+\t\targv[i] = path_list->items[i].string;\n+\targv[path_list->nr] = NULL;\n+\n+\tif (prefix && chdir(prefix))\n+\t\terror(\"Failed to chdir: %s\", prefix);\n+\tstatus = run_command_v_opt(argv, RUN_USING_SHELL);\n+\tif (status)\n+\t\texit(status);\n+}\n+\n+static int grep_cache(struct grep_opt *opt, const char **paths,\n+\t\t      int cached, int show_in_pager, const char *prefix)\n {\n \tint hit = 0;\n \tint nr;\n@@ -590,6 +619,8 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \t\tif (hit && opt->status_only)\n \t\t\tbreak;\n \t}\n+\tif (hit && show_in_pager)\n+\t\trun_pager(opt, prefix);\n \tfree_grep_patterns(opt);\n \treturn hit;\n }\n@@ -675,7 +706,8 @@ static int grep_object(struct grep_opt *opt, const char **paths,\n \tdie(\"unable to grep from object of type %s\", typename(obj->type));\n }\n \n-static int grep_directory(struct grep_opt *opt, const char **paths)\n+static int grep_directory(struct grep_opt *opt, const char **paths,\n+\t\t\t  int show_in_pager, const char *prefix)\n {\n \tstruct dir_struct dir;\n \tint i, hit = 0;\n@@ -689,6 +721,8 @@ static int grep_directory(struct grep_opt *opt, const char **paths)\n \t\tif (hit && opt->status_only)\n \t\t\tbreak;\n \t}\n+\tif (hit && show_in_pager)\n+\t\trun_pager(opt, prefix);\n \tfree_grep_patterns(opt);\n \treturn hit;\n }\n@@ -782,9 +816,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint cached = 0;\n \tint seen_dashdash = 0;\n \tint external_grep_allowed__ignored;\n+\tint show_in_pager = 0;\n \tstruct grep_opt opt;\n \tstruct object_array list = { 0, 0, NULL };\n \tconst char **paths = NULL;\n+\tstruct string_list path_list = { NULL, 0, 0, 0 };\n \tint i;\n \tint dummy;\n \tint nongit = 0, use_index = 1;\n@@ -868,6 +904,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN(0, \"all-match\", &opt.all_match,\n \t\t\t\"show only matches from files that match all patterns\"),\n \t\tOPT_GROUP(\"\"),\n+\t\tOPT_BOOLEAN('O', \"open-files-in-pager\", &show_in_pager,\n+\t\t\t\"show matching files in the pager\"),\n \t\tOPT_BOOLEAN(0, \"ext-grep\", &external_grep_allowed__ignored,\n \t\t\t    \"allow calling of grep(1) (ignored by this build)\"),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n@@ -943,6 +981,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\targc--;\n \t}\n \n+\tif (show_in_pager) {\n+\t\tconst char *pager = git_pager(1);\n+\t\tif (!pager) {\n+\t\t\tshow_in_pager = 0;\n+\t\t} else {\n+\t\t\topt.name_only = 1;\n+\t\t\topt.null_following_name = 1;\n+\t\t\topt.output_priv = &path_list;\n+\t\t\topt.output = append_path;\n+\t\t\tstring_list_append(pager, &path_list);\n+\t\t\tuse_threads = 0;\n+\t\t}\n+\t}\n+\n \tif (!opt.pattern_list)\n \t\tdie(\"no pattern given.\");\n \tif (!opt.fixed && opt.ignore_case)\n@@ -999,13 +1051,36 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tpaths[1] = NULL;\n \t}\n \n+\tif (show_in_pager && (cached || list.nr))\n+\t\tdie(\"--open-files-in-pager only works on the worktree\");\n+\n+\tif (show_in_pager && opt.pattern_list && !opt.pattern_list->next) {\n+\t\tconst char *pager = path_list.items[0].string;\n+\t\tint len = strlen(pager);\n+\n+\t\tif (len > 4 && is_dir_sep(pager[len - 5]))\n+\t\t\tpager += len - 4;\n+\n+\t\tif (!strcmp(\"less\", pager) || !strcmp(\"vi\", pager)) {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&buf, \"+/%s%s\",\n+\t\t\t\t\tstrcmp(\"less\", pager) ? \"\" : \"*\",\n+\t\t\t\t\topt.pattern_list->pattern);\n+\t\t\tstring_list_append(buf.buf, &path_list);\n+\t\t\tstrbuf_detach(&buf, NULL);\n+\t\t}\n+\t}\n+\n+\tif (!show_in_pager)\n+\t\tsetup_pager();\n+\n \tif (!use_index) {\n \t\tint hit;\n \t\tif (cached)\n \t\t\tdie(\"--cached cannot be used with --no-index.\");\n \t\tif (list.nr)\n \t\t\tdie(\"--no-index cannot be used with revs.\");\n-\t\thit = grep_directory(&opt, paths);\n+\t\thit = grep_directory(&opt, paths, show_in_pager, prefix);\n \t\tif (use_threads)\n \t\t\thit |= wait_all();\n \t\treturn !hit;\n@@ -1016,7 +1091,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tif (!cached)\n \t\t\tsetup_work_tree();\n \n-\t\thit = grep_cache(&opt, paths, cached);\n+\t\thit = grep_cache(&opt, paths, cached, show_in_pager, prefix);\n \t\tif (use_threads)\n \t\t\thit |= wait_all();\n \t\treturn !hit;\ndiff --git a/git.c b/git.c\nindex 99f0363..265fa09 100644\n--- a/git.c\n+++ b/git.c\n@@ -329,7 +329,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"fsck-objects\", cmd_fsck, RUN_SETUP },\n \t\t{ \"gc\", cmd_gc, RUN_SETUP },\n \t\t{ \"get-tar-commit-id\", cmd_get_tar_commit_id },\n-\t\t{ \"grep\", cmd_grep, USE_PAGER },\n+\t\t{ \"grep\", cmd_grep },\n \t\t{ \"hash-object\", cmd_hash_object },\n \t\t{ \"help\", cmd_help },\n \t\t{ \"index-pack\", cmd_index_pack },\ndiff --git a/t/lib-pager.sh b/t/lib-pager.sh\nnew file mode 100644\nindex 0000000..f8c6025\n--- /dev/null\n+++ b/t/lib-pager.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_expect_success 'determine default pager' '\n+\ttest_might_fail git config --unset core.pager &&\n+\tless=$(\n+\t\tunset PAGER GIT_PAGER;\n+\t\tgit var GIT_PAGER\n+\t) &&\n+\ttest -n \"$less\"\n+'\n+\n+if expr \"$less\" : '^[a-z][a-z]*$' >/dev/null\n+then\n+\ttest_set_prereq SIMPLEPAGER\n+fi\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 3bc7a2a..fc993fc 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -3,6 +3,7 @@\n test_description='Test automatic use of a pager.'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-pager.sh\n \n cleanup_fail() {\n \techo >&2 cleanup failed\n@@ -158,21 +159,12 @@ test_expect_success 'color when writing to a file intended for a pager' '\n \tcolorful colorful.log\n '\n \n-test_expect_success 'determine default pager' '\n-\tunset PAGER GIT_PAGER &&\n-\ttest_might_fail git config --unset core.pager ||\n-\tcleanup_fail &&\n-\n-\tless=$(git var GIT_PAGER) &&\n-\ttest -n \"$less\"\n-'\n-\n-if expr \"$less\" : '^[a-z][a-z]*$' >/dev/null && test_have_prereq TTY\n+if test_have_prereq SIMPLEPAGER && test_have_prereq TTY\n then\n-\ttest_set_prereq SIMPLEPAGER\n+\ttest_set_prereq SIMPLEPAGERTTY\n fi\n \n-test_expect_success SIMPLEPAGER 'default pager is used by default' '\n+test_expect_success SIMPLEPAGERTTY 'default pager is used by default' '\n \tunset PAGER GIT_PAGER &&\n \ttest_might_fail git config --unset core.pager &&\n \trm -f default_pager_used ||\ndiff --git a/t/t7002-grep.sh b/t/t7810-grep.sh\nsimilarity index 100%\nrename from t/t7002-grep.sh\nrename to t/t7810-grep.sh\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nnew file mode 100644\nindex 0000000..72e4023\n--- /dev/null\n+++ b/t/t7811-grep-open.sh\n@@ -0,0 +1,154 @@\n+#!/bin/sh\n+\n+test_description='git grep --open-files-in-pager\n+'\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-pager.sh\n+unset PAGER GIT_PAGER\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial grep.h \"\n+enum grep_pat_token {\n+\tGREP_PATTERN,\n+\tGREP_PATTERN_HEAD,\n+\tGREP_PATTERN_BODY,\n+\tGREP_AND,\n+\tGREP_OPEN_PAREN,\n+\tGREP_CLOSE_PAREN,\n+\tGREP_NOT,\n+\tGREP_OR,\n+};\" &&\n+\n+\ttest_commit add-user revision.c \"\n+\t}\n+\tif (seen_dashdash)\n+\t\tread_pathspec_from_stdin(revs, &sb, prune);\n+\tstrbuf_release(&sb);\n+}\n+\n+static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n+{\n+\tappend_grep_pattern(&revs->grep_filter, ptn, \\\"command line\\\", 0, what);\n+\" &&\n+\n+\tmkdir subdir &&\n+\ttest_commit subdir subdir/grep.c \"enum grep_pat_token\" &&\n+\n+\ttest_commit uninteresting unrelated \"hello, world\" &&\n+\n+\techo GREP_PATTERN >untracked\n+'\n+\n+test_expect_success SIMPLEPAGER 'git grep -O' '\n+\tcat >$less <<-\\EOF &&\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >pager-args\n+\tEOF\n+\tchmod +x $less &&\n+\tcat >expect.less <<-\\EOF &&\n+\t+/*GREP_PATTERN\n+\tgrep.h\n+\tEOF\n+\techo grep.h >expect.notless &&\n+\t>empty &&\n+\n+\tPATH=.:$PATH git grep -O GREP_PATTERN >out &&\n+\t{\n+\t\ttest_cmp expect.less pager-args ||\n+\t\ttest_cmp expect.notless pager-args\n+\t} &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'git grep -O --cached' '\n+\ttest_must_fail git grep --cached -O GREP_PATTERN >out 2>msg &&\n+\tgrep open-files-in-pager msg\n+'\n+\n+test_expect_success 'git grep -O --no-index' '\n+\trm -f expect.less pager-args out &&\n+\tcat >expect <<-\\EOF &&\n+\tgrep.h\n+\tuntracked\n+\tEOF\n+\t>empty &&\n+\n+\t(\n+\t\tGIT_PAGER='\\''printf \"%s\\n\" >pager-args'\\'' &&\n+\t\texport GIT_PAGER &&\n+\t\tgit grep --no-index -O GREP_PATTERN >out\n+\t) &&\n+\ttest_cmp expect pager-args &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'setup: fake \"less\"' '\n+\tcat >less <<-\\EOF\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >actual\n+\tEOF\n+'\n+\n+test_expect_success 'git grep -O jumps to line in less' '\n+\tcat >expect <<-\\EOF &&\n+\t+/*GREP_PATTERN\n+\tgrep.h\n+\tEOF\n+\t>empty &&\n+\n+\tGIT_PAGER=./less git grep -O GREP_PATTERN >out\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'modified file' '\n+\trm -f actual &&\n+\tcat >less <<-\\EOF &&\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >actual\n+\tEOF\n+\tchmod +x $less &&\n+\tcat >expect <<-\\EOF &&\n+\t+/*enum grep_pat_token\n+\tgrep.h\n+\trevision.c\n+\tsubdir/grep.c\n+\tunrelated\n+\tEOF\n+\t>empty &&\n+\n+\techo \"enum grep_pat_token\" >unrelated &&\n+\ttest_when_finished \"git checkout HEAD unrelated\" &&\n+\tGIT_PAGER=./less git grep -F -O \"enum grep_pat_token\" >out &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'run from subdir' '\n+\trm -f actual &&\n+\techo grep.c >expect &&\n+\t>empty &&\n+\n+\t(\n+\t\tcd subdir &&\n+\t\texport GIT_PAGER &&\n+\t\tGIT_PAGER='\\''printf \"%s\\n\" >../args'\\'' &&\n+\t\tgit grep -O \"enum grep_pat_token\" >../out\n+\t\tGIT_PAGER=\"pwd >../dir; :\" &&\n+\t\tgit grep -O \"enum grep_pat_token\" >../out2\n+\t) &&\n+\tcase $(cat dir) in\n+\t*subdir)\n+\t\t: good\n+\t\t;;\n+\t*)\n+\t\tfalse\n+\t\t;;\n+\tesac &&\n+\ttest_cmp expect args &&\n+\ttest_cmp empty out &&\n+\ttest_cmp empty out2\n+'\n+\n+test_done\n-- \n1.7.1\n"},{"id":"143008","messageId":"20100605005552.GC8774@progeny.tock","threadId":"24013","inReplyTo":"20100605005116.GA8774@progeny.tock","subject":"[PATCH 2/2] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-05T00:55:52Z","receivedAt":"2010-06-05T00:55:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSuppose you want to edit all files that contain a specific search term.\nOf course, you can do something totally trivial such as\n\n\tgit grep -z -e <term> | xargs -0r vi +/<term>\n\nbut maybe you are happy that the same will be achieved by\n\n\tgit grep -Ovi <term>\n\nnow.\n\n[jn: rebased and added tests]\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThings I didn’t think hard about:\n\n - the exit status\n - the special-casing of \"less\" and \"vi\"\n\nMaybe the latter could be improved by also accepting \"more\" and\n\"vim\", which both will accept the +/<term> option.\n\nThanks for reading.\n\n Documentation/git-grep.txt |    6 +++---\n builtin/grep.c             |   32 +++++++++++++++-----------------\n t/t7811-grep-open.sh       |   11 +++++++----\n 3 files changed, 25 insertions(+), 24 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 8fdd8e1..d89ec32 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-F | --fixed-strings] [-n]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n-\t   [-O | --open-files-in-pager]\n+\t   [(-O | --open-files-in-pager) [<pager>]]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match] [-q | --quiet]\n \t   [--max-depth <depth>]\n@@ -105,8 +105,8 @@ OPTIONS\n \tFor better compatibility with 'git diff', `--name-only` is a\n \tsynonym for `--files-with-matches`.\n \n--O::\n---open-files-in-pager::\n+-O [<pager>]::\n+--open-files-in-pager [<pager>]::\n \tOpen the matching files in the pager (not the output of 'grep').\n \tIf the pager happens to be \"less\" or \"vi\", and the user\n \tspecified only one pattern, the first file is positioned at\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex bc6c2ea..f9551a7 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -584,8 +584,8 @@ static void run_pager(struct grep_opt *opt, const char *prefix)\n \t\texit(status);\n }\n \n-static int grep_cache(struct grep_opt *opt, const char **paths,\n-\t\t      int cached, int show_in_pager, const char *prefix)\n+static int grep_cache(struct grep_opt *opt, const char **paths, int cached,\n+\t\t      const char *show_in_pager, const char *prefix)\n {\n \tint hit = 0;\n \tint nr;\n@@ -707,7 +707,7 @@ static int grep_object(struct grep_opt *opt, const char **paths,\n }\n \n static int grep_directory(struct grep_opt *opt, const char **paths,\n-\t\t\t  int show_in_pager, const char *prefix)\n+\t\t\t  const char *show_in_pager, const char *prefix)\n {\n \tstruct dir_struct dir;\n \tint i, hit = 0;\n@@ -816,7 +816,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint cached = 0;\n \tint seen_dashdash = 0;\n \tint external_grep_allowed__ignored;\n-\tint show_in_pager = 0;\n+\tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n \tstruct grep_opt opt;\n \tstruct object_array list = { 0, 0, NULL };\n \tconst char **paths = NULL;\n@@ -904,8 +904,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN(0, \"all-match\", &opt.all_match,\n \t\t\t\"show only matches from files that match all patterns\"),\n \t\tOPT_GROUP(\"\"),\n-\t\tOPT_BOOLEAN('O', \"open-files-in-pager\", &show_in_pager,\n-\t\t\t\"show matching files in the pager\"),\n+\t\t{ OPTION_STRING, 'O', \"open-files-in-pager\", &show_in_pager,\n+\t\t\t\"pager\", \"show matching files in the pager\",\n+\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },\n \t\tOPT_BOOLEAN(0, \"ext-grep\", &external_grep_allowed__ignored,\n \t\t\t    \"allow calling of grep(1) (ignored by this build)\"),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n@@ -981,18 +982,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\targc--;\n \t}\n \n+\tif (show_in_pager == default_pager)\n+\t\tshow_in_pager = git_pager(1);\n \tif (show_in_pager) {\n-\t\tconst char *pager = git_pager(1);\n-\t\tif (!pager) {\n-\t\t\tshow_in_pager = 0;\n-\t\t} else {\n-\t\t\topt.name_only = 1;\n-\t\t\topt.null_following_name = 1;\n-\t\t\topt.output_priv = &path_list;\n-\t\t\topt.output = append_path;\n-\t\t\tstring_list_append(pager, &path_list);\n-\t\t\tuse_threads = 0;\n-\t\t}\n+\t\topt.name_only = 1;\n+\t\topt.null_following_name = 1;\n+\t\topt.output_priv = &path_list;\n+\t\topt.output = append_path;\n+\t\tstring_list_append(show_in_pager, &path_list);\n+\t\tuse_threads = 0;\n \t}\n \n \tif (!opt.pattern_list)\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nindex 72e4023..fdf45b8 100644\n--- a/t/t7811-grep-open.sh\n+++ b/t/t7811-grep-open.sh\n@@ -97,9 +97,13 @@ test_expect_success 'git grep -O jumps to line in less' '\n \tEOF\n \t>empty &&\n \n-\tGIT_PAGER=./less git grep -O GREP_PATTERN >out\n+\tGIT_PAGER=./less git grep -O GREP_PATTERN >out1\n \ttest_cmp expect actual &&\n-\ttest_cmp empty out\n+\ttest_cmp empty out1 &&\n+\n+\tgit grep -O./less GREP_PATTERN >out2 &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out2\n '\n \n test_expect_success 'modified file' '\n@@ -135,8 +139,7 @@ test_expect_success 'run from subdir' '\n \t\texport GIT_PAGER &&\n \t\tGIT_PAGER='\\''printf \"%s\\n\" >../args'\\'' &&\n \t\tgit grep -O \"enum grep_pat_token\" >../out\n-\t\tGIT_PAGER=\"pwd >../dir; :\" &&\n-\t\tgit grep -O \"enum grep_pat_token\" >../out2\n+\t\tgit grep -O\"pwd >../dir; :\" \"enum grep_pat_token\" >../out2\n \t) &&\n \tcase $(cat dir) in\n \t*subdir)\n-- \n1.7.1\n"},{"id":"143009","messageId":"20100605014021.GA1778@progeny.tock","threadId":"24013","inReplyTo":"20100605005322.GB8774@progeny.tock","subject":"Re: [PATCH 1/2] grep: Add the option '--open-files-in-pager'","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-05T01:40:21Z","receivedAt":"2010-06-05T01:40:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> +test_expect_success 'git grep -O jumps to line in less' '\n> +\tcat >expect <<-\\EOF &&\n> +\t+/*GREP_PATTERN\n> +\tgrep.h\n> +\tEOF\n> +\t>empty &&\n> +\n> +\tGIT_PAGER=./less git grep -O GREP_PATTERN >out\n> +\ttest_cmp expect actual &&\n\nUgh.  We need a static analyzer for shell scripts. :)  Short of that,\nhere’s a fixup to squash in.\n\nSorry for the trouble.\n\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nindex 72e4023..fcfc56e 100644\n--- a/t/t7811-grep-open.sh\n+++ b/t/t7811-grep-open.sh\n@@ -97,7 +97,7 @@ test_expect_success 'git grep -O jumps to line in less' '\n \tEOF\n \t>empty &&\n \n-\tGIT_PAGER=./less git grep -O GREP_PATTERN >out\n+\tGIT_PAGER=./less git grep -O GREP_PATTERN >out &&\n \ttest_cmp expect actual &&\n \ttest_cmp empty out\n '\n@@ -134,7 +134,7 @@ test_expect_success 'run from subdir' '\n \t\tcd subdir &&\n \t\texport GIT_PAGER &&\n \t\tGIT_PAGER='\\''printf \"%s\\n\" >../args'\\'' &&\n-\t\tgit grep -O \"enum grep_pat_token\" >../out\n+\t\tgit grep -O \"enum grep_pat_token\" >../out &&\n \t\tGIT_PAGER=\"pwd >../dir; :\" &&\n \t\tgit grep -O \"enum grep_pat_token\" >../out2\n \t) &&\n-- \n"},{"id":"143041","messageId":"alpine.DEB.1.00.1006051810300.1732@bonsai2","threadId":"24013","inReplyTo":"20100605005116.GA8774@progeny.tock","subject":"Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-06-05T16:11:40Z","receivedAt":"2010-06-05T16:11:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 4 Jun 2010, Jonathan Nieder wrote:\n\n> This series teaches ‘git grep’ to accept\n> \n> \tgit grep -Ovi some_function_name\n> \n> to find all the call sites for some_function_name so they can be\n> adjusted.  Dscho’s cover letter[1] explains it better.\n> \n> And in fact, Dscho did all the work here; this iteration just rebases it\n> on master and adds some tests.\n> \n> Happy hacking,\n> Jonathan\n> \n> Johannes Schindelin (2):\n>   grep: Add the option '--open-files-in-pager'\n>   grep -O: allow optional argument specifying the pager (or editor)\n\nLast time I tried, it also needed the patch \"Unify code paths of threaded \ngreps\". Don't know if that one made it in already.\n\nCiao,\nDscho"},{"id":"143556","messageId":"20100612075542.GB2918@burratino","threadId":"24013","inReplyTo":"alpine.DEB.1.00.1006051810300.1732@bonsai2","subject":"Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T07:55:42Z","receivedAt":"2010-06-12T07:55:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Schindelin wrote:\n> On Fri, 4 Jun 2010, Jonathan Nieder wrote:\n\n>> Johannes Schindelin (2):\n>>   grep: Add the option '--open-files-in-pager'\n>>   grep -O: allow optional argument specifying the pager (or editor)\n>\n> Last time I tried, it also needed the patch \"Unify code paths of threaded \n> greps\". Don't know if that one made it in already.\n\nThanks for the pointer, but I can’t seem to find any such patch.  Maybe\nit was squashed into Fredrik’s patch (5b594f4)?\n\nJonathan\n"},{"id":"143559","messageId":"alpine.DEB.1.00.1006121139290.2689@bonsai2","threadId":"24013","inReplyTo":"20100612075542.GB2918@burratino","subject":"Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-06-12T09:46:38Z","receivedAt":"2010-06-12T09:46:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 12 Jun 2010, Jonathan Nieder wrote:\n\n> Johannes Schindelin wrote:\n> > On Fri, 4 Jun 2010, Jonathan Nieder wrote:\n> \n> >> Johannes Schindelin (2):\n> >>   grep: Add the option '--open-files-in-pager'\n> >>   grep -O: allow optional argument specifying the pager (or editor)\n> >\n> > Last time I tried, it also needed the patch \"Unify code paths of threaded \n> > greps\". Don't know if that one made it in already.\n> \n> Thanks for the pointer, but I can’t seem to find any such patch.  Maybe\n> it was squashed into Fredrik’s patch (5b594f4)?\n\nNope, the patch is still there, even after a rebase.\n\nIn any case, the stuff is available in 4msysgit.git, and working (I was \nsick and tired of maintaining two different forks, and it was costing me \ntoo much time and work anyway, so I made 4msysgit.git's devel my main \nbranch).\n\nThanks,\nJohannes\n"},{"id":"143565","messageId":"4C1371CE.8010309@gnu.org","threadId":"24013","inReplyTo":"20100605005116.GA8774@progeny.tock","subject":"Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2010-06-12T11:38:54Z","receivedAt":"2010-06-12T11:38:54Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 06/05/2010 02:51 AM, Jonathan Nieder wrote:\n> This series teaches ‘git grep’ to accept\n>\n> \tgit grep -Ovi some_function_name\n>\n> to find all the call sites for some_function_name so they can be\n> adjusted.  Dscho’s cover letter[1] explains it better.\n>\n> And in fact, Dscho did all the work here; this iteration just rebases it\n> on master and adds some tests.\n>\n> Happy hacking,\n> Jonathan\n>\n> Johannes Schindelin (2):\n>    grep: Add the option '--open-files-in-pager'\n\nJust a nit, why not just \"--open-files-in\"?  It is not necessarily a \npager, as you cover letter shows.  Of course the shorter version works, \nbut I'm asking because I would like to add this option to GNU grep and I \nwould definitely prefer to avoid the \"pager\" reference.\n\nPaolo\n"},{"id":"143569","messageId":"20100612145021.GA1406@burratino","threadId":"24013","inReplyTo":"4C1371CE.8010309@gnu.org","subject":"Re: [PATCH v2 0/2] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T14:50:21Z","receivedAt":"2010-06-12T14:50:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Paolo Bonzini wrote:\n\n> Just a nit, why not just \"--open-files-in\"?\n\nI assume it was to convey the default.  The effect of\n\"git grep -e foo --open-files-in-pager\" is more obvious than\n\"git grep -e foo --open-files-in\".\n\nBut I do see your point.  I have no preferences about what the long\noption is named.  Care to prepare a follow-up patch?\n\n> I'm asking because I would like to add this option to GNU\n> grep\n\nThat would be nice indeed.\n\nThanks,\nJonathan\n"},{"id":"143576","messageId":"20100612162945.GB1406@burratino","threadId":"24013","inReplyTo":"alpine.DEB.1.00.1006121139290.2689@bonsai2","subject":"[PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T16:29:45Z","receivedAt":"2010-06-12T16:29:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Schindelin wrote:\n\n> Nope, the patch is still there, even after a rebase.\n\nAh, I see.  Looks like a good patch (d00d655, Unify code paths of threaded\ngreps, 2010-02-07) though in principle a separate topic, since grep -O\nalready sets use_threads = 0.  I think it is good to base grep-O on it\nbecause it allows cleaner code.\n\nHere’s a rebased version.\n\nJohannes Schindelin (3):\n  Unify code paths of threaded greps\n  grep: Add the option '--open-files-in-pager'\n  grep -O: allow optional argument specifying the pager (or editor)\n\nJonathan Nieder (1):\n  grep: refactor grep_objects loop into its own function\n\n Documentation/git-grep.txt         |    8 ++\n builtin/grep.c                     |  121 ++++++++++++++++++++++------\n git.c                              |    2 +-\n t/lib-pager.sh                     |   15 ++++\n t/t7006-pager.sh                   |   16 +---\n t/{t7002-grep.sh => t7810-grep.sh} |    0\n t/t7811-grep-open.sh               |  157 ++++++++++++++++++++++++++++++++++++\n 7 files changed, 280 insertions(+), 39 deletions(-)\n create mode 100644 t/lib-pager.sh\n rename t/{t7002-grep.sh => t7810-grep.sh} (100%)\n create mode 100755 t/t7811-grep-open.sh\n"},{"id":"143577","messageId":"20100612163118.GA5657@burratino","threadId":"24013","inReplyTo":"20100612162945.GB1406@burratino","subject":"[PATCH 1/4] grep: refactor grep_objects loop into its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T16:31:18Z","receivedAt":"2010-06-12T16:31:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Simplify cmd_grep by splitting off the loop that finds matches in a\nlist of trees.  So now the main part of cmd_grep looks like:\n\n\tif (!use_index) {\n\t\tint hit = grep_directory(&opt, paths);\n\t\tif (use_threads)\n\t\t\thit |= wait_all();\n\t\treturn !hit;\n\t}\n\tif (!list.nr) {\n\t\tif (!cached)\n\t\t\tsetup_work_tree();\n\t\tint hit = grep_cache(&opt, paths, cached);\n\t\tif (use_threads)\n\t\t\thit |= wait_all;\n\t\treturn !hit;\n\t}\n\thit = grep_objects(&opt, path, &list);\n\tif (use_threads)\n\t\thit |= wait_all();\n\treturn !hit;\n\nand is ripe for further refactoring.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/grep.c |   30 ++++++++++++++++++++----------\n 1 files changed, 20 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex b194ea3..5b8879f 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -675,6 +675,25 @@ static int grep_object(struct grep_opt *opt, const char **paths,\n \tdie(\"unable to grep from object of type %s\", typename(obj->type));\n }\n \n+static int grep_objects(struct grep_opt *opt, const char **paths,\n+\t\t\tconst struct object_array *list)\n+{\n+\tunsigned int i;\n+\tint hit = 0;\n+\tconst unsigned int nr = list->nr;\n+\n+\tfor (i = 0; i < nr; i++) {\n+\t\tstruct object *real_obj;\n+\t\treal_obj = deref_tag(list->objects[i].item, NULL, 0);\n+\t\tif (grep_object(opt, paths, real_obj, list->objects[i].name)) {\n+\t\t\thit = 1;\n+\t\t\tif (opt->status_only)\n+\t\t\t\tbreak;\n+\t\t}\n+\t}\n+\treturn hit;\n+}\n+\n static int grep_directory(struct grep_opt *opt, const char **paths)\n {\n \tstruct dir_struct dir;\n@@ -1024,16 +1043,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (cached)\n \t\tdie(\"both --cached and trees are given.\");\n-\n-\tfor (i = 0; i < list.nr; i++) {\n-\t\tstruct object *real_obj;\n-\t\treal_obj = deref_tag(list.objects[i].item, NULL, 0);\n-\t\tif (grep_object(&opt, paths, real_obj, list.objects[i].name)) {\n-\t\t\thit = 1;\n-\t\t\tif (opt.status_only)\n-\t\t\t\tbreak;\n-\t\t}\n-\t}\n+\thit = grep_objects(&opt, paths, &list);\n \n \tif (use_threads)\n \t\thit |= wait_all();\n-- \n1.7.1\n"},{"id":"143578","messageId":"20100612163211.GB5657@burratino","threadId":"24013","inReplyTo":"20100612162945.GB1406@burratino","subject":"[PATCH 2/4] Unify code paths of threaded greps","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T16:32:11Z","receivedAt":"2010-06-12T16:32:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThere were three awfully similar code paths ending the threaded grep. It\nis better to avoid duplicated code, though.\n\nThis change might very well prevent a race, where the grep patterns were\nfree()d before waiting that all threads finished.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n builtin/grep.c |   22 +++++-----------------\n 1 files changed, 5 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 5b8879f..2111212 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -590,7 +590,6 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n \t\tif (hit && opt->status_only)\n \t\t\tbreak;\n \t}\n-\tfree_grep_patterns(opt);\n \treturn hit;\n }\n \n@@ -708,7 +707,6 @@ static int grep_directory(struct grep_opt *opt, const char **paths)\n \t\tif (hit && opt->status_only)\n \t\t\tbreak;\n \t}\n-\tfree_grep_patterns(opt);\n \treturn hit;\n }\n \n@@ -1019,32 +1017,22 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (!use_index) {\n-\t\tint hit;\n \t\tif (cached)\n \t\t\tdie(\"--cached cannot be used with --no-index.\");\n \t\tif (list.nr)\n \t\t\tdie(\"--no-index cannot be used with revs.\");\n \t\thit = grep_directory(&opt, paths);\n-\t\tif (use_threads)\n-\t\t\thit |= wait_all();\n-\t\treturn !hit;\n-\t}\n-\n-\tif (!list.nr) {\n-\t\tint hit;\n+\t} else if (!list.nr) {\n \t\tif (!cached)\n \t\t\tsetup_work_tree();\n \n \t\thit = grep_cache(&opt, paths, cached);\n-\t\tif (use_threads)\n-\t\t\thit |= wait_all();\n-\t\treturn !hit;\n+\t} else {\n+\t\tif (cached)\n+\t\t\tdie(\"both --cached and trees are given.\");\n+\t\thit = grep_objects(&opt, paths, &list);\n \t}\n \n-\tif (cached)\n-\t\tdie(\"both --cached and trees are given.\");\n-\thit = grep_objects(&opt, paths, &list);\n-\n \tif (use_threads)\n \t\thit |= wait_all();\n \tfree_grep_patterns(&opt);\n-- \n1.7.1\n"},{"id":"143580","messageId":"20100612163651.GC5657@burratino","threadId":"24013","inReplyTo":"20100612162945.GB1406@burratino","subject":"[PATCH 3/4] grep: Add the option '--open-files-in-pager'","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T16:36:51Z","receivedAt":"2010-06-12T16:36:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThis adds an option to open the matching files in the pager, and if the\npager happens to be \"less\" (or \"vi\") and there is only one grep pattern,\nit also jumps to the first match right away.\n\nThe short option was chose as '-O' to avoid clashes with GNU grep's\noptions (as suggested by Junio).\n\nSo, 'git grep -O abc' is a short form for 'less +/abc $(grep -l abc)'\nexcept that it works also with spaces in file names, and it does not\nstart the pager if there was no matching file.\n\n[jn: rebased and added tests; with error handling fix from Junio\nsquashed in]\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nChanges from previous version:\n\n - fix leaked argv\n - die on chdir failure\n - chmod +x t/t7811-grep-open.sh\n\nThere was some discussion about changing the option’s name, but it\nmight be better to instead split it into two options, along these\nlines:\n\n  -O | --view-files\n  --viewer=\n\nI have left it as is for now.\n\n Documentation/git-grep.txt         |    8 ++\n builtin/grep.c                     |   73 +++++++++++++++++\n git.c                              |    2 +-\n t/lib-pager.sh                     |   15 ++++\n t/t7006-pager.sh                   |   16 +---\n t/{t7002-grep.sh => t7810-grep.sh} |    0\n t/t7811-grep-open.sh               |  154 ++++++++++++++++++++++++++++++++++++\n 7 files changed, 255 insertions(+), 13 deletions(-)\n create mode 100644 t/lib-pager.sh\n rename t/{t7002-grep.sh => t7810-grep.sh} (100%)\n create mode 100755 t/t7811-grep-open.sh\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 4b32322..8fdd8e1 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -14,6 +14,7 @@ SYNOPSIS\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-F | --fixed-strings] [-n]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n+\t   [-O | --open-files-in-pager]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match] [-q | --quiet]\n \t   [--max-depth <depth>]\n@@ -104,6 +105,13 @@ OPTIONS\n \tFor better compatibility with 'git diff', `--name-only` is a\n \tsynonym for `--files-with-matches`.\n \n+-O::\n+--open-files-in-pager::\n+\tOpen the matching files in the pager (not the output of 'grep').\n+\tIf the pager happens to be \"less\" or \"vi\", and the user\n+\tspecified only one pattern, the first file is positioned at\n+\tthe first match automatically.\n+\n -z::\n --null::\n \tOutput \\0 instead of the character that normally follows a\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 2111212..1e8b946 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -11,6 +11,8 @@\n #include \"tree-walk.h\"\n #include \"builtin.h\"\n #include \"parse-options.h\"\n+#include \"string-list.h\"\n+#include \"run-command.h\"\n #include \"userdiff.h\"\n #include \"grep.h\"\n #include \"quote.h\"\n@@ -556,6 +558,33 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \t}\n }\n \n+static void append_path(struct grep_opt *opt, const void *data, size_t len)\n+{\n+\tstruct string_list *path_list = opt->output_priv;\n+\n+\tif (len == 1 && *(const char *)data == '\\0')\n+\t\treturn;\n+\tstring_list_append(xstrndup(data, len), path_list);\n+}\n+\n+static void run_pager(struct grep_opt *opt, const char *prefix)\n+{\n+\tstruct string_list *path_list = opt->output_priv;\n+\tconst char **argv = xmalloc(sizeof(const char *) * (path_list->nr + 1));\n+\tint i, status;\n+\n+\tfor (i = 0; i < path_list->nr; i++)\n+\t\targv[i] = path_list->items[i].string;\n+\targv[path_list->nr] = NULL;\n+\n+\tif (prefix && chdir(prefix))\n+\t\tdie(\"Failed to chdir: %s\", prefix);\n+\tstatus = run_command_v_opt(argv, RUN_USING_SHELL);\n+\tif (status)\n+\t\texit(status);\n+\tfree(argv);\n+}\n+\n static int grep_cache(struct grep_opt *opt, const char **paths, int cached)\n {\n \tint hit = 0;\n@@ -799,9 +828,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint cached = 0;\n \tint seen_dashdash = 0;\n \tint external_grep_allowed__ignored;\n+\tint show_in_pager = 0;\n \tstruct grep_opt opt;\n \tstruct object_array list = { 0, 0, NULL };\n \tconst char **paths = NULL;\n+\tstruct string_list path_list = { NULL, 0, 0, 0 };\n \tint i;\n \tint dummy;\n \tint nongit = 0, use_index = 1;\n@@ -885,6 +916,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN(0, \"all-match\", &opt.all_match,\n \t\t\t\"show only matches from files that match all patterns\"),\n \t\tOPT_GROUP(\"\"),\n+\t\tOPT_BOOLEAN('O', \"open-files-in-pager\", &show_in_pager,\n+\t\t\t\"show matching files in the pager\"),\n \t\tOPT_BOOLEAN(0, \"ext-grep\", &external_grep_allowed__ignored,\n \t\t\t    \"allow calling of grep(1) (ignored by this build)\"),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n@@ -960,6 +993,20 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\targc--;\n \t}\n \n+\tif (show_in_pager) {\n+\t\tconst char *pager = git_pager(1);\n+\t\tif (!pager) {\n+\t\t\tshow_in_pager = 0;\n+\t\t} else {\n+\t\t\topt.name_only = 1;\n+\t\t\topt.null_following_name = 1;\n+\t\t\topt.output_priv = &path_list;\n+\t\t\topt.output = append_path;\n+\t\t\tstring_list_append(pager, &path_list);\n+\t\t\tuse_threads = 0;\n+\t\t}\n+\t}\n+\n \tif (!opt.pattern_list)\n \t\tdie(\"no pattern given.\");\n \tif (!opt.fixed && opt.ignore_case)\n@@ -1016,6 +1063,30 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tpaths[1] = NULL;\n \t}\n \n+\tif (show_in_pager && (cached || list.nr))\n+\t\tdie(\"--open-files-in-pager only works on the worktree\");\n+\n+\tif (show_in_pager && opt.pattern_list && !opt.pattern_list->next) {\n+\t\tconst char *pager = path_list.items[0].string;\n+\t\tint len = strlen(pager);\n+\n+\t\tif (len > 4 && is_dir_sep(pager[len - 5]))\n+\t\t\tpager += len - 4;\n+\n+\t\tif (!strcmp(\"less\", pager) || !strcmp(\"vi\", pager)) {\n+\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&buf, \"+/%s%s\",\n+\t\t\t\t\tstrcmp(\"less\", pager) ? \"\" : \"*\",\n+\t\t\t\t\topt.pattern_list->pattern);\n+\t\t\tstring_list_append(buf.buf, &path_list);\n+\t\t\tstrbuf_detach(&buf, NULL);\n+\t\t}\n+\t}\n+\n+\tif (!show_in_pager)\n+\t\tsetup_pager();\n+\n+\n \tif (!use_index) {\n \t\tif (cached)\n \t\t\tdie(\"--cached cannot be used with --no-index.\");\n@@ -1035,6 +1106,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (use_threads)\n \t\thit |= wait_all();\n+\tif (hit && show_in_pager)\n+\t\trun_pager(&opt, prefix);\n \tfree_grep_patterns(&opt);\n \treturn !hit;\n }\ndiff --git a/git.c b/git.c\nindex 99f0363..265fa09 100644\n--- a/git.c\n+++ b/git.c\n@@ -329,7 +329,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"fsck-objects\", cmd_fsck, RUN_SETUP },\n \t\t{ \"gc\", cmd_gc, RUN_SETUP },\n \t\t{ \"get-tar-commit-id\", cmd_get_tar_commit_id },\n-\t\t{ \"grep\", cmd_grep, USE_PAGER },\n+\t\t{ \"grep\", cmd_grep },\n \t\t{ \"hash-object\", cmd_hash_object },\n \t\t{ \"help\", cmd_help },\n \t\t{ \"index-pack\", cmd_index_pack },\ndiff --git a/t/lib-pager.sh b/t/lib-pager.sh\nnew file mode 100644\nindex 0000000..f8c6025\n--- /dev/null\n+++ b/t/lib-pager.sh\n@@ -0,0 +1,15 @@\n+#!/bin/sh\n+\n+test_expect_success 'determine default pager' '\n+\ttest_might_fail git config --unset core.pager &&\n+\tless=$(\n+\t\tunset PAGER GIT_PAGER;\n+\t\tgit var GIT_PAGER\n+\t) &&\n+\ttest -n \"$less\"\n+'\n+\n+if expr \"$less\" : '^[a-z][a-z]*$' >/dev/null\n+then\n+\ttest_set_prereq SIMPLEPAGER\n+fi\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 3bc7a2a..fc993fc 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -3,6 +3,7 @@\n test_description='Test automatic use of a pager.'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-pager.sh\n \n cleanup_fail() {\n \techo >&2 cleanup failed\n@@ -158,21 +159,12 @@ test_expect_success 'color when writing to a file intended for a pager' '\n \tcolorful colorful.log\n '\n \n-test_expect_success 'determine default pager' '\n-\tunset PAGER GIT_PAGER &&\n-\ttest_might_fail git config --unset core.pager ||\n-\tcleanup_fail &&\n-\n-\tless=$(git var GIT_PAGER) &&\n-\ttest -n \"$less\"\n-'\n-\n-if expr \"$less\" : '^[a-z][a-z]*$' >/dev/null && test_have_prereq TTY\n+if test_have_prereq SIMPLEPAGER && test_have_prereq TTY\n then\n-\ttest_set_prereq SIMPLEPAGER\n+\ttest_set_prereq SIMPLEPAGERTTY\n fi\n \n-test_expect_success SIMPLEPAGER 'default pager is used by default' '\n+test_expect_success SIMPLEPAGERTTY 'default pager is used by default' '\n \tunset PAGER GIT_PAGER &&\n \ttest_might_fail git config --unset core.pager &&\n \trm -f default_pager_used ||\ndiff --git a/t/t7002-grep.sh b/t/t7810-grep.sh\nsimilarity index 100%\nrename from t/t7002-grep.sh\nrename to t/t7810-grep.sh\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nnew file mode 100755\nindex 0000000..fcfc56e\n--- /dev/null\n+++ b/t/t7811-grep-open.sh\n@@ -0,0 +1,154 @@\n+#!/bin/sh\n+\n+test_description='git grep --open-files-in-pager\n+'\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-pager.sh\n+unset PAGER GIT_PAGER\n+\n+test_expect_success 'setup' '\n+\ttest_commit initial grep.h \"\n+enum grep_pat_token {\n+\tGREP_PATTERN,\n+\tGREP_PATTERN_HEAD,\n+\tGREP_PATTERN_BODY,\n+\tGREP_AND,\n+\tGREP_OPEN_PAREN,\n+\tGREP_CLOSE_PAREN,\n+\tGREP_NOT,\n+\tGREP_OR,\n+};\" &&\n+\n+\ttest_commit add-user revision.c \"\n+\t}\n+\tif (seen_dashdash)\n+\t\tread_pathspec_from_stdin(revs, &sb, prune);\n+\tstrbuf_release(&sb);\n+}\n+\n+static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)\n+{\n+\tappend_grep_pattern(&revs->grep_filter, ptn, \\\"command line\\\", 0, what);\n+\" &&\n+\n+\tmkdir subdir &&\n+\ttest_commit subdir subdir/grep.c \"enum grep_pat_token\" &&\n+\n+\ttest_commit uninteresting unrelated \"hello, world\" &&\n+\n+\techo GREP_PATTERN >untracked\n+'\n+\n+test_expect_success SIMPLEPAGER 'git grep -O' '\n+\tcat >$less <<-\\EOF &&\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >pager-args\n+\tEOF\n+\tchmod +x $less &&\n+\tcat >expect.less <<-\\EOF &&\n+\t+/*GREP_PATTERN\n+\tgrep.h\n+\tEOF\n+\techo grep.h >expect.notless &&\n+\t>empty &&\n+\n+\tPATH=.:$PATH git grep -O GREP_PATTERN >out &&\n+\t{\n+\t\ttest_cmp expect.less pager-args ||\n+\t\ttest_cmp expect.notless pager-args\n+\t} &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'git grep -O --cached' '\n+\ttest_must_fail git grep --cached -O GREP_PATTERN >out 2>msg &&\n+\tgrep open-files-in-pager msg\n+'\n+\n+test_expect_success 'git grep -O --no-index' '\n+\trm -f expect.less pager-args out &&\n+\tcat >expect <<-\\EOF &&\n+\tgrep.h\n+\tuntracked\n+\tEOF\n+\t>empty &&\n+\n+\t(\n+\t\tGIT_PAGER='\\''printf \"%s\\n\" >pager-args'\\'' &&\n+\t\texport GIT_PAGER &&\n+\t\tgit grep --no-index -O GREP_PATTERN >out\n+\t) &&\n+\ttest_cmp expect pager-args &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'setup: fake \"less\"' '\n+\tcat >less <<-\\EOF\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >actual\n+\tEOF\n+'\n+\n+test_expect_success 'git grep -O jumps to line in less' '\n+\tcat >expect <<-\\EOF &&\n+\t+/*GREP_PATTERN\n+\tgrep.h\n+\tEOF\n+\t>empty &&\n+\n+\tGIT_PAGER=./less git grep -O GREP_PATTERN >out &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'modified file' '\n+\trm -f actual &&\n+\tcat >less <<-\\EOF &&\n+\t#!/bin/sh\n+\tprintf \"%s\\n\" \"$@\" >actual\n+\tEOF\n+\tchmod +x $less &&\n+\tcat >expect <<-\\EOF &&\n+\t+/*enum grep_pat_token\n+\tgrep.h\n+\trevision.c\n+\tsubdir/grep.c\n+\tunrelated\n+\tEOF\n+\t>empty &&\n+\n+\techo \"enum grep_pat_token\" >unrelated &&\n+\ttest_when_finished \"git checkout HEAD unrelated\" &&\n+\tGIT_PAGER=./less git grep -F -O \"enum grep_pat_token\" >out &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out\n+'\n+\n+test_expect_success 'run from subdir' '\n+\trm -f actual &&\n+\techo grep.c >expect &&\n+\t>empty &&\n+\n+\t(\n+\t\tcd subdir &&\n+\t\texport GIT_PAGER &&\n+\t\tGIT_PAGER='\\''printf \"%s\\n\" >../args'\\'' &&\n+\t\tgit grep -O \"enum grep_pat_token\" >../out &&\n+\t\tGIT_PAGER=\"pwd >../dir; :\" &&\n+\t\tgit grep -O \"enum grep_pat_token\" >../out2\n+\t) &&\n+\tcase $(cat dir) in\n+\t*subdir)\n+\t\t: good\n+\t\t;;\n+\t*)\n+\t\tfalse\n+\t\t;;\n+\tesac &&\n+\ttest_cmp expect args &&\n+\ttest_cmp empty out &&\n+\ttest_cmp empty out2\n+'\n+\n+test_done\n-- \n1.7.1\n"},{"id":"143581","messageId":"20100612163946.GD5657@burratino","threadId":"24013","inReplyTo":"20100612162945.GB1406@burratino","subject":"[PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T16:39:46Z","receivedAt":"2010-06-12T16:39:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSuppose you want to edit all files that contain a specific search term.\nOf course, you can do something totally trivial such as\n\n\tgit grep -z -e <term> | xargs -0r vi +/<term>\n\nbut maybe you are happy that the same will be achieved by\n\n\tgit grep -Ovi <term>\n\nnow.\n\n[jn: rebased and added tests]\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nAs before.  Thanks for reading.\n\n Documentation/git-grep.txt |    6 +++---\n builtin/grep.c             |   26 ++++++++++++--------------\n t/t7811-grep-open.sh       |    9 ++++++---\n 3 files changed, 21 insertions(+), 20 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 8fdd8e1..d89ec32 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-F | --fixed-strings] [-n]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n-\t   [-O | --open-files-in-pager]\n+\t   [(-O | --open-files-in-pager) [<pager>]]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match] [-q | --quiet]\n \t   [--max-depth <depth>]\n@@ -105,8 +105,8 @@ OPTIONS\n \tFor better compatibility with 'git diff', `--name-only` is a\n \tsynonym for `--files-with-matches`.\n \n--O::\n---open-files-in-pager::\n+-O [<pager>]::\n+--open-files-in-pager [<pager>]::\n \tOpen the matching files in the pager (not the output of 'grep').\n \tIf the pager happens to be \"less\" or \"vi\", and the user\n \tspecified only one pattern, the first file is positioned at\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 1e8b946..f32fbbc 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -828,7 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint cached = 0;\n \tint seen_dashdash = 0;\n \tint external_grep_allowed__ignored;\n-\tint show_in_pager = 0;\n+\tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n \tstruct grep_opt opt;\n \tstruct object_array list = { 0, 0, NULL };\n \tconst char **paths = NULL;\n@@ -916,8 +916,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN(0, \"all-match\", &opt.all_match,\n \t\t\t\"show only matches from files that match all patterns\"),\n \t\tOPT_GROUP(\"\"),\n-\t\tOPT_BOOLEAN('O', \"open-files-in-pager\", &show_in_pager,\n-\t\t\t\"show matching files in the pager\"),\n+\t\t{ OPTION_STRING, 'O', \"open-files-in-pager\", &show_in_pager,\n+\t\t\t\"pager\", \"show matching files in the pager\",\n+\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)default_pager },\n \t\tOPT_BOOLEAN(0, \"ext-grep\", &external_grep_allowed__ignored,\n \t\t\t    \"allow calling of grep(1) (ignored by this build)\"),\n \t\t{ OPTION_CALLBACK, 0, \"help-all\", &options, NULL, \"show usage\",\n@@ -993,18 +994,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\targc--;\n \t}\n \n+\tif (show_in_pager == default_pager)\n+\t\tshow_in_pager = git_pager(1);\n \tif (show_in_pager) {\n-\t\tconst char *pager = git_pager(1);\n-\t\tif (!pager) {\n-\t\t\tshow_in_pager = 0;\n-\t\t} else {\n-\t\t\topt.name_only = 1;\n-\t\t\topt.null_following_name = 1;\n-\t\t\topt.output_priv = &path_list;\n-\t\t\topt.output = append_path;\n-\t\t\tstring_list_append(pager, &path_list);\n-\t\t\tuse_threads = 0;\n-\t\t}\n+\t\topt.name_only = 1;\n+\t\topt.null_following_name = 1;\n+\t\topt.output_priv = &path_list;\n+\t\topt.output = append_path;\n+\t\tstring_list_append(show_in_pager, &path_list);\n+\t\tuse_threads = 0;\n \t}\n \n \tif (!opt.pattern_list)\ndiff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh\nindex fcfc56e..8db4fc8 100755\n--- a/t/t7811-grep-open.sh\n+++ b/t/t7811-grep-open.sh\n@@ -99,7 +99,11 @@ test_expect_success 'git grep -O jumps to line in less' '\n \n \tGIT_PAGER=./less git grep -O GREP_PATTERN >out &&\n \ttest_cmp expect actual &&\n-\ttest_cmp empty out\n+\ttest_cmp empty out &&\n+\n+\tgit grep -O./less GREP_PATTERN >out2 &&\n+\ttest_cmp expect actual &&\n+\ttest_cmp empty out2\n '\n \n test_expect_success 'modified file' '\n@@ -135,8 +139,7 @@ test_expect_success 'run from subdir' '\n \t\texport GIT_PAGER &&\n \t\tGIT_PAGER='\\''printf \"%s\\n\" >../args'\\'' &&\n \t\tgit grep -O \"enum grep_pat_token\" >../out &&\n-\t\tGIT_PAGER=\"pwd >../dir; :\" &&\n-\t\tgit grep -O \"enum grep_pat_token\" >../out2\n+\t\tgit grep -O\"pwd >../dir; :\" \"enum grep_pat_token\" >../out2\n \t) &&\n \tcase $(cat dir) in\n \t*subdir)\n-- \n1.7.1\n"},{"id":"143585","messageId":"4C13C0E1.3070707@gnu.org","threadId":"24013","inReplyTo":"20100612163946.GD5657@burratino","subject":"Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2010-06-12T17:16:17Z","receivedAt":"2010-06-12T17:16:17Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 06/12/2010 06:39 PM, Jonathan Nieder wrote:\n> From: Johannes Schindelin<johannes.schindelin@gmx.de>\n>\n> Suppose you want to edit all files that contain a specific search term.\n> Of course, you can do something totally trivial such as\n>\n> \tgit grep -z -e<term>  | xargs -0r vi +/<term>\n>\n> but maybe you are happy that the same will be achieved by\n>\n> \tgit grep -Ovi<term>\n>\n> now.\n\nAh, I had missed that the argument was optional.  I think that optional \narguments are slightly frowned upon for POSIX utilities, because they're \na bit different.  In fact, I think the only one is -i in sed (and BSD \nsed instead makes it mandatory...).  Personally I have no problem with \nmaking -O in GNU grep optional too, I'll see what the other maintainers say.\n\nAcked-by: Paolo Bonzini <bonzini@gnu.org>\n\nPaolo\n"},{"id":"143591","messageId":"alpine.DEB.1.00.1006122335190.2689@bonsai2","threadId":"24013","inReplyTo":"20100612162945.GB1406@burratino","subject":"Re: [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-06-12T21:36:02Z","receivedAt":"2010-06-12T21:36:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 12 Jun 2010, Jonathan Nieder wrote:\n\n> Johannes Schindelin wrote:\n> \n> > Nope, the patch is still there, even after a rebase.\n> \n> Ah, I see.  Looks like a good patch (d00d655, Unify code paths of \n> threaded greps, 2010-02-07) though in principle a separate topic, since \n> grep -O already sets use_threads = 0.\n\nNo. It applies without the patch. But in my tests, it just does not work \non multi-core machines without the patch.\n\nCiao,\nDscho\n"},{"id":"143592","messageId":"20100612221255.GA2718@burratino","threadId":"24013","inReplyTo":"alpine.DEB.1.00.1006122335190.2689@bonsai2","subject":"Re: [PATCH v3 0/4] Teach 'git grep' about --open-files-in-pager=[<pager>]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-12T22:12:55Z","receivedAt":"2010-06-12T22:12:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Schindelin wrote:\n\n> But in my tests, it just does not work \n> on multi-core machines without the patch.\n\nDid you try the v2 series I sent earlier?  I tested it on a machine\nwith two multi-core CPUs.\n\nNow that I’ve read your patch, though, I don’t want to go back.  The\ncode duplication in the v2 series was ugly and did not leave the door\nopen to easily adding threading support later, because it ran the\npager _before_ waiting for pending threads.  And besides, the \"Unify\ncode paths\" patch contains an important fix for grep without -O.\n\nThanks for both patches.\nJonathan\n"},{"id":"143614","messageId":"7vvd9mevqy.fsf@alter.siamese.dyndns.org","threadId":"24013","inReplyTo":"20100612163946.GD5657@burratino","subject":"Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-13T16:35:17Z","receivedAt":"2010-06-13T16:35:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index 8fdd8e1..d89ec32 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -14,7 +14,7 @@ SYNOPSIS\n>  \t   [-E | --extended-regexp] [-G | --basic-regexp]\n>  \t   [-F | --fixed-strings] [-n]\n>  \t   [-l | --files-with-matches] [-L | --files-without-match]\n> -\t   [-O | --open-files-in-pager]\n> +\t   [(-O | --open-files-in-pager) [<pager>]]\n>  \t   [-z | --null]\n>  \t   [-c | --count] [--all-match] [-q | --quiet]\n>  \t   [--max-depth <depth>]\n\nHmm, does \"git grep -e Heh -O frotz\" look for Heh and show in the frotz\npager, or does it look for Heh in paths under frotz/ directory and show\nhits in the default pager?\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 1e8b946..f32fbbc 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -828,7 +828,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n>  \tint cached = 0;\n>  \tint seen_dashdash = 0;\n>  \tint external_grep_allowed__ignored;\n> -\tint show_in_pager = 0;\n> +\tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n\nIf there were another instance of constant string \"dummy\" elsewhere in the\nprogram, is a clever compiler-linker combo allowed to optimize memory use\nby allocating one instance of such a string and pointing default_pager\npointer to it?  IOW, if the patch were:\n\n> +\tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n> +\tconst char *another_dummy = \"dummy\";\n\ncould another_dummy and default_pager start out with the same value?\n\nThis is just out of curiosity and does not affect correctness of the code,\nbut I am wondering...\n"},{"id":"143615","messageId":"AANLkTikny5zhO18cbGvq2zcMOdfAcDbAG6LiOLGTtepv@mail.gmail.com","threadId":"24013","inReplyTo":"7vvd9mevqy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2010-06-13T17:04:44Z","receivedAt":"2010-06-13T17:04:44Z","isPatch":true,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":">> -        [-O | --open-files-in-pager]\n>> +        [(-O | --open-files-in-pager) [<pager>]]\n>\n> Hmm, does \"git grep -e Heh -O frotz\" look for Heh and show in the frotz\n> pager, or does it look for Heh in paths under frotz/ directory and show\n> hits in the default pager?\n\nThe latter.\n\n>> +     const char *show_in_pager = NULL, *default_pager = \"dummy\";\n>\n> If there were another instance of constant string \"dummy\" elsewhere in the\n> program, is a clever compiler-linker combo allowed to optimize memory use\n> by allocating one instance of such a string and pointing default_pager\n> pointer to it?  IOW, if the patch were:\n>\n>> +     const char *show_in_pager = NULL, *default_pager = \"dummy\";\n>> +     const char *another_dummy = \"dummy\";\n>\n> could another_dummy and default_pager start out with the same value?\n\nIn the same file, the compiler will do it already today.  In another\nfile, no (except if it does link-time optimization of course).\n\nPaolo\n"},{"id":"143639","messageId":"20100614063125.GA2006@burratino","threadId":"24013","inReplyTo":"7vvd9mevqy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 4/4] grep -O: allow optional argument specifying the pager (or editor)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-06-14T06:31:25Z","receivedAt":"2010-06-14T06:31:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> -\t   [-O | --open-files-in-pager]\n>> +\t   [(-O | --open-files-in-pager) [<pager>]]\n[...]\n>\n> Hmm, does \"git grep -e Heh -O frotz\" look for Heh and show in the frotz\n> pager, or does it look for Heh in paths under frotz/ directory and show\n> hits in the default pager?\n\nAs Paolo mentioned, it is the latter.  Patch for squashing\nis below.\n\n>> +\tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n>\n> If there were another instance of constant string \"dummy\" elsewhere in the\n> program, is a clever compiler-linker combo allowed to optimize memory use\n> by allocating one instance of such a string and pointing default_pager\n> pointer to it?\n\nFor a moment, you had me worried: would an (insane) compiler be\nallowed to intern strings that appear as arguments, making argv[i]\nactually compare equal to default_pager?\n\nLuckily, the answer is no, because the strings pointed to in argv\nare guaranteed to be modifiable (see: Execution environments → Hosted\nenvironment → Program startup).\n\n-- 8< --\nSubject: grep -O: do not advertize non-\"sticked\" form in documentation\n\nTo avoid ambiguity in option parsing, the -O option will\nonly take an argument if it is stuck to the option like\n-Ovi or --open-files-in-pager=vim.  The un-sticked form\n-O vi means to search paths under 'vi' and show hits in\nthe default pager.\n\nNoticed-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n Documentation/git-grep.txt |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex d89ec32..0e12fe4 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-F | --fixed-strings] [-n]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n-\t   [(-O | --open-files-in-pager) [<pager>]]\n+\t   [-O[<pager>] | --open-files-in-pager[=<pager>]]\n \t   [-z | --null]\n \t   [-c | --count] [--all-match] [-q | --quiet]\n \t   [--max-depth <depth>]\n@@ -106,7 +106,7 @@ OPTIONS\n \tsynonym for `--files-with-matches`.\n \n--O [<pager>]::\n---open-files-in-pager [<pager>]::\n+-O[<pager>]::\n+--open-files-in-pager=[<pager>]::\n \tOpen the matching files in the pager (not the output of 'grep').\n \tIf the pager happens to be \"less\" or \"vi\", and the user\n \tspecified only one pattern, the first file is positioned at\n-- \n1.7.1.246.g398e5.dirty\n"}]}