git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
To: Chris Packham <judge.packham@gmail.com>
Cc: Jonathan Nieder <jrnieder@gmail.com>,
	git@vger.kernel.org, Jens.Lehmann@web.de, gitster@pobox.com
Subject: Re: [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd
Date: Sun, 17 Oct 2010 17:01:38 +0700	[thread overview]
Message-ID: <20101017094524.GA6034@do> (raw)
In-Reply-To: <4CBA63E2.8030502@gmail.com>

On Sat, Oct 16, 2010 at 07:48:02PM -0700, Chris Packham wrote:
> On 16/10/10 11:42, Jonathan Nieder wrote:
> > Hi,
> > 
> > Chris Packham wrote:
> > 
> >> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> >>
> >> When both GIT_DIR and GIT_WORK_TREE are set, if cwd is outside worktree,
> >> prefix (the one passed to every builtin commands) will be set to NULL,
> >> which means "user stays at worktree topdir".
> >>
> >> As a consequence, command line arguments are supposed to be relative
> >> to worktree topdir, not current working directory. Not very intuitive.
> > 
> > Thanks.  More detailed history for this patch:
> > 
> >  - v0: http://thread.gmane.org/gmane.comp.version-control.git/157599/focus=157601
> >  - v1: http://thread.gmane.org/gmane.comp.version-control.git/158287
> >  - v2: http://thread.gmane.org/gmane.comp.version-control.git/158369
> > 
> 
> I think I must have missed v2. I was playing around with my gmail
> filters around that time so I could have missed them. Actually now I've
> found the message it's missing the last 'm' in gmail.com. I'll grab the
> latest patch and give it a test when I get a chance.

I missed the last "m" in your email address. That's why v2 never reached you.
I thought I sent you an email but probably forgot it.

Anyway v2 does not work if worktree and cwd are on different Windows drives.
This on top should fix it:

---8<---
diff --git a/setup.c b/setup.c
index 2389a9e..35d2691 100644
--- a/setup.c
+++ b/setup.c
@@ -371,12 +371,8 @@ static const char *setup_prefix(const char *cwd)
 	}
 	/* get /foo/, not /foo/baa if /foo/baa1 and /foo/baa2 are given */
 	else if (worktree[len] && cwd[len]) {
-		while (len && !is_dir_sep(worktree[len]))
-			len--;
-		len++;
-
 		/* Worktree and cwd are on different drives? */
-		if (len == 3 && has_dos_drive_prefix(cwd)) {
+		if (!len && has_dos_drive_prefix(cwd)) {
 			if (startup_info) {
 				/* make_path_to_path will add the trailing slash */
 				startup_info->cwd_to_worktree = make_path_to_path(NULL, worktree);
@@ -384,6 +380,10 @@ static const char *setup_prefix(const char *cwd)
 			}
 			return NULL;
 		}
+
+		while (len && !is_dir_sep(worktree[len]))
+			len--;
+		len++;
 	}
 	else {
 		if (worktree[len]) {
---8<---

> 
> > Any thoughts about the previous questions?
> > 
> 
> I haven't caught up on the newest thread so no great revelations. Except
> that for the grep submodules use-case we can assume that the worktree
> will be a subdirectory of the cwd. I don't think we want to limit
> ourselves to that one use-case.

While at it, have you thought of support --recursive and
--full-tree [1]? There are issues with --full-tree and prefixes [2],
which is why it is dropped but I think it's a good idea.

--full-tree disregards where you stand and greps in whole repo. In a
repo with submodules, that would mean grep the supermodule and all
submodules regardless where you stand, even if you stand in a
submodule.

[1] http://mid.gmane.org/7vk4xggv27.fsf@alter.siamese.dyndns.org
[2] http://mid.gmane.org/7vskaqptvj.fsf@alter.siamese.dyndns.org
-- 
Duy

  reply	other threads:[~2010-10-17 10:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-15 23:26 [RGC/PATCHv2] grep: submodule support Chris Packham
2010-10-15 23:26 ` [RFC/PATCHv2 1/5] worktree: provide better prefix to go back to original cwd Chris Packham
2010-10-16 18:42   ` Jonathan Nieder
2010-10-17  2:48     ` Chris Packham
2010-10-17 10:01       ` Nguyen Thai Ngoc Duy [this message]
2010-10-18  2:05         ` Chris Packham
2010-10-15 23:26 ` [RFC/PATCHv2 2/5] grep: output the path from cwd to worktree Chris Packham
2010-10-15 23:26 ` [RFC/PATCHv2 3/5] grep_cache: check pathspec first Chris Packham
2010-10-15 23:26 ` [RFC/PATCHv2 4/5] add test for git grep --recursive Chris Packham
2010-10-15 23:26 ` [RFC/PATCHv2 5/5] grep: add support for grepping in submodules Chris Packham
2010-10-17 10:28   ` Nguyen Thai Ngoc Duy
2010-10-18  2:01     ` Chris Packham
2010-10-18  3:37       ` Nguyen Thai Ngoc Duy
2010-10-16 15:54 ` [RGC/PATCHv2] grep: submodule support Jens Lehmann
2010-10-17  2:13   ` Chris Packham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101017094524.GA6034@do \
    --to=pclouds@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=judge.packham@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).