unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* PATCH: make linum.el play nicely with other margin-setting extensions
@ 2015-11-12 12:23 João Távora
  2015-11-12 16:24 ` Eli Zaretskii
  2015-11-13 10:01 ` martin rudalics
  0 siblings, 2 replies; 15+ messages in thread
From: João Távora @ 2015-11-12 12:23 UTC (permalink / raw)
  To: emacs-devel, markus.triska

The problem popped up in darkroom.el which is in ELPA. The margins
aren't correctly in any situation intermingling calls to M-x linum-mode
and M-x darkroom-mode.

This patch to linum.el appears to work nicely, and probably fixes stuff
for other margin-setting extensions, but I'd like a review from the
maintainer or someone else more knowledgeable:

diff --git a/lisp/linum.el b/lisp/linum.el
index 23e5605..776247d 100644
--- a/lisp/linum.el
+++ b/lisp/linum.el
@@ -120,7 +120,13 @@ 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 needed
+    (let ((set-margins (window-parameter w 'linum--set-margins))
+          (current-margins (window-margins w)))
+      (when (and set-margins
+                 (equal set-margins current-margins))
+        (set-window-margins w 0 (cdr current-margins))
+        (set-window-parameter w 'linum--set-margins nil)))))
 
 (defun linum-update-current ()
   "Update line numbers for the current buffer."
@@ -178,7 +184,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, and record that
+    ;; fact as a the window-parameter `linum--set-margins'
+    (let ((existing-margins (window-margins win)))
+      (when (> width (or (car existing-margins) 0))
+        (set-window-margins win width (cdr existing-margins))
+        (set-window-parameter win 'linum--set-margins (window-margins win))))))
 
 (defun linum-after-change (beg end _len)
   ;; update overlays on deletions, and after newlines are inserted


Thanks,
João






^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-12 12:23 PATCH: make linum.el play nicely with other margin-setting extensions João Távora
@ 2015-11-12 16:24 ` Eli Zaretskii
  2015-11-13  8:32   ` João Távora
  2015-11-13 10:01 ` martin rudalics
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-11-12 16:24 UTC (permalink / raw)
  To: João Távora; +Cc: markus.triska, emacs-devel

> From: joaotavora@gmail.com (João Távora)
> Date: Thu, 12 Nov 2015 12:23:02 +0000
> 
> The problem popped up in darkroom.el which is in ELPA. The margins
> aren't correctly in any situation intermingling calls to M-x linum-mode
> and M-x darkroom-mode.
> 
> This patch to linum.el appears to work nicely, and probably fixes stuff
> for other margin-setting extensions, but I'd like a review from the
> maintainer or someone else more knowledgeable:

Thanks.

I'm not familiar with darkroom-mode.  Can you tell in more detail what
problem your patch attempts to solve?




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-12 16:24 ` Eli Zaretskii
@ 2015-11-13  8:32   ` João Távora
  2015-11-13 12:23     ` Juanma Barranquero
  2015-11-14  8:53     ` Eli Zaretskii
  0 siblings, 2 replies; 15+ messages in thread
From: João Távora @ 2015-11-13  8:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: markus.triska, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joaotavora@gmail.com (João Távora)
>> Date: Thu, 12 Nov 2015 12:23:02 +0000
>> 
>> The problem popped up in darkroom.el which is in ELPA. The margins
>> aren't correctly in any situation intermingling calls to M-x linum-mode
>> and M-x darkroom-mode.
>> 
>> This patch to linum.el appears to work nicely, and probably fixes stuff
>> for other margin-setting extensions, but I'd like a review from the
>> maintainer or someone else more knowledgeable:
>
> Thanks.
>
> I'm not familiar with darkroom-mode.  Can you tell in more detail what
> problem your patch attempts to solve?


Sure, darkroom-mode (and many other "writeroom" clones) hide visual
distractions to simulate an 1980's text-processing experience. :-) In
Emacs that means adjusting the left and right margins to center the text
on a narrower section of the window.

So, if you `M-x linum-mode` after `M-x darkroom-mode` you almost
immediately lose the left margin that darkroom-mode set: it now now only
shows the linum line numbers numbers.

Similarly, if you `M-x darkroom-mode` after `M-x linum-mode`,
`darkroom-mode` can't set the margin.

Other problems happen when exiting those modes.

Read on for some explanation regarding the workings of the patch, or
read the patch itself.

linum.el uses the left margin to make just enough to show numbered
indicators. The gist of this patch is: if there is enough space there
already, linum.el won't set the left margin width.

But if it does, it records that fact in a window parameter, to check
later, when turning off linum-mode, if resetting the margin is
needed. In this situation, resetting the margin is only needed if
linum-mode was the only extension to touch it, otherwise one considers
that some other extension or interactive "set-margins" call touched the
margins.

The way I implemented "some other extension ... touched the margins" is
an equal test of current margins vs remembered set margins.

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.

In fact, this pattern of multiple extensions/commands competing for a
Emacs setting is relatively common, I wonder we could abstract some more
generic solution... 


-- 
João Távora



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-12 12:23 PATCH: make linum.el play nicely with other margin-setting extensions João Távora
  2015-11-12 16:24 ` Eli Zaretskii
@ 2015-11-13 10:01 ` martin rudalics
  2015-11-13 11:11   ` João Távora
  1 sibling, 1 reply; 15+ messages in thread
From: martin rudalics @ 2015-11-13 10:01 UTC (permalink / raw)
  To: João Távora, emacs-devel, markus.triska

 >     (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.

Alternatively, we could invent a new window parameter, say
‘left-margin-min-width’, and have packages like ‘linum-mode’ respect it.

martin




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13 10:01 ` martin rudalics
@ 2015-11-13 11:11   ` João Távora
  2015-11-13 14:53     ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: João Távora @ 2015-11-13 11:11 UTC (permalink / raw)
  To: martin rudalics; +Cc: markus.triska, emacs-devel

martin rudalics <rudalics@gmx.at> 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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13  8:32   ` João Távora
@ 2015-11-13 12:23     ` Juanma Barranquero
  2015-11-14  8:53     ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Juanma Barranquero @ 2015-11-13 12:23 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, Markus Triska, Emacs developers

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

On Fri, Nov 13, 2015 at 9:32 AM, João Távora <joaotavora@gmail.com> wrote:

> In fact, this pattern of multiple extensions/commands competing for a
> Emacs setting is relatively common, I wonder we could abstract some more
> generic solution...

Don't know about generic (meaning, not only margins), but it certainly
would help to have a package/library/API/whatever to help packages
coordinate use of the window margins.

[-- Attachment #2: Type: text/html, Size: 562 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13 11:11   ` João Távora
@ 2015-11-13 14:53     ` martin rudalics
  2015-11-13 15:32       ` João Távora
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2015-11-13 14:53 UTC (permalink / raw)
  To: João Távora; +Cc: markus.triska, emacs-devel

 > 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.

I see no problems here.  We can have each mode add its preferred margin
settings as a triple with a minimum, preferred and maximum width for the
left and right margins and some kind of priority.  And do similar things
for the text area, fringes, scrollbars and whatever we want.  The
greatest problem is to handle all these options.

The parameter with the highest priority would specify the preferred
size, the remaining ones constrain minimum and maximum.  And all these
constrained by the total size of the window/frame.

 > 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?

But wasn't the problem that darkroom should respect the minimum width of
‘linum-mode’ and vice-versa?  How would "owning" fit into this?

 > 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

This "before" is problematic when a second or third mode kicks in after
an owner has set its preferred size.

 >          ;; else do nothing
 >          )

And how do we decide prioritization when the "owner" is deactivated and
there are two or more contenders?

martin




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13 14:53     ` martin rudalics
@ 2015-11-13 15:32       ` João Távora
  2015-11-14  8:27         ` martin rudalics
  0 siblings, 1 reply; 15+ messages in thread
From: João Távora @ 2015-11-13 15:32 UTC (permalink / raw)
  To: martin rudalics; +Cc: markus.triska, emacs-devel

On Fri, Nov 13, 2015 at 2:53 PM, martin rudalics <rudalics@gmx.at> wrote:
>> 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.
>
> I see no problems here.  We can have each mode add its preferred margin
> settings as a triple with a minimum, preferred and maximum width for the
> left and right margins and some kind of priority.  And do similar things
> for the text area, fringes, scrollbars and whatever we want.

Yes, it works.

> The greatest problem is to handle all these options.

Obviously, it's a lot new option-related cruft, right?

> The parameter with the highest priority would specify the preferred
> size, the remaining ones constrain minimum and maximum.  And all these
> constrained by the total size of the window/frame.
>
>> 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?
>
> But wasn't the problem that darkroom should respect the minimum width of
> ‘linum-mode’ and vice-versa?  How would "owning" fit into this?

Yes. Did you see the example implementation, which provides the example
you request?

>> 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
>
> This "before" is problematic when a second or third mode kicks in after
> an owner has set its preferred size.

But then it's those other modes that need to be fixed, to follow
whatever their policy is regarding overwriting current settings. Just
like linum.el does in its `linum-update-window'.

The policy there is: "make just the space I need. If I need to change to
change the margins, then I own them, and it's my responsibility to reset
them to what they were before unless someone else takes that
responsibility from me".

    ;; 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)))

And the cleanup code

    ;; 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))))

