bug#51614: 29.0.50; [PATCH] vc-switches ignore file or directory variables

bug#51614: 29.0.50; [PATCH] vc-switches ignore file or directory variables

[Aleksandr Vityazev]

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


Hello,

The current implementation of the vc-switches function from
lisp/vc/vc.el ignores the values ​​of vc-BACKEND-diff-switches,
vc-diff-switches set as local directory or file variables.

Here's a reproduction:

1. Run "emacs -Q"
2. "C-x C-f" to open file /tmp/test-project/.dir-locals.el
3. Type this into the buffer:
((nil ((vc-git-diff-switches . ("--binary" "--textconv")))))
4. "C-x C-s" to save the buffer
5. "C-x C-f" to open the file /tmp/test-project/test.gpg.
Without --binary and --textconv, diff will not be displayed
for a file with a .gpg extension.
6. Type something in the buffer for example: "Just a test"
7. "C-x C-s" to save the file
8. "C-x v v" to choose VCS backend: in this case, Git
and register the file under VCS
9. "C-x v v" to commit changes
10. Type commit message and "C-c C-c"
11. Type something in the buffer with file /tmp/test-project/test.gpg
12. "C-x C-s" to save the buffer
13. "C-x v =" to show the diff
14. Bug occurs: diff is not displayed.


Perhaps the attached patch can solve the problem.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: lisp/vc/vc.el --]
[-- Type: text/x-patch, Size: 1828 bytes --]

From 9b031423294224a6fe142abf6bd469b5cfa99c83 Mon Sep 17 00:00:00 2001
From: Alexandr Vityazev <avityazev@posteo.org>
Date: Thu, 28 Oct 2021 22:43:28 +0300
Subject: [PATCH] * lisp/vc/vc.el (vc-switches): Handle dir local variables.

* lisp/vc/vc.el (vc-switches): Handle vc-BACKEND-diff-switches,
vc-diff-switches set as local directory or file variables.
---
 lisp/vc/vc.el | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 927a241a60..a858cbc451 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1700,16 +1700,17 @@ diff only, `diff-switches'.
 If the chosen value is not a string or a list, return nil.
 This is so that you may set, e.g. `vc-svn-diff-switches' to t in order
 to override the value of `vc-diff-switches' and `diff-switches'."
-  (let ((switches
-	 (or (when backend
-	       (let ((sym (vc-make-backend-sym
-			   backend (intern (concat (symbol-name op)
-						   "-switches")))))
-		   (when (boundp sym) (symbol-value sym))))
-	     (let ((sym (intern (format "vc-%s-switches" (symbol-name op)))))
-	       (when (boundp sym) (symbol-value sym)))
-	     (cond
-	      ((eq op 'diff) diff-switches)))))
+  (let* ((buffer (other-buffer (current-buffer) t))
+         (switches
+	  (or (when backend
+	        (let ((sym (vc-make-backend-sym
+			    backend (intern (concat (symbol-name op)
+						    "-switches")))))
+		  (when (boundp sym) (buffer-local-value sym buffer))))
+	      (let ((sym (intern (format "vc-%s-switches" (symbol-name op)))))
+	        (when (boundp sym) (buffer-local-value sym buffer)))
+	      (cond
+	       ((eq op 'diff) diff-switches)))))
     (if (stringp switches) (list switches)
       ;; If not a list, return nil.
       ;; This is so we can set vc-diff-switches to t to override
-- 
2.33.1


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


-- 
Best regards,
Aleksandr Vityazev

bug#51614: 29.0.50; [PATCH] vc-switches ignore file or directory variables

[Lars Ingebrigtsen]

Aleksandr Vityazev <avityazev@posteo.org> writes:

> The current implementation of the vc-switches function from
> lisp/vc/vc.el ignores the values ​​of vc-BACKEND-diff-switches,
> vc-diff-switches set as local directory or file variables.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

> Perhaps the attached patch can solve the problem.

[...]

> +  (let* ((buffer (other-buffer (current-buffer) t))
> +         (switches
> +	  (or (when backend
> +	        (let ((sym (vc-make-backend-sym
> +			    backend (intern (concat (symbol-name op)
> +						    "-switches")))))
> +		  (when (boundp sym) (buffer-local-value sym buffer))))
> +	      (let ((sym (intern (format "vc-%s-switches" (symbol-name op)))))
> +	        (when (boundp sym) (buffer-local-value sym buffer)))
> +	      (cond
> +	       ((eq op 'diff) diff-switches)))))

I don't think using `other-buffer' here is a reliable way of getting the
buffer we're interested in here, so I think this would have to be fixed
in a different way.

I'm adding Dmitry to the CCs; perhaps he has some comments here.

bug#51614: 29.0.50; [PATCH] vc-switches ignore file or directory variables

[Dmitry Gutov]

On 10.09.2022 07:43, Lars Ingebrigtsen wrote:
>> +  (let* ((buffer (other-buffer (current-buffer) t))
>> +         (switches
>> +	  (or (when backend
>> +	        (let ((sym (vc-make-backend-sym
>> +			    backend (intern (concat (symbol-name op)
>> +						    "-switches")))))
>> +		  (when (boundp sym) (buffer-local-value sym buffer))))
>> +	      (let ((sym (intern (format "vc-%s-switches" (symbol-name op)))))
>> +	        (when (boundp sym) (buffer-local-value sym buffer)))
>> +	      (cond
>> +	       ((eq op 'diff) diff-switches)))))
> I don't think using `other-buffer' here is a reliable way of getting the
> buffer we're interested in here, so I think this would have to be fixed
> in a different way.

Right.

> I'm adding Dmitry to the CCs; perhaps he has some comments here.

I was thinking vc-switches could have a new, optional argument: buffer. 
But even that seems hard to get ahold of in vc-diff-internal (the FILES 
var might have one element, which is a directory, with no visiting buffers).

Perhaps support for this would need to be added on command-by-command 
basis, with some helper which would collect all buffer-local vars and 
bind them dynamically.

Alternatively, we could support setting 'vc-XX-switches' to a function 
value. That function could take the current default-directory and 
compute the value based on it. I'm not sure if that'll fit Aleksandr's 
workflow, though.