bug#69606: [PATCH] Ensure default-directory exists when generating diff

bug#69606: [PATCH] Ensure default-directory exists when generating diff

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

When I generate a diff in some directory that I would afterwards delete,
the diff buffer appears to remain in the now non-existent directory
(according to `default-directory'), even if I want to generate a diff
for some other file.  The issue now is that make-process complains that
it cannot set the CWD when starting "diff", and instead fails with a
slightly confusing error message:

  start-process: Setting current directory: No such file or directory, /some/directory/that/doesnt/exist/anymore

This would fix the issue, but I don't think the solution is ideal:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] Ensure default-directory exists when generating diff --]
[-- Type: text/x-patch, Size: 1008 bytes --]

From 73a93310a9f38b15a7a849b75b4ea2b3805041ad Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 7 Mar 2024 10:06:48 +0100
Subject: [PATCH] Ensure default-directory exists when generating diff

* lisp/vc/diff.el (diff-no-select): Fall back to the users home
directory if default-directory is not a valid path.
---
 lisp/vc/diff.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/diff.el b/lisp/vc/diff.el
index a64fbc47853..47566dbc40b 100644
--- a/lisp/vc/diff.el
+++ b/lisp/vc/diff.el
@@ -188,7 +188,9 @@ diff-no-select
                                   (list (or old-alt old)
                                         (or new-alt new)))))
 		     " "))
-	 (thisdir default-directory))
+	 (thisdir (if (file-exists-p default-directory)
+                      default-directory
+                    (expand-file-name "~"))))
     (with-current-buffer buf
       (setq buffer-read-only t)
       (buffer-disable-undo (current-buffer))
-- 
2.44.0


[-- Attachment #3: Type: text/plain, Size: 116 bytes --]


Any other ideas?  Perhaps we should always set the default directory to
that 

-- 
	Philip Kaludercic on peregrine

bug#69606: [PATCH] Ensure default-directory exists when generating diff

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Philip Kaludercic <philipk@posteo.net> writes:

Hi Philip,

> diff --git a/lisp/vc/diff.el b/lisp/vc/diff.el
> index a64fbc47853..47566dbc40b 100644
> --- a/lisp/vc/diff.el
> +++ b/lisp/vc/diff.el
> @@ -188,7 +188,9 @@ diff-no-select
>                                    (list (or old-alt old)
>                                          (or new-alt new)))))
>  		     " "))
> -	 (thisdir default-directory))
> +	 (thisdir (if (file-exists-p default-directory)
> +                      default-directory
> +                    (expand-file-name "~"))))
>      (with-current-buffer buf
>        (setq buffer-read-only t)
>        (buffer-disable-undo (current-buffer))

I would use temporary-file-directory (or even
small-temporary-file-directory). Spamming the home directory with
(temporary) diff files doesn't sound like a good idea.

> Any other ideas?  Perhaps we should always set the default directory to
> that

Perhaps, but I don't know whether there are undesired side effects, for
example when using relative file names. OTOH, if default-directory is
remote or unwritable, we might have problems anyway.

Best regards, Michael.

bug#69606: [PATCH] Ensure default-directory exists when generating diff

[Philip Kaludercic]

Michael Albinus <michael.albinus@gmx.de> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> Hi Philip,
>
>> diff --git a/lisp/vc/diff.el b/lisp/vc/diff.el
>> index a64fbc47853..47566dbc40b 100644
>> --- a/lisp/vc/diff.el
>> +++ b/lisp/vc/diff.el
>> @@ -188,7 +188,9 @@ diff-no-select
>>                                    (list (or old-alt old)
>>                                          (or new-alt new)))))
>>  		     " "))
>> -	 (thisdir default-directory))
>> +	 (thisdir (if (file-exists-p default-directory)
>> +                      default-directory
>> +                    (expand-file-name "~"))))
>>      (with-current-buffer buf
>>        (setq buffer-read-only t)
>>        (buffer-disable-undo (current-buffer))
>
> I would use temporary-file-directory (or even
> small-temporary-file-directory). Spamming the home directory with
> (temporary) diff files doesn't sound like a good idea.

If I am not mistaken, the command does not create any files, the output
of diff is written directly into the *diff* buffer.

>> Any other ideas?  Perhaps we should always set the default directory to
>> that
>
> Perhaps, but I don't know whether there are undesired side effects, for
> example when using relative file names. OTOH, if default-directory is
> remote or unwritable, we might have problems anyway.

I don't quite understand how, but it seems that some specific sequence
of commands can start diff in a directory I just deleted, without this
being necessary.  The reason I was thinking about a more general
solution, is that something like

--8<---------------cut here---------------start------------->8---
(let ((default-directory "/this/does/not/exists"))
  (make-process :command '("true")))
--8<---------------cut here---------------end--------------->8---

will always fail, even though "true" doesn't use the current working
directory.  I think it would be useful to have some :fallback option for
these situations, to ensure that if `default-directory' doesn't exist,
any other directory should be used instead.

> Best regards, Michael.

-- 
	Philip Kaludercic on peregrine

bug#69606: [PATCH] Ensure default-directory exists when generating diff

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Philip Kaludercic <philipk@posteo.net> writes:

Hi Philip,

> The reason I was thinking about a more general solution, is that
> something like
>
> (let ((default-directory "/this/does/not/exists"))
>   (make-process :command '("true")))
>
> will always fail, even though "true" doesn't use the current working
> directory.  I think it would be useful to have some :fallback option for
> these situations, to ensure that if `default-directory' doesn't exist,
> any other directory should be used instead.

This is a more general request than just make it work for "diff".

Well, there is a reason that `default-directory' isn't set to something
else behind your back, if it doesn't exist. Processes can use relative
file names as arguments, and it is always better to fail with an error
message instead of doing something unexpected you even don't know about.

Best regards, Michael.