From: David Turner Date: Wed, 07 May 2014 03:33:51 GMT Subject: Re: [PATCH 1/3] After chdir to run grep, return to old directory Message-ID: <1399433631.11843.57.camel@stross> In-Reply-To: <20140507030037.GB20726@sigill.intra.peff.net> This causes my test to pass and generally seems correct to me. On Tue, 2014-05-06 at 23:00 -0400, Jeff King wrote: ... > That being said, this really seems like something that the run-command > interface should be doing, since it can handle the chdir in the forked > child. And indeed, it seems to support that. > > Maybe: > > -- >8 -- > Subject: grep: use run-command's "dir" option for --open-files-in-pager > > Git generally changes directory to the repository root on > startup. When running "grep --open-files-in-pager" from a > subdirectory, we chdir back to the original directory before > running the pager, so that we can feed the relative > pathnames to the pager. > > We currently do this chdir manually, but we can ask > run_command to do it for us. This is fewer lines of code, > and as a bonus, the chdir is limited to the child process, > which avoids any unexpected surprises for code running after > the pager (there isn't any currently, but this is > future-proofing). > > Signed-off-by: Jeff King > --- > builtin/grep.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/builtin/grep.c b/builtin/grep.c > index 69ac2d8..43af5b7 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -361,9 +361,7 @@ static void run_pager(struct grep_opt *opt, const char *prefix) > 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); > + status = run_command_v_opt_cd_env(argv, RUN_USING_SHELL, prefix, NULL); > if (status) > exit(status); > free(argv);