Let's be clear: My solution is not a panaceia, it doesn't magically
fix all other
packages that are competing for setting a property. But it allows those
packages to adhere to a simple protocol if they see fit, and is
considerably less onerous on new interfaces to offer to said extensions.

> And how do we decide prioritization when the "owner" is deactivated and
> there are two or more contenders?

We don't. Up to the package. Every package has its policy and can decide
to take ownership. A good policy for taking ownership is "I needed to
change it, I take ownership, so I can check later if I should revert
it". But there might be other cases

João



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13 15:32       ` João Távora
@ 2015-11-14  8:27         ` martin rudalics
  2015-11-14 19:55           ` João Távora
  0 siblings, 1 reply; 15+ messages in thread
From: martin rudalics @ 2015-11-14  8:27 UTC (permalink / raw)
  To: João Távora; +Cc: markus.triska, emacs-devel

 >> But wasn't the problem that darkroom should respect the minimum width of
 >> ‘linum-mode’ and vice-versa?  How would "owning" fit into this?
 >
 > Yes. Did you see the example implementation, which provides the example
 > you request?

If you have a "noisy" owner like ‘linum-mode’ that recalculates margin
widths every redisplay cycle, things are easy.  But with a "quiet" owner
that establishes a preferred margin width once only when it gets
started, things are not that easy.  You would have to wake such modes up
somehow every time an owner resigns.

 >> This "before" is problematic when a second or third mode kicks in after
 >> an owner has set its preferred size.
 >
 > But then it's those other modes that need to be fixed, to follow
 > whatever their policy is regarding overwriting current settings. Just
 > like linum.el does in its `linum-update-window'.

