Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Stefan Monnier]

Hi Alan,

I have a few questions abut your change.

>     * lisp/progmodes/compile.el (compilation-context-lines): Add the new value t.
>     (compilation-set-window): Amend to handle compilation-context-lines being t.
>     (overlay-arrow-overlay): New variable holding an overlay with before-string
>     property "=>".

1- Why `overlay-arrow-overlay` without a `compilation-` prefix?
2- Why insert a prefix string (an "inserted arrow") instead of using
   a "regular overwriting arrow"?
3- Why link the choice of inserted-arrow vs regular-overwriting-arrow to
   compilation-context-lines specifically?


-- Stefan "maybe part of my confusion is that I'm not sufficiently
           familiar with the behavior when there's no left-fringe"

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Stefan.

On Sun, Aug 25, 2019 at 14:39:46 -0400, Stefan Monnier wrote:
> Hi Alan,

> I have a few questions abut your change.

> >     * lisp/progmodes/compile.el (compilation-context-lines): Add the new value t.
> >     (compilation-set-window): Amend to handle compilation-context-lines being t.
> >     (overlay-arrow-overlay): New variable holding an overlay with before-string
> >     property "=>".

> 1- Why `overlay-arrow-overlay` without a `compilation-` prefix?

It goes together with overlay-arrow-string, and overlay-arrow-position.

> 2- Why insert a prefix string (an "inserted arrow") instead of using
>    a "regular overwriting arrow"?

Because the overwriting arrow would obliterate the first two characters
of the file name.  I actually tried this first, and it wasn't
satisfactory.  This contrasts with another use of the overwriting arrow
in edebug, where (usually) only WS gets overwritten, and it is important
not to disturb the visible indentation.

> 3- Why link the choice of inserted-arrow vs regular-overwriting-arrow to
>    compilation-context-lines specifically?

Simplicity: mainly confusion on my part as just to where all the bits of
the new code would have to go if the "insertion" arrow were to be made a
general feature.  If the new mechanism is a success, it should be easier
then to make it more general.

> -- Stefan "maybe part of my confusion is that I'm not sufficiently
>            familiar with the behavior when there's no left-fringe"

That behaviour involves scrolling the current line to the top of the
buffer, and several years of that had exceeded my irritation threshold.
I usually run on a Linux tty, where there's no possibility of a fringe.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Sun, 25 Aug 2019 19:06:37 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: emacs-devel@gnu.org
> 
> > 2- Why insert a prefix string (an "inserted arrow") instead of using
> >    a "regular overwriting arrow"?
> 
> Because the overwriting arrow would obliterate the first two characters
> of the file name.  I actually tried this first, and it wasn't
> satisfactory.  This contrasts with another use of the overwriting arrow
> in edebug, where (usually) only WS gets overwritten, and it is important
> not to disturb the visible indentation.

Did you consider to use the display margin instead?  You can have an
overlay display in the margin by putting a 'display' property on the
overlay string.  The advantage of using the margin is that it will
keep the buffer text aligned.

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Eli.

On Sun, Aug 25, 2019 at 22:37:39 +0300, Eli Zaretskii wrote:
> > Date: Sun, 25 Aug 2019 19:06:37 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: emacs-devel@gnu.org

> > > 2- Why insert a prefix string (an "inserted arrow") instead of using
> > >    a "regular overwriting arrow"?

> > Because the overwriting arrow would obliterate the first two characters
> > of the file name.  I actually tried this first, and it wasn't
> > satisfactory.  This contrasts with another use of the overwriting arrow
> > in edebug, where (usually) only WS gets overwritten, and it is important
> > not to disturb the visible indentation.

> Did you consider to use the display margin instead?  You can have an
> overlay display in the margin by putting a 'display' property on the
> overlay string.  The advantage of using the margin is that it will
> keep the buffer text aligned.

Yes, indeed.  Thanks!  I wasn't actually aware of this facility (despite
seeing it many times in gdb).  As soon as I get my head around the
documentation (using the margin seems to be a bit twisted, somehow), I'll
amend the code to use it.

Should I have been aware of this?  Somehow, in the elisp manual, it seems
buried under several levels of chapters, sections, and sub-sections.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Mon, 26 Aug 2019 16:26:51 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> Should I have been aware of this?

What is "this" in this context?  The display margins or how to use
them with overlays?

> Somehow, in the elisp manual, it seems buried under several levels
> of chapters, sections, and sub-sections.

Well, Emacs is a large program, and display features in particular are
a legion...

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Eli.

On Mon, Aug 26, 2019 at 19:29:16 +0300, Eli Zaretskii wrote:
> > Date: Mon, 26 Aug 2019 16:26:51 +0000
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Should I have been aware of this?

> What is "this" in this context?  The display margins or how to use
> them with overlays?

I think, more the first of these.

> > Somehow, in the elisp manual, it seems buried under several levels
> > of chapters, sections, and sub-sections.

