GNU bug report logs - #71604
diff--get-revision-properties calls set-auto-mode twice

Previous Next

Package: emacs;

Reported by: Dmitry Gutov <dmitry <at> gutov.dev>

Date: Mon, 17 Jun 2024 02:26:02 UTC

Severity: normal

Tags: patch

Done: Dmitry Gutov <dmitry <at> gutov.dev>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71604 in the body.
You can then email your comments to 71604 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to juri <at> linkov.net, bug-gnu-emacs <at> gnu.org:
bug#71604; Package emacs. (Mon, 17 Jun 2024 02:26:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Dmitry Gutov <dmitry <at> gutov.dev>:
New bug report received and forwarded. Copy sent to juri <at> linkov.net, bug-gnu-emacs <at> gnu.org. (Mon, 17 Jun 2024 02:26:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: bug-gnu-emacs <at> gnu.org
Subject: diff--get-revision-properties calls set-auto-mode twice
Date: Mon, 17 Jun 2024 05:25:31 +0300
X-Debbugs-Cc: Juri Linkov <juri <at> linkov.net>
Tags: patch

vc-find-revision-no-save calls set-auto-mode, and then we repeat that 
call in diff-syntax-fontify-props.

Which is a problem with CC Mode because it parses the buffer inside the 
major mode function, and ends up doing that twice, at least the first 
time the buffer is initialized before being saved to revision cache. Not 
exactly double the work, but more work nevertheless.

This is a split off from https://debbugs.gnu.org/61667#587. I thought I 
recalled an existing report of vc-diff being slow, but apparently not.

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index c0269d882b9..cc5509ca347 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -2883,9 +2883,6 @@ diff--get-revision-properties
          (buffer (cdr entry)))
     (if (buffer-live-p buffer)
         (progn
-          ;; Don't re-initialize the buffer (which would throw
-          ;; away the previous fontification work).
-          (setq file nil)
           (setq diff--cached-revision-buffers
                 (cons entry
                       (delq entry diff--cached-revision-buffers))))
@@ -2905,7 +2902,8 @@ diff--get-revision-properties
     (diff--cache-schedule-clean)
     (and buffer
          (with-current-buffer buffer
-           (diff-syntax-fontify-props file text line-nb)))))
+           ;; major mode is set in vc-find-revision-no-save already.
+           (diff-syntax-fontify-props nil text line-nb)))))

 (defun diff-syntax-fontify-hunk (beg end old)
   "Highlight source language syntax in diff hunk between BEG and END.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#71604; Package emacs. (Mon, 17 Jun 2024 16:35:02 GMT) Full text and rfc822 format available.

Message #8 received at 71604 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 71604 <at> debbugs.gnu.org
Subject: Re: bug#71604: diff--get-revision-properties calls set-auto-mode twice
Date: Mon, 17 Jun 2024 19:33:03 +0300
> vc-find-revision-no-save calls set-auto-mode, and then we repeat that call
> in diff-syntax-fontify-props.
>
> Which is a problem with CC Mode because it parses the buffer inside the
> major mode function, and ends up doing that twice, at least the first time
> the buffer is initialized before being saved to revision cache. Not exactly
> double the work, but more work nevertheless.
>
> This is a split off from https://debbugs.gnu.org/61667#587. I thought
> I recalled an existing report of vc-diff being slow, but apparently not.
>
> @@ -2883,9 +2883,6 @@ diff--get-revision-properties
>           (buffer (cdr entry)))
>      (if (buffer-live-p buffer)
>          (progn
> -          ;; Don't re-initialize the buffer (which would throw
> -          ;; away the previous fontification work).
> -          (setq file nil)
>            (setq diff--cached-revision-buffers
>                  (cons entry
>                        (delq entry diff--cached-revision-buffers))))
> @@ -2905,7 +2902,8 @@ diff--get-revision-properties
>      (diff--cache-schedule-clean)
>      (and buffer
>           (with-current-buffer buffer
> -           (diff-syntax-fontify-props file text line-nb)))))
> +           ;; major mode is set in vc-find-revision-no-save already.
> +           (diff-syntax-fontify-props nil text line-nb)))))

Thanks, looks correct.




Reply sent to Dmitry Gutov <dmitry <at> gutov.dev>:
You have taken responsibility. (Mon, 17 Jun 2024 20:02:02 GMT) Full text and rfc822 format available.

Notification sent to Dmitry Gutov <dmitry <at> gutov.dev>:
bug acknowledged by developer. (Mon, 17 Jun 2024 20:02:02 GMT) Full text and rfc822 format available.

Message #13 received at 71604-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 71604-done <at> debbugs.gnu.org
Subject: Re: bug#71604: diff--get-revision-properties calls set-auto-mode twice
Date: Mon, 17 Jun 2024 23:01:34 +0300
On 17/06/2024 19:33, Juri Linkov wrote:
>> vc-find-revision-no-save calls set-auto-mode, and then we repeat that call
>> in diff-syntax-fontify-props.
>>
>> Which is a problem with CC Mode because it parses the buffer inside the
>> major mode function, and ends up doing that twice, at least the first time
>> the buffer is initialized before being saved to revision cache. Not exactly
>> double the work, but more work nevertheless.
>>
>> This is a split off from https://debbugs.gnu.org/61667#587. I thought
>> I recalled an existing report of vc-diff being slow, but apparently not.
>>
>> @@ -2883,9 +2883,6 @@ diff--get-revision-properties
>>            (buffer (cdr entry)))
>>       (if (buffer-live-p buffer)
>>           (progn
>> -          ;; Don't re-initialize the buffer (which would throw
>> -          ;; away the previous fontification work).
>> -          (setq file nil)
>>             (setq diff--cached-revision-buffers
>>                   (cons entry
>>                         (delq entry diff--cached-revision-buffers))))
>> @@ -2905,7 +2902,8 @@ diff--get-revision-properties
>>       (diff--cache-schedule-clean)
>>       (and buffer
>>            (with-current-buffer buffer
>> -           (diff-syntax-fontify-props file text line-nb)))))
>> +           ;; major mode is set in vc-find-revision-no-save already.
>> +           (diff-syntax-fontify-props nil text line-nb)))))
> 
> Thanks, looks correct.

Pushed to master, thanks.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Tue, 16 Jul 2024 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 33 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.