If ‘linum-mode’ prevails what should those other modes do?

 > The policy there is: "make just the space I need. If I need to change to
 > change the margins, then I own them, and it's my responsibility to reset
 > them to what they were before unless someone else takes that
 > responsibility from me".

This would work if we assume some stacking order on how modes set up
margins.  In practice there might be no such order.

 >> And how do we decide prioritization when the "owner" is deactivated and
 >> there are two or more contenders?
 >
 > We don't. Up to the package. Every package has its policy and can decide
 > to take ownership. A good policy for taking ownership is "I needed to
 > change it, I take ownership, so I can check later if I should revert
 > it". But there might be other cases

The problem is not that of taking ownership.  It's that of what to do
when "I should revert it".

martin




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-13  8:32   ` João Távora
  2015-11-13 12:23     ` Juanma Barranquero
@ 2015-11-14  8:53     ` Eli Zaretskii
  2015-11-15 14:19       ` João Távora
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2015-11-14  8:53 UTC (permalink / raw)
  To: João Távora; +Cc: markus.triska, emacs-devel

> From: joaotavora@gmail.com (João Távora)
> Date: Fri, 13 Nov 2015 08:32:57 +0000
> Cc: markus.triska@gmx.at, emacs-devel@gnu.org
> 
> linum.el uses the left margin to make just enough to show numbered
> indicators. The gist of this patch is: if there is enough space there
> already, linum.el won't set the left margin width.
> 
> But if it does, it records that fact in a window parameter, to check
> later, when turning off linum-mode, if resetting the margin is
> needed. In this situation, resetting the margin is only needed if
> linum-mode was the only extension to touch it, otherwise one considers
> that some other extension or interactive "set-margins" call touched the
> margins.
> 
> The way I implemented "some other extension ... touched the margins" is
> an equal test of current margins vs remembered set margins.
> 
> 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.