> Well, Emacs is a large program, and display features in particular are
> a legion...

I found implementing these margins (see patch in separate post) to be
particularly difficult.  There is no complete example of how to do this
on the "Display Margins" elisp manual page.  There, the word "property"
is used, rather than "text property or overlay property", leaving me
uncertain of whether I could have implemented this "=>" in the margin
solely with overlays.  I was uncertain, right up till near when I had
the thing ready, whether the variables implementing this need to be
buffer local or not.  (It also didn't help that I tried at first to put
the `display' text properties on overlay-arrow-string, which, of course,
can't work.)

Also, the setup seems wierd.  Why isn't there a direct `margin' overlay
property, much the same way there's a `before-string' overlay property?
Instead, what we have is a required "dummy" `before-string', which is
supplanted by a `display' text property on it.  It just seems a strange
way of implementing it.  This certainly doesn't promote easy
understanding.  IMHO, of course.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Tue, 27 Aug 2019 20:05:20 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > Should I have been aware of this?
> 
> > What is "this" in this context?  The display margins or how to use
> > them with overlays?
> 
> I think, more the first of these.
> 
> > > Somehow, in the elisp manual, it seems buried under several levels
> > > of chapters, sections, and sub-sections.

The subsection describing the display margins is part of the section
which documents the 'display' properties in general, so I think it's
in a correct and logical place.

Where would you expect it to be mentioned, not to be "buried"?

> I found implementing these margins (see patch in separate post) to be
> particularly difficult.  There is no complete example of how to do this
> on the "Display Margins" elisp manual page.

Feel free to provide an example.  The simplest way, one that doesn't
use an overlay string, is just setting a display property on buffer
text, something that hardly needs an example.

> There, the word "property" is used, rather than "text property or
> overlay property", leaving me uncertain of whether I could have
> implemented this "=>" in the margin solely with overlays.

This is a subsection of a section about 'display' property, which is
only available for text in buffers and Lisp strings, not for overlays.
One should read the manual in the context of the parent sections.

> I was uncertain, right up till near when I had the thing ready,
> whether the variables implementing this need to be buffer local or
> not.

Which variables?  And why did you think they need to be buffer-local?
Text properties already localize their values enough, so there should
be no need for anything buffer-local.  That's in general, not only for
the display properties.

> Also, the setup seems wierd.  Why isn't there a direct `margin' overlay
> property, much the same way there's a `before-string' overlay property?

Because no one wrote the code to do that.  This is a display property,
so you can put it on buffer text or on a Lisp string, in particular on
a Lisp string that is before- or after-string of an overlay.

> Instead, what we have is a required "dummy" `before-string', which is
> supplanted by a `display' text property on it.  It just seems a strange
> way of implementing it.  This certainly doesn't promote easy
> understanding.  IMHO, of course.

Display properties (really, any "replacing" properties) are tricky in
implementation, you should look up the code some day.  Then you will
maybe understand why no one ever wanted to add one more such
facility.  The way of using the display properties to display in the
margins was originally implemented only for buffer text; placing it on
overlay strings had bugs until Emacs 24 or 25, I think.

In any case, the ELisp manual describes this possibility, precisely
because it is not easy to come up on one's own.  So I think we are OK
here, documentation-wise.

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Eli.

On Thu, Aug 29, 2019 at 21:22:06 +0300, Eli Zaretskii wrote:
> > Date: Tue, 27 Aug 2019 20:05:20 +0000
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > Should I have been aware of this?

> > > What is "this" in this context?  The display margins or how to use
> > > them with overlays?

> > I think, more the first of these.

> > > > Somehow, in the elisp manual, it seems buried under several levels
> > > > of chapters, sections, and sub-sections.

> The subsection describing the display margins is part of the section
> which documents the 'display' properties in general, so I think it's
> in a correct and logical place.

> Where would you expect it to be mentioned, not to be "buried"?

Maybe on the page "Emacs Display", rather than the current menu point:

    * Display Property      Enabling special display features.

, something like:

    * Display Property      Images, margins, text size, etc.

might be more helpful.

> > I found implementing these margins (see patch in separate post) to be
> > particularly difficult.  There is no complete example of how to do this
> > on the "Display Margins" elisp manual page.

> Feel free to provide an example.  The simplest way, one that doesn't
> use an overlay string, is just setting a display property on buffer
> text, something that hardly needs an example.

This would involve that text not getting displayed.  I'm not sure doing
that would ever be useful - if such a display text property were put on
the first character of a line, and that line were empty, it would
(presumably) make that line disappear from the display.

I will get an example together, but at the moment Real Life is somewhat
stressful.  Maybe I could base it on the margin I'm putting into
fringeless windows in compilation-mode.

> > There, the word "property" is used, rather than "text property or
> > overlay property", leaving me uncertain of whether I could have
> > implemented this "=>" in the margin solely with overlays.

> This is a subsection of a section about 'display' property, which is
> only available for text in buffers and Lisp strings, not for overlays.
> One should read the manual in the context of the parent sections.

I was floundering around in confusion for several hours, trying to get
some concrete handle on how to code the margins.  If the sentence in
"Display Margins" had read:

    The way to display something in the margins is to specify it in a
    margin display specification in the `display' TEXT property of some
    text.
                                                  ^^^^

, this would have helped me.

> > I was uncertain, right up till near when I had the thing ready,
> > whether the variables implementing this need to be buffer local or
> > not.

> Which variables?

compilation-margin-string ;; the "=>" which appears in the margin
compilation--dummy-string ;; the before-string which has a `display' text
                          ;; property for the margin.
compilation-arrow-overlay ;; The overlay causing the "=>" to be displayed
                          ;; at the appropriate line in the window.

Of these, only the last needs to be buffer local, since there might be
several compilation-mode windows live at the same time.  Really it ought
to be a "window local variable", but we don't have these.  That's
probably a good thing.

> And why did you think they need to be buffer-local?  Text properties
> already localize their values enough, so there should be no need for
> anything buffer-local.  That's in general, not only for the display
> properties.

Yes.  I think I've got that, now.

> > Also, the setup seems wierd.  Why isn't there a direct `margin' overlay
> > property, much the same way there's a `before-string' overlay property?

> Because no one wrote the code to do that.

That's a good answer.  Thanks!

> This is a display property, so you can put it on buffer text or on a
> Lisp string, in particular on a Lisp string that is before- or
> after-string of an overlay.

> > Instead, what we have is a required "dummy" `before-string', which is
> > supplanted by a `display' text property on it.  It just seems a strange
> > way of implementing it.  This certainly doesn't promote easy
> > understanding.  IMHO, of course.

> Display properties (really, any "replacing" properties) are tricky in
> implementation, you should look up the code some day.  Then you will
> maybe understand why no one ever wanted to add one more such
> facility.

Yes.  I spent quite a bit of time a couple of weeks ago getting my head
around the code in xdisp.c for before-strings and after-strings.  Maybe
adding margins into the mix wouldn't really be a good idea.  ;-)

> The way of using the display properties to display in the margins was
> originally implemented only for buffer text; placing it on overlay
> strings had bugs until Emacs 24 or 25, I think.

Ah.

> In any case, the ELisp manual describes this possibility, precisely
> because it is not easy to come up on one's own.  So I think we are OK
> here, documentation-wise.

As above, I think a complete example in the "Display Margins" page would
be helpful.  I'll get around to formulating this some time (soon?).

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Sat, 31 Aug 2019 10:53:16 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > The subsection describing the display margins is part of the section
> > which documents the 'display' properties in general, so I think it's
> > in a correct and logical place.
> 
> > Where would you expect it to be mentioned, not to be "buried"?
> 
> Maybe on the page "Emacs Display", rather than the current menu point:
> 
>     * Display Property      Enabling special display features.
> 
> , something like:
> 
>     * Display Property      Images, margins, text size, etc.
> 
> might be more helpful.

We could perhaps mention it in some higher-level section, but moving
this subsection up two levels is too much, I think.

> As above, I think a complete example in the "Display Margins" page would
> be helpful.  I'll get around to formulating this some time (soon?).

Thanks.

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Eli.

On Sat, Aug 31, 2019 at 14:06:30 +0300, Eli Zaretskii wrote:
> > Date: Sat, 31 Aug 2019 10:53:16 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org

> > > The subsection describing the display margins is part of the section
> > > which documents the 'display' properties in general, so I think it's
> > > in a correct and logical place.

> > > Where would you expect it to be mentioned, not to be "buried"?

> > Maybe on the page "Emacs Display", rather than the current menu point:

> >     * Display Property      Enabling special display features.

> > , something like:

> >     * Display Property      Images, margins, text size, etc.

> > might be more helpful.

> We could perhaps mention it in some higher-level section, but moving
> this subsection up two levels is too much, I think.

What I've proposed is purely changing the caption on the menu point, not
moving any section text.

I think that the text "Enabling special display features" is
frustratingly vague, repetitive and uninformative.  "Images, margins,
text size, etc.", by contrast, gives the reader reason to follow the menu
point.

> > As above, I think a complete example in the "Display Margins" page would
> > be helpful.  I'll get around to formulating this some time (soon?).

> Thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Mon, 2 Sep 2019 19:34:05 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > Maybe on the page "Emacs Display", rather than the current menu point:
> 
> > >     * Display Property      Enabling special display features.
> 
> > > , something like:
> 
> > >     * Display Property      Images, margins, text size, etc.
> 
> > > might be more helpful.
> 
> > We could perhaps mention it in some higher-level section, but moving
> > this subsection up two levels is too much, I think.
> 
> What I've proposed is purely changing the caption on the menu point, not
> moving any section text.

That's fine with me.

Margins example in the Elisp manual. [Was: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling]

[Alan Mackenzie]

Hello, Eli.

On Sat, Aug 31, 2019 at 14:06:30 +0300, Eli Zaretskii wrote:
> > Date: Sat, 31 Aug 2019 10:53:16 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org

[ .... ]

> > As above, I think a complete example in the "Display Margins" page would
> > be helpful.  I'll get around to formulating this some time (soon?).

> Thanks.

OK, here's a first draught of the example, based on the emacs-26 branch.
As always, comments and criticism are welcome.



diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 55a0a2f924..f2379cc8c7 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -4956,6 +4956,64 @@ Display Margins
 selected window is used.
 @end defun
 
+@menu
+* Margin Example::              A working example of the use of margins.
+@end menu
+
+@node Margin Example
+@subsubsection Margin Example
+@cindex margin example
+@findex foo-mode
+
+The following incomplete, but working, minor mode illustrates one way
+of using margins in your code (@pxref{Display Margins}).  To see it
+working, load the code into your Emacs, then toggle the minor mode on
+with @kbd{M-x foo-mode @key{RET}} in some window.  You will see a two
+character left margin containing the string ''=>'' on the line
+containing point.
+
+@example
+(defvar-local foo-arrow-overlay nil
+  "Overlay with the before-string property of `foo-dummy-string'.
+
+When non-nil, this overlay causes redisplay to display `foo-margin-string'
+at the overlay's start position.")
+
+(defconst foo-margin-string
+  (propertize "=>" 'face 'default)
+  "The string which will appear in the margin in foo mode.")
+
+(defconst foo-dummy-string
+  (propertize ">" 'display
+              `((margin left-margin) ,foo-margin-string))
+  "A string which is only a placeholder for foo-margin-string.
+Actual value is never used, only the text property.")
+
+(defun foo-post-command-hook ()
+  (when foo-arrow-overlay
+    (move-overlay foo-arrow-overlay
+                  (line-beginning-position) (line-beginning-position))))
+(add-hook 'post-command-hook #'foo-post-command-hook)
+
+(defvar-local foo-mode nil)
+(defun foo-mode (&optional arg)
+  "Minor mode to indicate current line by a \"=>\" in the margin."
+  (interactive "P")
+  (setq foo-mode
+        (cond
+         ((null arg) (not foo-mode))
+         ((> arg 0) t)
+         (t nil)))
+  (if foo-mode
+      (progn
+        (setq foo-arrow-overlay (make-overlay (point) (point)))
+        (overlay-put foo-arrow-overlay 'before-string foo-dummy-string)
+        (set-window-margins (selected-window) (+ (or (car (window-margins)) 0) 2)))
+    (delete-overlay foo-arrow-overlay)
+    (setq foo-arrow-overlay nil)
+    (set-window-margins (selected-window) (- (car (window-margins)) 2))))
+@end example
+
 @node Images
 @section Images
 @cindex images in buffers


-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Margins example in the Elisp manual. [Was: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling]

[Eli Zaretskii]

> Date: Sun, 8 Sep 2019 09:41:56 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > As above, I think a complete example in the "Display Margins" page would
> > > be helpful.  I'll get around to formulating this some time (soon?).
> 
> > Thanks.
> 
> OK, here's a first draught of the example, based on the emacs-26 branch.
> As always, comments and criticism are welcome.

Looks significantly larger than what I had in mind.  I thought you
were talking about a simpler example that just shows how to display a
Lisp string in the margin, not necessarily an entire mode.

If you think only such a large example will do, please make it
conditioned by @ifnottex (and be sure to do the same with the menu
item in @detailmenu, which you omitted in this patch).

Thanks.

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Eli and Stefan.

On Sun, Aug 25, 2019 at 22:37:39 +0300, Eli Zaretskii wrote:
> > Date: Sun, 25 Aug 2019 19:06:37 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: emacs-devel@gnu.org

> > > 2- Why insert a prefix string (an "inserted arrow") instead of using
> > >    a "regular overwriting arrow"?

> > Because the overwriting arrow would obliterate the first two characters
> > of the file name.  I actually tried this first, and it wasn't
> > satisfactory.  This contrasts with another use of the overwriting arrow
> > in edebug, where (usually) only WS gets overwritten, and it is important
> > not to disturb the visible indentation.

> Did you consider to use the display margin instead?  You can have an
> overlay display in the margin by putting a 'display' property on the
> overlay string.  The advantage of using the margin is that it will
> keep the buffer text aligned.

OK, thanks for telling me about this.  I propose installing the following
patch which implements the arrow in the margin (and supplements my
earlier commit) into master.  Criticism, as always, will be welcomed.




diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 09188dc14b..931f66ef03 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2602,45 +2602,70 @@ compilation-set-window
 			       (point)))
          (set-window-point w mk))))
 
