Using different default-directory and relative paths in VC, Was: Re: bug#21383

Using different default-directory and relative paths in VC, Was: Re: bug#21383

[Dmitry Gutov]

On 09/07/2015 01:29 AM, Stefan Monnier wrote:

>> Does vc-status get a relative file name as input?
>
> Currently, I don't think so.  But that could be changed if needed.

Hopefully not.

>> b) how are you going to enforce it?.
>
> By checking all callers?

That's an odd thing to do for an API. What if diff-hl calls vc-state 
with an absolute path? Will it get a slap on the wrist?

>> (*) If we try to eliminate this duplication of info, we introduce
>> duplication of effort inside VC. Seems to be pointless complexity.
>
> Clearly it's easy to get back the absolute file name with just
> (expand-file-name <FILE>), and I don't see where we'd obviously get
> duplication of efforts.  So I'm not sure why you think it'd add complexity.

I was thinking that due to assuming that default-directory will have to 
be bound to the root. But indeed, simply using the parent directory of 
the file in question should work well enough. Although when a command is 
passed a list of files, determining their common parent will require 
more effort.

> What I do see as a problem is that it would require a careful
> study&adjustment of the whole VC code.  I'm not sure if the result would
> be more complex or simpler, admittedly.  I think it'd be about the same,
> just with cleaner semantics.

Something like the below might work, but it'll need to be done in every 
command, and some of them take a list of files; other ones take a 
directory and a set of files too.

And we'll have to be on the lookout for commands that output relative 
paths when passed relative paths as input (like 'find' does).

Note that if FILE is a relative name, (file-name-directory file) returns 
an unsuitable value.

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 3e6d2a9..e273c6d 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -475,10 +475,12 @@ status of this file.  Otherwise, the value 
returned is one of:
    ;; FIXME: New (sub)states needed (?):
    ;; - `copied' and `moved' (might be handled by `removed' and `added')
    (or (vc-file-getprop file 'vc-state)
-      (when (> (length file) 0)         ;Why??  --Stef
-	(setq backend (or backend (vc-responsible-backend file)))
-	(when backend
-          (vc-state-refresh file backend)))))
+      (let ((default-directory (file-name-directory file))
+            (file (file-relative-name file)))
+        (when (> (length file) 0)         ;Why??  --Stef
+          (setq backend (or backend (vc-responsible-backend file)))
+          (when backend
+            (vc-state-refresh file backend))))))

  (defun vc-state-refresh (file backend)
    "Quickly recompute the `state' of FILE."

Re: Using different default-directory and relative paths in VC, Was: Re: bug#21383

[Stefan Monnier]

>>> Does vc-status get a relative file name as input?
>> Currently, I don't think so.  But that could be changed if needed.
> Hopefully not.
>>> b) how are you going to enforce it?.
>> By checking all callers?
> That's an odd thing to do for an API. What if diff-hl calls vc-state with an
> absolute path? Will it get a slap on the wrist?

Sorry, I didn't realize you were talking about vc-state.  No I guess
vc-state would have to keep working with absolute file names (tho it
could be extended to also accept relative file names).

> in question should work well enough. Although when a command is
> passed a list of files, determining their common parent will require
> more effort.

I'd expect that the caller should be able to easily arrange for each
file in the list to *already* be relative to default-directory, so
there's no need to look for the common parent.


        Stefan

Re: Using different default-directory and relative paths in VC, Was: Re: bug#21383

[Dmitry Gutov]

On 09/08/2015 01:33 AM, Stefan Monnier wrote:

> Sorry, I didn't realize you were talking about vc-state.  No I guess
> vc-state would have to keep working with absolute file names (tho it
> could be extended to also accept relative file names).

It was just an example. Would vc-state keep working with absolute file 
names, but vc-working-revision start only accepting relative ones?

> I'd expect that the caller should be able to easily arrange for each
> file in the list to *already* be relative to default-directory, so
> there's no need to look for the common parent.

If the relative file names start with "../", the result is pointless: we 
still risk calling Git from outside of the repository checkout.

If only some of them start with "../", the result may be subtly 
different, depending on the presence of submodules, I guess, 
sub-repositories combined with ignore files.

Re: Using different default-directory and relative paths in VC, Was: Re: bug#21383

[Dmitry Gutov]

On 09/08/2015 02:29 AM, Dmitry Gutov wrote:

> If only some of them start with "../", the result may be subtly
> different, depending on the presence of submodules, I guess,
                                                     ^
                                                     and
> sub-repositories combined with ignore files.

Re: Using different default-directory and relative paths in VC, Was: Re: bug#21383

[Stefan Monnier]

>> Sorry, I didn't realize you were talking about vc-state.  No I guess
>> vc-state would have to keep working with absolute file names (tho it
>> could be extended to also accept relative file names).
> It was just an example.  Would vc-state keep working with absolute file
> names, but vc-working-revision start only accepting relative ones?

I guess all those functions which are part of the API exposed by VC to
external packages would need to preserve support for absolute file
names, yes.

>> I'd expect that the caller should be able to easily arrange for each
>> file in the list to *already* be relative to default-directory, so
>> there's no need to look for the common parent.
> If the relative file names start with "../", the result is pointless:

I explicitly ruled them out in my earlier definition of "relative file names".


        Stefan