Thanks for the explanations.  Could you please look at nlinum.el (in
ELPA) and see whether its solution to this problem is better, or at
least not worse, and satisfies the needs of darkroom-mode?  If so, I'd
rather we adopted the nlinum solution in linum as well.

If there's something to be done for coexistence with nlinum, I'd
prefer to have a solution that works with both linum and nlinum.

Thanks.




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-14  8:27         ` martin rudalics
@ 2015-11-14 19:55           ` João Távora
  0 siblings, 0 replies; 15+ messages in thread
From: João Távora @ 2015-11-14 19:55 UTC (permalink / raw)
  To: martin rudalics; +Cc: markus.triska, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

> If you have a "noisy" owner like ‘linum-mode’ that recalculates margin
> widths every redisplay cycle, things are easy.

Sure, but notice that my basic scheme makes linum-mode work even with
ignorant modes like darkroom.el.

> But with a "quiet" owner that establishes a preferred margin width
> once only when it gets started, things are not that easy.  You would
> have to wake such modes up somehow every time an owner resigns.

We could wake it up. Or we could patch the other ignorant modes to
follow some ownership-aware logic, perhaps saving what they " found" for
later restoration.

I'm just trying to enable that some modes can follow certain policies
that play slightly more nicely with other modes that have no policy at
all. As more modes have at least some kind ownership-aware policy, we
get better interoperation.

