GNU bug report logs - #75993
Special mode-class for diff-mode

Previous Next

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

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

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

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Special mode-class for diff-mode
Date: Sat, 01 Feb 2025 19:33:03 +0200

Tags: patch

Visiting a diff file and trying to copy lines with 'w' (diff-kill-ring-save)
does something unexpected since view-mode overrides 'w' with
'View-scroll-page-backward-set-page-size' for non-nil 'view-read-only'.

Here is the fix like for all modes that use single-letter keys:

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 77807fc4f35..b88dd4bf736 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1550,6 +1550,7 @@ diff-mode-read-only
 (defvar whitespace-style)
 (defvar whitespace-trailing-regexp)
 
+(put 'diff-mode 'mode-class 'special)
 ;;;###autoload
 (define-derived-mode diff-mode fundamental-mode "Diff"
   "Major mode for viewing/editing context diffs.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Sun, 02 Feb 2025 18:35:48 -0500

> Visiting a diff file and trying to copy lines with 'w' (diff-kill-ring-save)
> does something unexpected since view-mode overrides 'w' with
> 'View-scroll-page-backward-set-page-size' for non-nil 'view-read-only'.
>
> Here is the fix like for all modes that use single-letter keys:
[...]
> +(put 'diff-mode 'mode-class 'special)
>  ;;;###autoload
>  (define-derived-mode diff-mode fundamental-mode "Diff"
>    "Major mode for viewing/editing context diffs.

Hmm... this doesn't smell right:

- I hate distinguishing between "mode-class = special" and "derives from
  `special-mode`".
- Whether we want to do that depends on `diff-mode-read-only` which is
  buffer-local.


        Stefan

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Mon, 03 Feb 2025 09:56:14 +0200

>> Visiting a diff file and trying to copy lines with 'w' (diff-kill-ring-save)
>> does something unexpected since view-mode overrides 'w' with
>> 'View-scroll-page-backward-set-page-size' for non-nil 'view-read-only'.
>>
>> Here is the fix like for all modes that use single-letter keys:
> [...]
>> +(put 'diff-mode 'mode-class 'special)
>>  ;;;###autoload
>>  (define-derived-mode diff-mode fundamental-mode "Diff"
>>    "Major mode for viewing/editing context diffs.
>
> Hmm... this doesn't smell right:
>
> - I hate distinguishing between "mode-class = special" and "derives from
>   `special-mode`".
> - Whether we want to do that depends on `diff-mode-read-only` which is
>   buffer-local.

AFAICS, the special mode-class is handled only in `read-only-mode`:

   ((and buffer-read-only view-read-only
         (not view-mode)
         (not (eq (get major-mode 'mode-class) 'special)))
    (view-mode-enter))

This is exactly what is needed in diff-mode when it switches to read-only
it should not enable view-mode.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Mon, 03 Feb 2025 06:16:56 -0500

>> Hmm... this doesn't smell right:
>>
>> - I hate distinguishing between "mode-class = special" and "derives from
>>   `special-mode`".
>> - Whether we want to do that depends on `diff-mode-read-only` which is
>>   buffer-local.
>
> AFAICS, the special mode-class is handled only in `read-only-mode`:
>
>    ((and buffer-read-only view-read-only
>          (not view-mode)
>          (not (eq (get major-mode 'mode-class) 'special)))
>     (view-mode-enter))
>
> This is exactly what is needed in diff-mode when it switches to read-only
> it should not enable view-mode.

Maybe, but:

- Nothing in its name or in its doc says so, AFAICT.  IOW, it's just an accident.
- As a symbol property, it's not buffer-local.


        Stefan

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Mon, 03 Feb 2025 19:59:03 +0200

>>> Hmm... this doesn't smell right:
>>>
>>> - I hate distinguishing between "mode-class = special" and "derives from
>>>   `special-mode`".
>>> - Whether we want to do that depends on `diff-mode-read-only` which is
>>>   buffer-local.
>>
>> AFAICS, the special mode-class is handled only in `read-only-mode`:
>>
>>    ((and buffer-read-only view-read-only
>>          (not view-mode)
>>          (not (eq (get major-mode 'mode-class) 'special)))
>>     (view-mode-enter))
>>
>> This is exactly what is needed in diff-mode when it switches to read-only
>> it should not enable view-mode.
>
> Maybe, but:
>
> - Nothing in its name or in its doc says so, AFAICT.  IOW, it's just an accident.
> - As a symbol property, it's not buffer-local.

This description seems to fit the purpose of diff-mode:

   • If this mode is appropriate only for specially-prepared text
     produced by the mode itself (rather than by the user typing at the
     keyboard or by an external file), then the major mode command
     symbol should have a property named ‘mode-class’ with value
     ‘special’, put on as follows:

          (put 'funny-mode 'mode-class 'special)

     This tells Emacs that new buffers created while the current buffer
     is in Funny mode should not be put in Funny mode, even though the
     default value of ‘major-mode’ is ‘nil’.  By default, the value of
     ‘nil’ for ‘major-mode’ means to use the current buffer's major mode
     when creating new buffers (*note Auto Major Mode::), but with such
     ‘special’ modes, Fundamental mode is used instead.  Modes such as
     Dired, Rmail, and Buffer List use this feature.

     The function ‘view-buffer’ does not enable View mode in buffers
     whose mode-class is special, because such modes usually provide
     their own View-like bindings.

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Tue, 04 Feb 2025 21:37:32 +0200

>>>> Hmm... this doesn't smell right:
>>>>
>>>> - I hate distinguishing between "mode-class = special" and "derives from
>>>>   `special-mode`".
>>>> - Whether we want to do that depends on `diff-mode-read-only` which is
>>>>   buffer-local.
>>>
>>> AFAICS, the special mode-class is handled only in `read-only-mode`:
>>>
>>>    ((and buffer-read-only view-read-only
>>>          (not view-mode)
>>>          (not (eq (get major-mode 'mode-class) 'special)))
>>>     (view-mode-enter))
>>>
>>> This is exactly what is needed in diff-mode when it switches to read-only
>>> it should not enable view-mode.
>>
>> Maybe, but:
>>
>> - Nothing in its name or in its doc says so, AFAICT.  IOW, it's just an accident.
>> - As a symbol property, it's not buffer-local.
>
> This description seems to fit the purpose of diff-mode:
>
>    • If this mode is appropriate only for specially-prepared text
>      produced by the mode itself (rather than by the user typing at the
>      keyboard or by an external file), then the major mode command
>      symbol should have a property named ‘mode-class’ with value
>      ‘special’, put on as follows:
>
>           (put 'funny-mode 'mode-class 'special)
>
>      This tells Emacs that new buffers created while the current buffer
>      is in Funny mode should not be put in Funny mode, even though the
>      default value of ‘major-mode’ is ‘nil’.  By default, the value of
>      ‘nil’ for ‘major-mode’ means to use the current buffer's major mode
>      when creating new buffers (*note Auto Major Mode::), but with such
>      ‘special’ modes, Fundamental mode is used instead.  Modes such as
>      Dired, Rmail, and Buffer List use this feature.

I still don't understand implications from the above text.
Dired is read-only by default but can be switched to editable mode,
so is Diff mode.

I tested with special mode-class, and see no negative side effect.

>      The function ‘view-buffer’ does not enable View mode in buffers
>      whose mode-class is special, because such modes usually provide
>      their own View-like bindings.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Tue, 04 Feb 2025 15:52:15 -0500

> I still don't understand implications from the above text.
> Dired is read-only by default but can be switched to editable mode,

Note that when you make it writable, Dired changes the `major-mode`.
Also, AFAIK you can't open a Dired buffer in writable mode.

> so is Diff mode.

In contrast, `diff-mode` can be used both read-only and writable without
changing `major-mode`.

> I tested with special mode-class, and see no negative side effect.

Have you tried to

    (setq diff-default-read-only nil)
    (setq view-read-only t)

and then open a read-only `.patch` file?
I think your patch will hinder
the auto-activation of `view-mode` in that case.


        Stefan

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Thu, 06 Feb 2025 09:58:59 +0200

>> I tested with special mode-class, and see no negative side effect.
>
> Have you tried to
>
>     (setq diff-default-read-only nil)
>     (setq view-read-only t)
>
> and then open a read-only `.patch` file?
> I think your patch will hinder
> the auto-activation of `view-mode` in that case.

Indeed, `view-mode` is not activated in this case,
and this is the right thing to do to allow using
diff-mode single letters like 'w' (diff-kill-ring-save)
instead of overriding it with
'View-scroll-page-backward-set-page-size'.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Thu, 06 Feb 2025 10:39:53 -0500

>>> I tested with special mode-class, and see no negative side effect.
>>
>> Have you tried to
>>
>>     (setq diff-default-read-only nil)
>>     (setq view-read-only t)
>>
>> and then open a read-only `.patch` file?
>> I think your patch will hinder
>> the auto-activation of `view-mode` in that case.
>
> Indeed, `view-mode` is not activated in this case,
> and this is the right thing to do to allow using
> diff-mode single letters like 'w' (diff-kill-ring-save)
> instead of overriding it with
> 'View-scroll-page-backward-set-page-size'.

Hmm... when you `(setq diff-default-read-only nil)`, `w` is not bound to
`diff-kill-ring-save` but to `self-insert-command`, so I think we *do*
want `view-read-only` to apply in this case.


        Stefan

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Thu, 06 Feb 2025 19:17:11 +0200

>>>> I tested with special mode-class, and see no negative side effect.
>>>
>>> Have you tried to
>>>
>>>     (setq diff-default-read-only nil)
>>>     (setq view-read-only t)
>>>
>>> and then open a read-only `.patch` file?
>>> I think your patch will hinder
>>> the auto-activation of `view-mode` in that case.
>>
>> Indeed, `view-mode` is not activated in this case,
>> and this is the right thing to do to allow using
>> diff-mode single letters like 'w' (diff-kill-ring-save)
>> instead of overriding it with
>> 'View-scroll-page-backward-set-page-size'.
>
> Hmm... when you `(setq diff-default-read-only nil)`, `w` is not bound to
> `diff-kill-ring-save` but to `self-insert-command`, so I think we *do*
> want `view-read-only` to apply in this case.

`w` is bound to `diff-kill-ring-save` because of
the read-only file and therefore read-only buffer.

There is no place for view-mode keybindings
since there are only 2 states:

- editable where `w` is `self-insert-command`
- read-only where `w` is `diff-kill-ring-save`

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> linkov.net>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Thu, 06 Feb 2025 13:22:52 -0500

>> Hmm... when you `(setq diff-default-read-only nil)`, `w` is not bound to
>> `diff-kill-ring-save` but to `self-insert-command`, so I think we *do*
>> want `view-read-only` to apply in this case.
> `w` is bound to `diff-kill-ring-save` because of
> the read-only file and therefore read-only buffer.

Hmm... looks like you're right.

So why don't we make `diff-mode` inherit from `special-mode`?


        Stefan

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Thu, 06 Feb 2025 20:49:47 +0200

> So why don't we make `diff-mode` inherit from `special-mode`?

Because it doesn't allow self-inserting keys in editable mode.

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

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 75993 <at> debbugs.gnu.org
Subject: Re: bug#75993: Special mode-class for diff-mode
Date: Sun, 09 Feb 2025 09:40:58 +0200

close 75993 31.0.50
thanks

>> So why don't we make `diff-mode` inherit from `special-mode`?
>
> Because it doesn't allow self-inserting keys in editable mode.

Ok, so now pushed.  The only problem was dependence of
view-mode activation on the symbol property with unrelated name
that is quite confusing.

Previous Next

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