-(defvar-local overlay-arrow-overlay nil
+(defvar-local compilation-arrow-overlay nil
   "Overlay with the before-string property of `overlay-arrow-string'.
 
 When non-nil, this overlay causes redisplay to display `overlay-arrow-string'
 at the overlay's start position.")
 
+(defvar compilation-margin-string "=>"
+  "The string which will appear in the margin in compilation mode.
+This must be two characters long; there should be no need to
+change the default.")
+(put-text-property 0 2 'face 'default compilation-margin-string)
+
+(defvar compilation-dummy-string ">"
+  "A string which is only a placeholder for compilation-margin-string.
+It's actual value is never used, but must be one character long.")
+(put-text-property 0 1 'display
+                   `((margin left-margin) ,compilation-margin-string)
+                   compilation-dummy-string)
+
+(defun compilation-set-up-arrow-spec-in-margin ()
+  "Set up compilation-arrow-overlay to display as an arrow in a margin."
+  (setq overlay-arrow-string "")
+  (overlay-put compilation-arrow-overlay
+               'before-string compilation-dummy-string)
+  (set-window-margins (selected-window) 2))
+
+(defun compilation-tear-down-arrow-spec-in-margin ()
+  "Restore compilation-arrow-overlay to not using the margin, which is removed."
+  (overlay-put compilation-arrow-overlay 'before-string nil)
+  (delete-overlay compilation-arrow-overlay)
+  (setq compilation-arrow-overlay nil)
+  (set-window-margins (selected-window) 0))
+
 (defun compilation-set-overlay-arrow (w)
   "Set up, or switch off, the overlay-arrow for window W."
-  (with-current-buffer (window-buffer w)
-    (if (and (eq compilation-context-lines t)
-             (equal (car (window-fringes w)) 0)) ; No left fringe
-        ;; Insert a "=>" before-string overlay at the beginning of the
-        ;; line pointed to by `overlay-arrow-position'.
-        (cond
-         ((overlayp overlay-arrow-overlay)
-          (when (not (eq (overlay-start overlay-arrow-overlay)
-		         overlay-arrow-position))
-	    (if overlay-arrow-position
-                (progn
-	          (move-overlay overlay-arrow-overlay
-			        overlay-arrow-position overlay-arrow-position)
-                  (setq overlay-arrow-string "=>")
-                  (overlay-put overlay-arrow-overlay
-                               'before-string overlay-arrow-string))
-	      (delete-overlay overlay-arrow-overlay)
-	      (setq overlay-arrow-overlay nil))))
-
-         (overlay-arrow-position
-          (setq overlay-arrow-overlay
-	        (make-overlay overlay-arrow-position overlay-arrow-position))
-          (setq overlay-arrow-string "=>")
-          (overlay-put overlay-arrow-overlay 'before-string overlay-arrow-string)))
-
-      ;; `compilation-context-lines' isn't t, or we've got a left
-      ;; fringe, so remove any overlay arrow.
-      (when (overlayp overlay-arrow-overlay)
-        (setq overlay-arrow-string "")
-        (delete-overlay overlay-arrow-overlay)
-        (setq overlay-arrow-overlay nil)))))
+  (with-selected-window w        ; So the later `goto-char' will work.
+    (with-current-buffer (window-buffer w)
+      (if (and (eq compilation-context-lines t)
+               (equal (car (window-fringes w)) 0)) ; No left fringe
+          ;; Insert a before-string overlay at the beginning of the line
+          ;; pointed to by `overlay-arrow-position', such that it will
+          ;; display in a 2-character margin.
+          (progn
+            (cond
+             ((overlayp compilation-arrow-overlay)
+              (when (not (eq (overlay-start compilation-arrow-overlay)
+		             overlay-arrow-position))
+	        (if overlay-arrow-position
+	            (move-overlay compilation-arrow-overlay
+			          overlay-arrow-position overlay-arrow-position)
+                  (compilation-tear-down-arrow-spec-in-margin))))
+
+             (overlay-arrow-position
+              (setq compilation-arrow-overlay
+	            (make-overlay overlay-arrow-position overlay-arrow-position))
+              (compilation-set-up-arrow-spec-in-margin)))
+            ;; Ensure that the "=>" remains in the window by causing
+            ;; the window to be scrolled, if needed.
+            (goto-char (overlay-start compilation-arrow-overlay)))
+
+        ;; `compilation-context-lines' isn't t, or we've got a left
+        ;; fringe, so remove any overlay arrow.
+        (when (overlayp compilation-arrow-overlay)
+          (compilation-tear-down-arrow-spec-in-margin))))))
 
 (defvar next-error-highlight-timer)
 

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Eli Zaretskii]

> Date: Tue, 27 Aug 2019 19:36:52 +0000
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> +(defvar compilation-dummy-string ">"
> +  "A string which is only a placeholder for compilation-margin-string.
> +It's actual value is never used, but must be one character long.")
   ^^^^
"Its".

Btw, it is customary to use just " " for such purposes.

Thanks.

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Stefan Monnier]

>> +(defvar compilation-dummy-string ">"
>> +  "A string which is only a placeholder for compilation-margin-string.
>> +It's actual value is never used, but must be one character long.")
>    ^^^^
> "Its".
>
> Btw, it is customary to use just " " for such purposes.

Funnily enough I thought you were talking not about the ">" string but
about the "It[']s" so I was surprised you said " " instead of "" since
indeed you could write it as just:

    Actual value is never used, but must be one character long.

and I was wondering why you'd want to use a leading space in
that sentence ;-O


        Stefan

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Stefan Monnier]

Hi Alan,

> +(defvar compilation-dummy-string ">"
> +  "A string which is only a placeholder for compilation-margin-string.
> +It's actual value is never used, but must be one character long.")
> +(put-text-property 0 1 'display
> +                   `((margin left-margin) ,compilation-margin-string)
> +                   compilation-dummy-string)

I'd suggest you use a "compilation--" prefix here since this is supposed
to be internal, IIUC.

Also, I'd do it as

    (defvar compilation--dummy-string
      (propertize ">" 'display
                  `((margin left-margin) ,compilation-margin-string))
      "A string which is only a placeholder for `compilation-margin-string'.
    Its actual value is never used, but must be one character long.")

so you can M-C-x it without ill-effect [note I also fixed "It's" to
"Its" and added `...' around the var ref.  ]

> +(defun compilation-tear-down-arrow-spec-in-margin ()
> +  "Restore compilation-arrow-overlay to not using the margin, which is removed."
> +  (overlay-put compilation-arrow-overlay 'before-string nil)
> +  (delete-overlay compilation-arrow-overlay)
> +  (setq compilation-arrow-overlay nil)

I think this `setq` loses the overlay and there's no code to reconstruct
it later on.

> +  (set-window-margins (selected-window) 0))

Of course, the main problem with this approach is that "margins don't
compose": any other package using the margins (e.g. (n)linum) will tend
to interfere with your own use.

> +  (with-selected-window w        ; So the later `goto-char' will work.

An alternative is to use `set-window-point` instead of `goto-char`.


        Stefan

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Stefan.

Thanks for all the suggestions and corrections.

On Tue, Aug 27, 2019 at 15:59:27 -0400, Stefan Monnier wrote:
> Hi Alan,

> > +(defvar compilation-dummy-string ">"
> > +  "A string which is only a placeholder for compilation-margin-string.
> > +It's actual value is never used, but must be one character long.")
> > +(put-text-property 0 1 'display
> > +                   `((margin left-margin) ,compilation-margin-string)
> > +                   compilation-dummy-string)

> I'd suggest you use a "compilation--" prefix here since this is supposed
> to be internal, IIUC.

DONE.

> Also, I'd do it as

>     (defvar compilation--dummy-string
>       (propertize ">" 'display
>                   `((margin left-margin) ,compilation-margin-string))
>       "A string which is only a placeholder for `compilation-margin-string'.
>     Its actual value is never used, but must be one character long.")

> so you can M-C-x it without ill-effect [note I also fixed "It's" to
> "Its" and added `...' around the var ref.  ]

Thanks.  I've made it a defconst, too.

> > +(defun compilation-tear-down-arrow-spec-in-margin ()
> > +  "Restore compilation-arrow-overlay to not using the margin, which is removed."
> > +  (overlay-put compilation-arrow-overlay 'before-string nil)
> > +  (delete-overlay compilation-arrow-overlay)
> > +  (setq compilation-arrow-overlay nil)

> I think this `setq` loses the overlay and there's no code to reconstruct
> it later on.

Actually, it got reconstructed in compilation-set-overlay-arrow, which
was suboptimal.  I've moved this bit to
compilation-set-up-arrow-spec-in-margin, which is where it should have
been all along.

> > +  (set-window-margins (selected-window) 0))

> Of course, the main problem with this approach is that "margins don't
> compose": any other package using the margins (e.g. (n)linum) will tend
> to interfere with your own use.

I've amended this to add or subtract 2 to/from the current margin width,
rather than setting an absolute width.

> > +  (with-selected-window w        ; So the later `goto-char' will work.

> An alternative is to use `set-window-point` instead of `goto-char`.

Maybe.  But I'm already using (selected-window) in a couple of places,
so I think I'll just leave this.

>         Stefan

Here's the current version of the patch.  Any objections to me
committing it?



diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 09188dc14b..c30908ea01 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2602,45 +2602,69 @@ compilation-set-window
 			       (point)))
          (set-window-point w mk))))
 
-(defvar-local overlay-arrow-overlay nil
+(defvar-local compilation-arrow-overlay nil
   "Overlay with the before-string property of `overlay-arrow-string'.
 
 When non-nil, this overlay causes redisplay to display `overlay-arrow-string'
 at the overlay's start position.")
 
+(defvar compilation-margin-string "=>"
+  "The string which will appear in the margin in compilation mode.
+This must be two characters long; there should be no need to
+change the default.")
+(put-text-property 0 2 'face 'default compilation-margin-string)
+
+(defconst compilation--dummy-string
+  (propertize ">" 'display
+              `((margin left-margin) ,compilation-margin-string))
+  "A string which is only a placeholder for compilation-margin-string.
+Actual value is never used, only the text property.")
+
+(defun compilation-set-up-arrow-spec-in-margin ()
+  "Set up compilation-arrow-overlay to display as an arrow in a margin."
+  (setq overlay-arrow-string "")
+  (setq compilation-arrow-overlay
+	(make-overlay overlay-arrow-position overlay-arrow-position))
+  (overlay-put compilation-arrow-overlay
+               'before-string compilation--dummy-string)
+  (set-window-margins (selected-window) (+ (or (car (window-margins)) 0) 2)))
+
+(defun compilation-tear-down-arrow-spec-in-margin ()
+  "Restore compilation-arrow-overlay to not using the margin, which is removed."
+  (overlay-put compilation-arrow-overlay 'before-string nil)
+  (delete-overlay compilation-arrow-overlay)
+  (setq compilation-arrow-overlay nil)
+  (set-window-margins (selected-window) (- (car (window-margins)) 2)))
+
 (defun compilation-set-overlay-arrow (w)
   "Set up, or switch off, the overlay-arrow for window W."
-  (with-current-buffer (window-buffer w)
-    (if (and (eq compilation-context-lines t)
-             (equal (car (window-fringes w)) 0)) ; No left fringe
-        ;; Insert a "=>" before-string overlay at the beginning of the
-        ;; line pointed to by `overlay-arrow-position'.
-        (cond
-         ((overlayp overlay-arrow-overlay)
-          (when (not (eq (overlay-start overlay-arrow-overlay)
-		         overlay-arrow-position))
-	    (if overlay-arrow-position
-                (progn
-	          (move-overlay overlay-arrow-overlay
-			        overlay-arrow-position overlay-arrow-position)
-                  (setq overlay-arrow-string "=>")
-                  (overlay-put overlay-arrow-overlay
-                               'before-string overlay-arrow-string))
-	      (delete-overlay overlay-arrow-overlay)
-	      (setq overlay-arrow-overlay nil))))
-
-         (overlay-arrow-position
-          (setq overlay-arrow-overlay
-	        (make-overlay overlay-arrow-position overlay-arrow-position))
-          (setq overlay-arrow-string "=>")
-          (overlay-put overlay-arrow-overlay 'before-string overlay-arrow-string)))
-
-      ;; `compilation-context-lines' isn't t, or we've got a left
-      ;; fringe, so remove any overlay arrow.
-      (when (overlayp overlay-arrow-overlay)
-        (setq overlay-arrow-string "")
-        (delete-overlay overlay-arrow-overlay)
-        (setq overlay-arrow-overlay nil)))))
+  (with-selected-window w        ; So the later `goto-char' will work.
+    (with-current-buffer (window-buffer w)
+      (if (and (eq compilation-context-lines t)
+               (equal (car (window-fringes w)) 0)) ; No left fringe
+          ;; Insert a before-string overlay at the beginning of the line
+          ;; pointed to by `overlay-arrow-position', such that it will
+          ;; display in a 2-character margin.
+          (progn
+            (cond
+             ((overlayp compilation-arrow-overlay)
+              (when (not (eq (overlay-start compilation-arrow-overlay)
+		             overlay-arrow-position))
+	        (if overlay-arrow-position
+	            (move-overlay compilation-arrow-overlay
+			          overlay-arrow-position overlay-arrow-position)
+                  (compilation-tear-down-arrow-spec-in-margin))))
+
+             (overlay-arrow-position
+              (compilation-set-up-arrow-spec-in-margin)))
+            ;; Ensure that the "=>" remains in the window by causing
+            ;; the window to be scrolled, if needed.
+            (goto-char (overlay-start compilation-arrow-overlay)))
+
+        ;; `compilation-context-lines' isn't t, or we've got a left
+        ;; fringe, so remove any overlay arrow.
+        (when (overlayp compilation-arrow-overlay)
+          (compilation-tear-down-arrow-spec-in-margin))))))
 
 (defvar next-error-highlight-timer)
 

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[martin rudalics]

+  (with-selected-window w        ; So the later `goto-char' will work.
+    (with-current-buffer (window-buffer w)

'with-selected-window' makes the buffer of its window argument current
so 'with-current-buffer' is not needed here.

martin

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Martin.

On Sat, Aug 31, 2019 at 14:07:26 +0200, martin rudalics wrote:
> +  (with-selected-window w        ; So the later `goto-char' will work.
> +    (with-current-buffer (window-buffer w)

> 'with-selected-window' makes the buffer of its window argument current
> so 'with-current-buffer' is not needed here.

Thanks, I've taken it out.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Stefan Monnier]

>> 1- Why `overlay-arrow-overlay` without a `compilation-` prefix?
> It goes together with overlay-arrow-string, and overlay-arrow-position.

I can see the logic of the name, but if it's in compile.el it should
have a `compilation-` prefix, IMO.  If you intend it to be a new generic
feature, then it should not be in compile.el.

>> 2- Why insert a prefix string (an "inserted arrow") instead of using
>>    a "regular overwriting arrow"?
> Because the overwriting arrow would obliterate the first two characters
> of the file name.  I actually tried this first, and it wasn't
> satisfactory.  This contrasts with another use of the overwriting arrow
> in edebug, where (usually) only WS gets overwritten, and it is important
> not to disturb the visible indentation.

I see.  Indeed, I guess most other uses of overlay-arrow will typically
end up overwriting whitespace whereas in *compilation* the first two
chars will usually not be whitespace.

Maybe worth a comment in the code.

>> 3- Why link the choice of inserted-arrow vs regular-overwriting-arrow to
>>    compilation-context-lines specifically?
> Simplicity: mainly confusion on my part as just to where all the bits of
> the new code would have to go if the "insertion" arrow were to be made a
> general feature.  If the new mechanism is a success, it should be easier
> then to make it more general.

Fair enough.

>> -- Stefan "maybe part of my confusion is that I'm not sufficiently
>>            familiar with the behavior when there's no left-fringe"
>
> That behaviour involves scrolling the current line to the top of the
> buffer, and several years of that had exceeded my irritation threshold.
> I usually run on a Linux tty, where there's no possibility of a fringe.

IIUC previously, in the absence of a left-fringe the position of the
error was indicated by the cursor, right?  So your change is to offer an
option "don't scroll" (or rather, let the redisplay scroll to keep point
visible but only when needed), and you added to it the
overlay-arrow-overlay because the cursor was not visible enough?

If so another option might be to replace the scrolling-or-overlayarrow
with some kind of transient highlighting.  Can be annoying as well, tho.


        Stefan

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Alan Mackenzie]

Hello, Stefan.

On Sun, Aug 25, 2019 at 16:54:27 -0400, Stefan Monnier wrote:
> >> 1- Why `overlay-arrow-overlay` without a `compilation-` prefix?
> > It goes together with overlay-arrow-string, and
> > overlay-arrow-position.

> I can see the logic of the name, but if it's in compile.el it should
> have a `compilation-` prefix, IMO.  If you intend it to be a new
> generic feature, then it should not be in compile.el.

OK, I've changed its name to compilation-arrow-overlay (see the patch in
another post just posted).

> >> 2- Why insert a prefix string (an "inserted arrow") instead of using
> >>    a "regular overwriting arrow"?
> > Because the overwriting arrow would obliterate the first two characters
> > of the file name.  I actually tried this first, and it wasn't
> > satisfactory.  This contrasts with another use of the overwriting arrow
> > in edebug, where (usually) only WS gets overwritten, and it is important
> > not to disturb the visible indentation.

> I see.  Indeed, I guess most other uses of overlay-arrow will typically
> end up overwriting whitespace whereas in *compilation* the first two
> chars will usually not be whitespace.

> Maybe worth a comment in the code.

Now that I've implemented Eli's suggestion of putting "=>" into a 2
character margin, such a comment doesn't seem needed any more.

[ .... ]

> >> -- Stefan "maybe part of my confusion is that I'm not sufficiently
> >>            familiar with the behavior when there's no left-fringe"

> > That behaviour involves scrolling the current line to the top of the
> > buffer, and several years of that had exceeded my irritation
> > threshold.  I usually run on a Linux tty, where there's no
> > possibility of a fringe.

> IIUC previously, in the absence of a left-fringe the position of the
> error was indicated by the cursor, right?  So your change is to offer
> an option "don't scroll" (or rather, let the redisplay scroll to keep
> point visible but only when needed), and you added to it the
> overlay-arrow-overlay because the cursor was not visible enough?

On emacs -nw in X, the cursor, although barely visible is actually
there.  On other terminals, in particular a Linux tty, there is no
indication of the cursor position whatsoever in non-selected windows.

> If so another option might be to replace the scrolling-or-overlayarrow
> with some kind of transient highlighting.  Can be annoying as well, tho.

s/w/h.  ;-)  The transient highlighting would be inadequate.  I think
Eli was concerned that there was no durable indication of which error
message was "current".  The "=>" in the margin now provides such a
durable indication.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [Emacs-diffs] master 29d1c72: Introduce new value t for compilation-context-lines to eliminate scrolling

[Stefan Monnier]

> Now that I've implemented Eli's suggestion of putting "=>" into a 2
> character margin, such a comment doesn't seem needed any more.

Agreed.

> On emacs -nw in X, the cursor, although barely visible is actually
> there.  On other terminals, in particular a Linux tty, there is no
> indication of the cursor position whatsoever in non-selected windows.

Oh, right!  Now I think I really understand.


        Stefan