emacs-compat / compat

COMPATibility Library for Emacs Lisp
https://elpa.gnu.org/packages/compat.html
GNU General Public License v3.0
69 stars 12 forks source link

Add Emacs 29's `clean-mode` #27

Closed josephmturner closed 1 year ago

josephmturner commented 1 year ago

The definition uses define-derived-mode, which has no direct correlate in compat-macs.el:

(define-derived-mode clean-mode fundamental-mode "Clean"
  "A mode that removes all overlays and text properties."
  (kill-all-local-variables t)
  (let ((inhibit-read-only t))
    (dolist (overlay (overlays-in (point-min) (point-max)))
      (delete-overlay overlay))
    (set-text-properties (point-min) (point-max) nil)
    (setq-local yank-excluded-properties t)))

Would you use compat-defmacro or maybe compat-defun on the macro-expansion of define-derived-mode?

minad commented 1 year ago

According to the Emacs 29 news:

** New major mode 'clean-mode'.
This is a new major mode meant for debugging.  It kills absolutely all
local variables and removes overlays and text properties.

The mode seems to be supposed to be triggered interactively for debugging only? We avoid to add interactive functionality in Compat, see https://elpa.gnu.org/packages/doc/compat.html#Limitations. So probably this mode is out of scope of Compat. Do you intend to use the mode programmatically in a package of yours? If yes, how do you use it?

phikal commented 1 year ago

Daniel Mendler @.***> writes:

According to the Emacs 29 news:

** New major mode 'clean-mode'.
This is a new major mode meant for debugging.  It kills absolutely all
local variables and removes overlays and text properties.

The mode seems to be supposed to be triggered interactively for debugging only? We avoid to add interactive functionality in Compat, see https://elpa.gnu.org/packages/doc/compat.html#Limitations. So probably this mode is out of scope of Compat. Do you intend to use the mode programmatically in a package of yours? If yes, how do you use it?

Joseph is using this in the new "hyperdrive" package

https://git.sr.ht/~ushin/hyperdrive.el/tree/master/item/hyperdrive-lib.el#L1098

minad commented 1 year ago

(@phikal We just wrote at the same time.)

@josephmturner I've found the relevant mail regarding your hyperdrive package: https://lists.gnu.org/archive/html/emacs-devel/2023-09/msg00300.html

(unless hyperdrive-mode
  ;; Remove overlays/text properties that might be in effect.
  (hyperdrive-clean-mode)
  (when hyperdrive-honor-auto-mode-alist
    (let ((buffer-file-name (hyperdrive-entry-name entry)))
      (set-auto-mode)))
  (hyperdrive-mode))

Maybe rewrite this without relying on clean-mode, since clean-mode may have other unintended side effects (hooks etc)? Also you first turn on clean-mode and then hyperdrive-mode, which seems like an uncommon pattern. I am really not sure about the intended usage of clean-mode. For your usage I'd imagine that a function clean-buffer-overlay-and-properties would be better, instead of the mode.

josephmturner commented 1 year ago

@phikal @minad Thanks for your feedback.

Good point about unintended side effects. For the time being, I'll add a function hyperdrive--clean-buffer-overlay-and-properties, and I'll suggest the addition of clean-buffer-overlay-and-properties on the emacs-devel ML.

minad commented 1 year ago

You're welcome!

I'll suggest the addition of clean-buffer-overlay-and-properties on the emacs-devel ML.

Sure, proposing and discussing it makes sense. I am not sure if adding such a function is worth the costs. It could be that the given code pattern, which removes overlays and properties, is repeated multiple times across the Emacs code base. clean-mode itself doesn't appear to be used at all in Emacs 29, so I believe it is really only for interactive debugging in case you want to completely reset a buffer.

josephmturner commented 1 year ago

I believe it is really only for interactive debugging in case you want to completely reset a buffer.

I agree, especially since clean-mode includes (setq-local yank-excluded-properties t) at the end.