[PATCH] fix vc git

[PATCH] fix vc git

[Duncan Mak]

Fix vc-git.el to not cd into a non-existent directory.

---

 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base 54851157acf707eb953eada2a84830897dde5c1d
last 9172c26ece749fc4289eeb89ef45c35936c5a869
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..d3ba93325255aea3dc52187dd703d7a8
63cd6e5d 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (if (file-exists-p dir) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil "ls-files"
 "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))
-- 
1.4.4.1

Re: [PATCH] fix vc git

[Jakub Narebski]

Duncan Mak wrote:

> -      (when dir (cd dir))
> +      (if (file-exists-p dir) (cd dir))

Adding `file-exist-p' is good, but there is no need to change
`when' to `if'.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] fix vc git

[Duncan Mak]

Fix vc-git.el to not cd into a non-existent directory.

---

 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base 54851157acf707eb953eada2a84830897dde5c1d
last c2af9380cd8ec0170b9b251c0ee8a5efa63bac87
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..b3c8040dd766c9b15e1d0a9247b6343fdf0a8caf
100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (when (file-exists-p dir) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
"ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))
-- 
1.4.4.1

Re: [PATCH] fix vc git

[Junio C Hamano]

"Duncan Mak" <duncan@a-chinaman.com> writes:

> --- a/contrib/emacs/vc-git.el
> +++ b/contrib/emacs/vc-git.el
> @@ -58,7 +58,7 @@
>   (with-temp-buffer
>     (let* ((dir (file-name-directory file))
>            (name (file-relative-name file dir)))
> -      (when dir (cd dir))
> +      (when (file-exists-p dir) (cd dir))
>       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
> "ls-files" "-c" "-z" "--" name)))
>            (let ((str (buffer-string)))
>              (and (> (length str) (length name))

I wonder what would happen when (file-name-directory file)
returns nil with this change...

Re: [PATCH] fix vc git

[Duncan Mak]

I don't think vc-git-registered-file will ever be called with a
filename without a directory, as it is used as a hook on
vc-next-action, which works on a real file.

Either way, it's a modified patch:

Fix vc-git.el to not cd into a non-existent directory.

---

 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base c902c9a608c1b727160d6fbb04fa363d9418cd00
last 96e9944b714094ab81f8091711a92506886b8004
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..668bd4a98653046aadf91e7a91af6cd76ba044d4
100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (when (and dir (file-exists-p dir)) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
"ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))
-- 
1.4.4.1

Re: [PATCH] fix vc git

[Junio C Hamano]

"Duncan Mak" <duncan@a-chinaman.com> writes:

> I don't think vc-git-registered-file will ever be called with a
> filename without a directory, as it is used as a hook on
> vc-next-action, which works on a real file.

I am showing my ignorance, but

 (1) if it works on a real file then can it ever give you a file
     that is in nonexisting directory?

 (2) when it works on a file at the root level of the project
     tree, say, just "Makefile" wouldn't dir which is the result
     of (file-name-directory file) be sometimes nil?  I suspect
     that is the reason of the original (when dir ...)

In either case, your change would not break anything.  I am just
being curious.

Re: [PATCH] fix vc git

[Alexandre Julliard]

"Duncan Mak" <duncan@a-chinaman.com> writes:

> I don't think vc-git-registered-file will ever be called with a
> filename without a directory, as it is used as a hook on
> vc-next-action, which works on a real file.

Well, if it's a real file I'd expect the directory to exist, and if it
doesn't, I'm not sure there's much point in calling git-ls-files at
all. In which case do you get a non-existent directory here?

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] fix vc git

[Duncan Mak]

On 12/22/06, Alexandre Julliard <julliard@winehq.org> wrote:
>
> Well, if it's a real file I'd expect the directory to exist, and if it
> doesn't, I'm not sure there's much point in calling git-ls-files at
> all. In which case do you get a non-existent directory here?
>

Yeah, but the issue is that, as you know, to create a new file in
emacs, you give find-file  a non-existent file and emacs will open up
a buffer for you and let you save it when you're done.

Once I installed the git vc backend, that feature stopped working for
me - that's the reason why I made this patch.

Duncan.

Re: [PATCH] fix vc git

[Alexandre Julliard]

"Duncan Mak" <duncan@a-chinaman.com> writes:

> Yeah, but the issue is that, as you know, to create a new file in
> emacs, you give find-file  a non-existent file and emacs will open up
> a buffer for you and let you save it when you're done.

OK, but in that case vc-git-registered needs to return failure, you
cannot call git-ls-files as it may find the file in the wrong
directory. I'd suggest something like this:


>From abf4311add221102957145255d5418a7ec06fe1d Mon Sep 17 00:00:00 2001
From: Alexandre Julliard <julliard@winehq.org>
Date: Fri, 22 Dec 2006 14:51:23 +0100
Subject: [PATCH] vc-git: Ignore errors caused by a non-existent directory in vc-git-registered.

Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
 contrib/emacs/vc-git.el |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b63619..3eb4bd1 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,8 +58,9 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
-      (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil "ls-files" "-c" "-z" "--" name)))
+      (and (ignore-errors
+             (when dir (cd dir))
+             (eq 0 (call-process "git" nil '(t nil) nil "ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))
                   (string= (substring str 0 (1+ (length name))) (concat name "\0"))))))))
-- 
1.4.4.2.g28ce

-- 
Alexandre Julliard
julliard@winehq.org

Re: [PATCH] fix vc git

[Duncan Mak]

On 12/22/06, Alexandre Julliard <julliard@winehq.org> wrote:
>
> OK, but in that case vc-git-registered needs to return failure, you
> cannot call git-ls-files as it may find the file in the wrong
> directory. I'd suggest something like this:
>

Ah, okay. I agree, that is a better patch.

Would you be committing it  soon?

Thanks,

Duncan.