Re: PATCH: make linum.el play nicely with other margin-setting extensions

[João Távora]

martin rudalics <address@hidden> writes:

>>     (mapc #'delete-overlay linum-overlays)
>>     (setq linum-overlays nil)
>>     (dolist (w (get-buffer-window-list (current-buffer) nil t))
>> -    (set-window-margins w 0 (cdr (window-margins w)))))
>> +    ;; restore margins if needed
>> +    (let ((set-margins (window-parameter w 'linum--set-margins))
>> +          (current-margins (window-margins w)))
>> +      (when (and set-margins
>> +                 (equal set-margins current-margins))
>
> I suppose this will reset the left margin to zero when the "other mode"
> was started after ‘linum-mode’ and incidentally used the same margin
> width as ‘linum-mode’.  Maybe nothing reasonable can be done about that.

Absolutely. That's what I meant earlier by 

>> It seems robust enough for now, but ideally linum-mode would know that
>> "his" margins have been overriden interactively or by some other
>> extension, and decide in accordance.

Though if we come up with `left-margin-min-width' we'll probably have to
come up with some `max-width' version some time in the future, no? And
so on for every other window-related property. And so on for every
frame-related property while we're at it.

Woundn't it be nicer that a package/mode, when it so sees fit, could
mark a particular window|frame|buffer setting as being "owned" by it and
then query arbitrarily later if it still owns it?

Clearly if *all* extensions played by these rules we would be out of the
woods: the test would become

    (if (eq (get-setting-owner 'set-window-margins w) 'linum)
        ;; reset the margins to whatever was there before
        ;; else do nothing
        )

The only difficulty here is that we can't magically make all extensions
abide by this rule. But we can advise 'set-window-margins to reset the
owner unconditionally. What do you think of this?

    (defvar *setting-owners* (make-hash-table :test #'equal))
     
    (cl-defgeneric get-setting-owner (setting-setter object)
      "Return the proclaimed owner of SETTING-SETTER in OBJECT.")
     
    (cl-defgeneric set-setting-owner (setting-setter object owner)
      "Return the proclaimed owner of SETTING-SETTER in OBJECT.")
     
    (defmacro define-owned-setting (setting-setter)
      "Make the function SETTING-SETTER reset the owner when called "
      `(progn
         (advice-add ',setting-setter :after
                     (lambda (object &rest _ignored)
                       (set-setting-owner ',setting-setter object nil)))
         (cl-defmethod get-setting-owner ((setting-setter (eql 
,setting-setter)) o)
           (gethash (list setting-setter o) *setting-owners*))
         (cl-defmethod set-setting-owner ((setting-setter (eql 
,setting-setter)) o owner)
           (if owner
               (puthash (list setting-setter o) owner *setting-owners*)
             (remhash (list setting-setter o) *setting-owners*)))))
     
    (define-owned-setting set-window-margins)

With this in place, my patch to linum.el becomes smaller, more robust,
and more readable. Patching other extensions that compete for other
window|frame|buffer|whatever resources also becomes easier.

The smaller, correct patch to linum.el follows after the sign-off.

João

diff --git a/lisp/linum.el b/lisp/linum.el
index 23e5605..4346d81 100644
--- a/lisp/linum.el
+++ b/lisp/linum.el
@@ -120,7 +120,10 @@ Linum mode is a buffer-local minor mode."
   (mapc #'delete-overlay linum-overlays)
   (setq linum-overlays nil)
   (dolist (w (get-buffer-window-list (current-buffer) nil t))
-    (set-window-margins w 0 (cdr (window-margins w)))))
+    ;; restore margins if we still own that setting
+    ;; 
+    (if (eq (get-setting-owner 'set-window-margins w) 'linum)
+        (set-window-margins w 0 (cdr (window-margins w))))))
 
 (defun linum-update-current ()
   "Update line numbers for the current buffer."
@@ -178,7 +181,12 @@ Linum mode is a buffer-local minor mode."
       (let ((inhibit-point-motion-hooks t))
         (forward-line))
       (setq line (1+ line)))
-    (set-window-margins win width (cdr (window-margins win)))))
+    ;; open up space in the left margin, if needed
+    ;; 
+    (let ((existing-margins (window-margins win)))
+      (when (> width (or (car existing-margins) 0))
+        (set-window-margins win width (cdr existing-margins))
+        (set-setting-owner 'set-window-margins win 'linum)))))
 
 (defun linum-after-change (beg end _len)
   ;; update overlays on deletions, and after newlines are inserted