But to allow even this minimalist approach even possible there has to be
some primitive support in Emacs, and that's what I'm proposing. (It
doesn't have to be called "setting owner", by the way, if the subpar
name is what's holding you back).

A more sophisticated API could (probably after a lengthy discussion and
analysis) bring us farther, and can be made compatible to the
"minimalist" scheme I'm proposing. We can certainly start that
discussion: for instance, perhaps this could work.

(defun set-and-own-setting (owner setting object args-fn)
 "whereby ARGS-FN is called the first time to provide args for
  OBJECT'S SETTING and all subsequent time when ownership changes.
  If ARGS-FN returns nil, it means that OWNER relinquishes
  ownership."
  ...)

(defun revert-to-previous-setting (owner setting object)
  "whereby OWNER relinquishes ownership of OBJECT's SETTING
  and SETTING is reverted to the previous owner.")

João






^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-14  8:53     ` Eli Zaretskii
@ 2015-11-15 14:19       ` João Távora
  2015-11-15 19:39         ` Eli Zaretskii
  2015-11-16  9:24         ` Yuri Khan
  0 siblings, 2 replies; 15+ messages in thread
From: João Távora @ 2015-11-15 14:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: markus.triska, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joaotavora@gmail.com (João Távora)
>> Date: Fri, 13 Nov 2015 08:32:57 +0000
>> Cc: markus.triska@gmx.at, emacs-devel@gnu.org
>> 
>> linum.el uses the left margin to make just enough to show numbered
>> indicators. The gist of this patch is: if there is enough space there
>> already, linum.el won't set the left margin width.
>> 
>> But if it does, it records that fact in a window parameter, to check
>> later, when turning off linum-mode, if resetting the margin is
>> needed. In this situation, resetting the margin is only needed if
>> linum-mode was the only extension to touch it, otherwise one considers
>> that some other extension or interactive "set-margins" call touched the
>> margins.
>> 
>> The way I implemented "some other extension ... touched the margins" is
>> an equal test of current margins vs remembered set margins.
>> 
>> 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.
>
> Thanks for the explanations.  Could you please look at nlinum.el (in
> ELPA) and see whether its solution to this problem is better, or at
> least not worse, and satisfies the needs of darkroom-mode?  If so, I'd
> rather we adopted the nlinum solution in linum as well.
>
Just did that. It's Stefan's commit
77c062f41bfe20d5a7a259beeda1c2a26b95f5d0 to elpa.git (curiously not
included in the freshly package-install'ed nlinum.el, but maybe that's
intentional).

It seems basically the same thing as the patch I originally proposed,
only Stefan's a bit more convoluted, so perhaps I'm not reading it
correctly.

Anyway, I say it's "basically the same"" because both patches fail in
the following situation. The reason is that nlinum.el is not as "noisy"
as linum.el (as Martin noticed elsewhere in this thread):

   M-x nlinum-mode   ;; good, line numbers
   M-x darkroom-mode ;; good, line-numbers + darkroom margins
   M-x darkroom-mode ;; oops, no line-numbers OR darkroom margins, even
                     ;; after a bit of scrolling (linum works here)
   C-x 2             ;; change the window configuration, or force
                     ;; nlinum--setup-window somehow, and line-numbers
                     ;; restored

Also notice Stefan's comment

-  ;; FIXME: The interaction between different uses of the margin is
-  ;; problematic.  We should have a way for different packages to indicate (and
-  ;; change) their preference independently.

Meaning that we do need to find a way to p *both* nlinum.el and
darkroom.el to use some kind of ownership-awareness scheme.

I'm discussing such a scheme with Martin (and Juanma). 

João





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-15 14:19       ` João Távora
@ 2015-11-15 19:39         ` Eli Zaretskii
  2015-11-16  9:24         ` Yuri Khan
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2015-11-15 19:39 UTC (permalink / raw)
  To: João Távora; +Cc: markus.triska, emacs-devel

> From: joaotavora@gmail.com (João Távora)
> Cc: markus.triska@gmx.at,  emacs-devel@gnu.org
> Date: Sun, 15 Nov 2015 14:19:59 +0000
> 
> Anyway, I say it's "basically the same"" because both patches fail in
> the following situation. The reason is that nlinum.el is not as "noisy"
> as linum.el (as Martin noticed elsewhere in this thread):
> 
>    M-x nlinum-mode   ;; good, line numbers
>    M-x darkroom-mode ;; good, line-numbers + darkroom margins
>    M-x darkroom-mode ;; oops, no line-numbers OR darkroom margins, even
>                      ;; after a bit of scrolling (linum works here)
>    C-x 2             ;; change the window configuration, or force
>                      ;; nlinum--setup-window somehow, and line-numbers
>                      ;; restored
> 
> Also notice Stefan's comment
> 
> -  ;; FIXME: The interaction between different uses of the margin is
> -  ;; problematic.  We should have a way for different packages to indicate (and
> -  ;; change) their preference independently.
> 
> Meaning that we do need to find a way to p *both* nlinum.el and
> darkroom.el to use some kind of ownership-awareness scheme.

IMO, the question is not whether there are still use cases where such
coexistence would be problematic.  The question is whether the
solution in nlinum solves enough problems to adopt it in linum, and
make the life of darkroom mode and similar modes easier.

If the answer is YES, I'd prefer to use Stefan's solution instead of
inventing yet another one that suffers from the same limitations.

We could (and probably should) discuss further measures to improve the
situation afterwards.  But first there should be no reason for linum
to fail to do at least what nlinum does.  Do you agree?

> I'm discussing such a scheme with Martin (and Juanma). 

Thanks.




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
  2015-11-15 14:19       ` João Távora
  2015-11-15 19:39         ` Eli Zaretskii
@ 2015-11-16  9:24         ` Yuri Khan
       [not found]           ` <CALDnm53Gkm7p2-O-xYf4xMBYPM8sqt2bG5ug-tZu3RKAUE1-gw@mail.gmail.com>
  1 sibling, 1 reply; 15+ messages in thread
From: Yuri Khan @ 2015-11-16  9:24 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, markus.triska, Emacs developers

On Sun, Nov 15, 2015 at 8:19 PM, João Távora <joaotavora@gmail.com> wrote:
> -  ;; FIXME: The interaction between different uses of the margin is
> -  ;; problematic.  We should have a way for different packages to indicate (and
> -  ;; change) their preference independently.
>
> Meaning that we do need to find a way to p *both* nlinum.el and
> darkroom.el to use some kind of ownership-awareness scheme.

Did anyone consider that the proper metaphor may not be ownership but
suballocation? E.g. modes declare how much they want from which side
and with what priority, and the window takes the sums of that. The
margins are then painted from inside outwards, in the order of
decreasing priority. (In this model, “greedy” modes such as darkroom
should have lowest priority so they only get whatever is left over.)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PATCH: make linum.el play nicely with other margin-setting extensions
       [not found]                 ` <CAP_d_8UjQQuM_45dzUWCGL6Y=3vJ+uZayyqSz1k+sAFmLEN-VQ@mail.gmail.com>
@ 2015-11-16 14:49                   ` João Távora
  0 siblings, 0 replies; 15+ messages in thread
From: João Távora @ 2015-11-16 14:49 UTC (permalink / raw)
  To: Yuri Khan, emacs-devel

[I unintentionally took this thread off-list, here's regaining the list again,
hopefully it's enough context]

On Mon, Nov 16, 2015 at 2:32 PM, Yuri Khan <yuri.v.khan@gmail.com> wrote:
> On Mon, Nov 16, 2015 at 7:02 PM, João Távora <joaotavora@gmail.com> wrote:
>
>> Anyway those were just two examples, I can find many more. For example:
>> darkroom-mode turns on visual-line-mode. What should it revert it to when
>> exiting? I say it should revert it to what if found the mode to be on startup,
>> or stay put if the user or some other extension has tweaked the setting, the
>> reasoning here being that darkroom-mode no longer "owns" it.
>
> You’re right, of course, a generic solution might be very convenient.
> It’s just genuinely generic solutions are rare and may conflict with
> specific solutions.
>
> Your logic for visual-line-mode is sound, although I might expect that
> if d-m visibly turns v-l-m on for me and then I manually toggle v-l-m
> off and back on, then I had taken manual control for a while and then
> have given it back to d-m. I will probably still expect it to clean up
> after itself.

If you turn it on and off through an ownership-aware proxy you
probably can still get that. But I wouldn't mind the alternative
"touch it and you're on manual" behaviour.

> This becomes difficult when two modes try to control the
> same setting.

True, though probably a well crafted ownership queue with
a not-too-ambitious contract can probably suffice for most problems.

João



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-11-16 14:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-12 12:23 PATCH: make linum.el play nicely with other margin-setting extensions João Távora
2015-11-12 16:24 ` Eli Zaretskii
2015-11-13  8:32   ` João Távora
2015-11-13 12:23     ` Juanma Barranquero
2015-11-14  8:53     ` Eli Zaretskii
2015-11-15 14:19       ` João Távora
2015-11-15 19:39         ` Eli Zaretskii
2015-11-16  9:24         ` Yuri Khan
     [not found]           ` <CALDnm53Gkm7p2-O-xYf4xMBYPM8sqt2bG5ug-tZu3RKAUE1-gw@mail.gmail.com>
     [not found]             ` <CAP_d_8VPyasNi21JKMRX+Hz55G5-CwtXuwS3FjXx4Ucp3UDKew@mail.gmail.com>
     [not found]               ` <CALDnm50EPe8HCucQEnHPadDHwVmgjN8DmeJ+kvh2Nbtb2tYeZw@mail.gmail.com>
     [not found]                 ` <CAP_d_8UjQQuM_45dzUWCGL6Y=3vJ+uZayyqSz1k+sAFmLEN-VQ@mail.gmail.com>
2015-11-16 14:49                   ` João Távora
2015-11-13 10:01 ` martin rudalics
2015-11-13 11:11   ` João Távora
2015-11-13 14:53     ` martin rudalics
2015-11-13 15:32       ` João Távora
2015-11-14  8:27         ` martin rudalics
2015-11-14 19:55           ` João Távora

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).