Overlay arrow in *compilation* and *grep* buffers

Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

A couple of weeks ago, a change to compile.el introduced an overlay
arrow into buffers created by compile.el: the overlay arrow now marks
the current line in the compilation buffer.  (Here, ``current line''
means the line whose locus is displayed in the other window;
next-error and mouse-1 change that line.)

At least on a text terminal, this feature is problematic.

First, suppose that "M-x compile" produced the following *compilation*
buffer (I indent the buffer contents 2 columns to the right to make it
stand out):

  -*- mode: compilation; default-directory: "~/" -*-
  gcc -Wall -O -c tfwd.c
  tfwd.c: In function `foo':
  tfwd.c:5: warning: implicit declaration of function `bar'
  tfwd.c: At top level:
  tfwd.c:11: warning: type mismatch with previous implicit declaration
  tfwd.c:5: warning: previous implicit declaration of `bar'
  tfwd.c:11: warning: `bar' was previously implicitly declared to return `int'
  tfwd.c: In function `bar':
  tfwd.c:13: warning: implicit declaration of function `callme'
  tfwd.c: At top level:
  tfwd.c:4: warning: `foo' defined but not used

  Compilation finished at Sun Apr 24 06:01:25

Now, if I type "C-x `", the window that displays the compilation
buffer looks like this:

  =>wd.c:5: warning: implicit declaration of function `bar'
  tfwd.c: At top level:
  tfwd.c:11: warning: type mismatch with previous implicit declaration
  tfwd.c:5: warning: previous implicit declaration of `bar'
  tfwd.c:11: warning: `bar' was previously implicitly declared to return `int'
  tfwd.c: In function `bar':
  tfwd.c:13: warning: implicit declaration of function `callme'
  tfwd.c: At top level:
  tfwd.c:4: warning: `foo' defined but not used

  Compilation finished at Sun Apr 24 06:01:25

Notice the overlay arrow that covered part of the file name: this is a
bug, IMHO.  If we want to have an arrow pointing out the current line,
we should indent the buffer text to the right as many columns as the
arrow string takes.

Moreover, next-error scrolls the display to keep the current line at
the top of the window.  I think it's a bit silly to mark the with an
arrow a line that is always at the top of its window; such an arrow
might make sense if we do not scroll the window except when the
current line is no longer visible.  Perhaps we need a user option to
control these two features (scrolling and arrow) in a way that would
by default prevent scrolling when the arrow is used to show the
current line.

Also, the arrow feature is not customizable.  What about users who
will dislike it and would wish to turn it off?

Finally, I don't really understand why new features such as this one
get installed while we are in a feature freeze.  At the very least, it
should have been discussed (such a discussion could also lead to
better design decisions wrt scrolling).  However, I couldn't find
anything related in emacs-devel archives (sorry if I missed
something).

Re: Overlay arrow in *compilation* and *grep* buffers

[Kim F. Storm]

"Eli Zaretskii" <eliz@gnu.org> writes:

>   =>wd.c:5: warning: implicit declaration of function `bar'

> Notice the overlay arrow that covered part of the file name: this is a
> bug, IMHO.  If we want to have an arrow pointing out the current line,
> we should indent the buffer text to the right as many columns as the
> arrow string takes.

On text terminals, the overlay-arrow is just that --- an overlay arrow.
So it is not a bug ...

> Also, the arrow feature is not customizable.  What about users who
> will dislike it and would wish to turn it off?

What about

 (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: Sun, 24 Apr 2005 20:42:47 +0200
> Cc: Juri Linkov <juri@jurta.org>, emacs-devel@gnu.org
> 
> "Eli Zaretskii" <eliz@gnu.org> writes:
> 
> >   =>wd.c:5: warning: implicit declaration of function `bar'
> 
> On text terminals, the overlay-arrow is just that --- an overlay arrow.
> So it is not a bug ...

By ``bug'' in this case I meant a design bug.

It's at least a bad misfeature, and surely shouldn't be on by default
in compilation buffers.  Unlike buffers that display program sources
(where GUD uses the overlay arrow), whose most lines tend to be
indented, compilation buffers have almost all of their lines start at
column zero, so the arrow will always hide something.

> > Also, the arrow feature is not customizable.  What about users who
> > will dislike it and would wish to turn it off?
> 
> What about
> 
>  (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")

That's not what is usually meant by ``customizable''.

Let me be sure I understand you correctly, Kim: are you actually
saying that this is a good feature, the way it behaves in current CVS?

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    Notice the overlay arrow that covered part of the file name: this is a
    bug, IMHO.  If we want to have an arrow pointing out the current line,
    we should indent the buffer text to the right as many columns as the
    arrow string takes.

It might be good to do that when overlaying the arrow on a compilation
error buffer, but it would be a misfeature to do that when overlaying
the arrow on a program file (which was the original use of the overlay
arrow).

      Perhaps we need a user option to
    control these two features (scrolling and arrow) in a way that would
    by default prevent scrolling when the arrow is used to show the
    current line.

    Also, the arrow feature is not customizable.  What about users who
    will dislike it and would wish to turn it off?

I'd probably prefer to turn off the arrow and use just the scrolling,
for compilation buffers.

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: emacs-devel@gnu.org, juri@jurta.org
> Date: Mon, 25 Apr 2005 12:05:44 -0400
> 
>     Notice the overlay arrow that covered part of the file name: this is a
>     bug, IMHO.  If we want to have an arrow pointing out the current line,
>     we should indent the buffer text to the right as many columns as the
>     arrow string takes.
> 
> It might be good to do that when overlaying the arrow on a compilation
> error buffer, but it would be a misfeature to do that when overlaying
> the arrow on a program file (which was the original use of the overlay
> arrow).

I was only talking about compilation buffers.  Buffers that visit
program sources tend to have non-zero indentation which makes this
problem almost non-existent.

> I'd probably prefer to turn off the arrow and use just the scrolling,
> for compilation buffers.

I have no problems with that, this is how it worked before the arrow
was introduced.

Btw, if the intent was to highlight the current line in the
compilation buffer, we could do this with a color overlay that spans
the entire line.  This would make the line stand out without obscuring
any of its text.

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    Btw, if the intent was to highlight the current line in the
    compilation buffer, we could do this with a color overlay that spans
    the entire line.  This would make the line stand out without obscuring
    any of its text.

It is worth a try.

Re: Overlay arrow in *compilation* and *grep* buffers

[Kim F. Storm]

"Eli Zaretskii" <eliz@gnu.org> writes:

> By ``bug'' in this case I meant a design bug.

Then we agree!

> It's at least a bad misfeature, and surely shouldn't be on by default
> in compilation buffers.  

on text terminals, yes.

>> > Also, the arrow feature is not customizable.  What about users who
>> > will dislike it and would wish to turn it off?
>> 
>> What about
>> 
>>  (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")
>
> That's not what is usually meant by ``customizable''.

No, it was a _temporary_ fix, and perhaps something compile.el could do.

(unless (display-graphic-p)
 (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))


>
> Let me be sure I understand you correctly, Kim: are you actually
> saying that this is a good feature, the way it behaves in current CVS?

No.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: storm@cua.dk (Kim F. Storm)
> Date: Wed, 27 Apr 2005 15:10:59 +0200
> Cc: juri@jurta.org, emacs-devel@gnu.org
> 
> "Eli Zaretskii" <eliz@gnu.org> writes:
> 
> > By ``bug'' in this case I meant a design bug.
> 
> Then we agree!
> 
> > It's at least a bad misfeature, and surely shouldn't be on by default
> > in compilation buffers.  
> 
> on text terminals, yes.

Agreed.

Juri, do you intend to fix this in some way?

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    > Let me be sure I understand you correctly, Kim: are you actually
    > saying that this is a good feature, the way it behaves in current CVS?

    No.

Why not do what was suggested?

    (unless (display-graphic-p)
     (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))

Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > Moreover, next-error scrolls the display to keep the current line at
 > the top of the window.  I think it's a bit silly to mark the with an
 > arrow a line that is always at the top of its window; such an arrow
 > might make sense if we do not scroll the window except when the
 > current line is no longer visible.  Perhaps we need a user option to
 > control these two features (scrolling and arrow) in a way that would
 > by default prevent scrolling when the arrow is used to show the
 > current line.

I made a similar point on emacs-devel:

compile-goto-error
Tue, 16 Nov 2004

Juri pointed me to compilation-context-lines and Stefan said:

SM> I recently realized that while the 0-context sometimes makes sense for
SM> C-x `, the "don't move" behavior would be preferable when getting
SM> to an error by using RET or mouse-2 on the actual error text.

 > Finally, I don't really understand why new features such as this one
 > get installed while we are in a feature freeze.  At the very least, it
 > should have been discussed (such a discussion could also lead to
 > better design decisions wrt scrolling).  However, I couldn't find
 > anything related in emacs-devel archives (sorry if I missed
 > something).

They were discussed as part of a thread that started as a bug report on
emacs-pretest-bug:

fringe arrow conflict between compile and gud?
JUAN-LEON Lahoz Garcia
Wed, 23 Mar 2005

I guess its a chicken and egg situation but a proper feature freeze that
lasted over a year would surely set back development.


Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Thu, 28 Apr 2005 23:34:33 +1200
> Cc: emacs-devel@gnu.org, Juri Linkov <juri@jurta.org>
> 
>  > Finally, I don't really understand why new features such as this one
>  > get installed while we are in a feature freeze.  At the very least, it
>  > should have been discussed (such a discussion could also lead to
>  > better design decisions wrt scrolling).  However, I couldn't find
>  > anything related in emacs-devel archives (sorry if I missed
>  > something).
> 
> They were discussed as part of a thread that started as a bug report on
> emacs-pretest-bug:
> 
> fringe arrow conflict between compile and gud?
> JUAN-LEON Lahoz Garcia
> Wed, 23 Mar 2005

Sorry, that's hardly a discussion: the arrow on text terminals was
introduced as a side effect of the fringe arrow, and the adverse
effects on text-only displays was never even mentioned.  I'm guessing
that no one tried the changes on a text terminal before they were
installed.

> I guess its a chicken and egg situation but a proper feature freeze that
> lasted over a year would surely set back development.

So what are you saying? let's ignore the freeze and install anything
we want because the show must go on no matter what?

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: eliz@gnu.org, juri@jurta.org, emacs-devel@gnu.org
> Date: Thu, 28 Apr 2005 07:01:00 -0400
> 
> Why not do what was suggested?
> 
>     (unless (display-graphic-p)
>      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))

Wouldn't this affect Edebug on text terminals?

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > > Why not do what was suggested?
 > > 
 > >     (unless (display-graphic-p)
 > >      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))
 > 
 > Wouldn't this affect Edebug on text terminals?

No, I don't think so. The thread that was hardly a discussion was about
conflicts and covered this. The changes that ensued meant that Edebug and Gnus
still use the global variable overlay-arrow-position while compile.el and
gud.el have their own overlay arrow, following Kim's earlier work to allow
multiple overlay arrows.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > > I guess its a chicken and egg situation but a proper feature freeze that
 > > lasted over a year would surely set back development.
 > 
 > So what are you saying? let's ignore the freeze and install anything
 > we want because the show must go on no matter what?

Major features like Unicode and multi-tty haven't been installed. This phase
seems to be more about updating the manuals. I don't think the feature freeze
will seem real until there is a date for the pretest. What are the rules once
the pretest is out?

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Fri, 29 Apr 2005 08:54:39 +1200
> Cc: juri@jurta.org, rms@gnu.org, emacs-devel@gnu.org
> 
>  > >     (unless (display-graphic-p)
>  > >      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))
>  > 
>  > Wouldn't this affect Edebug on text terminals?
> 
> No, I don't think so. The thread that was hardly a discussion was about
> conflicts and covered this. The changes that ensued meant that Edebug and Gnus
> still use the global variable overlay-arrow-position while compile.el and
> gud.el have their own overlay arrow, following Kim's earlier work to allow
> multiple overlay arrows.

So, if the next-error-overlay-arrow-position's string doesn't affect
anything but compile.el, how come text terminals started to display
the arrow as result of a discussion about GUD and Edebug?  What am I
missing?

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > So, if the next-error-overlay-arrow-position's string doesn't affect
 > anything but compile.el, how come text terminals started to display
 > the arrow as result of a discussion about GUD and Edebug?  What am I
 > missing?

I might be losing your meaning, but the conflict was between GUD and
next-error. Juri and I changed them to use their own overlay arrow. Juri then
suggested that the default value for the property overlay-arrow-string should
be changed to "=>" which was done by Kim (2005-04-08). Previously it was nil
which meant that no arrow was displayed in a text terminal which was not by
design, but might have been your preference.

Currently simple.el has:

(put 'next-error-overlay-arrow-position 'overlay-arrow-string "=>")

but this could be removed as it is the default now anyway (I think Juri
disappeared again before 2005-04-08).

Alternatively, as suggested, it could be changed to:

(put 'next-error-overlay-arrow-position 'overlay-arrow-string "")

I don't think the condition

(unless (display-graphic-p)...

is necessary (or desirable). If there is a fringe, a bitmap is used and
overlay-arrow-string is not used. If there is no fringe, then presumably you
don't want "=>" overwriting text whether the terminal is text or graphic.

I suggest this latter change until Juri (or maybe Stefan) comes up with a
better solution that will work with "=>".


Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    SM> I recently realized that while the 0-context sometimes makes sense for
    SM> C-x `, the "don't move" behavior would be preferable when getting
    SM> to an error by using RET or mouse-2 on the actual error text.

That is true *if* we use the overlay arrow.  However, as has just been
pointed out, using the overlay arrow in error message lists is very
annoying on text consoles.  On text consoles, I think it would be
better to move to the top, even for RET or Mouse-2, than to display
the overlay arrow.

    They were discussed as part of a thread that started as a bug report on
    emacs-pretest-bug:

    fringe arrow conflict between compile and gud?
    JUAN-LEON Lahoz Garcia
    Wed, 23 Mar 2005

Sometimes a bug fix can cause new problems that need other fixes.
And sometimes the only clean way to fix a bug requires adding a
small feature.

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > Why not do what was suggested?
 > 
 >     (unless (display-graphic-p)
 >      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))

Actually, its just occurred to me that this is the wrong arrow. I think this
one is used to display the error in the *source* buffer. I now think that
the line:

  (set (make-local-variable 'overlay-arrow-string) "=>")

in compilation-setup should be changed to

  (set (make-local-variable 'overlay-arrow-string) "")

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 >     SM> I recently realized that while the 0-context sometimes makes sense
 >     for SM> C-x `, the "don't move" behavior would be preferable when
 >     getting SM> to an error by using RET or mouse-2 on the actual error
 >     text.
 > 
 > That is true *if* we use the overlay arrow.  However, as has just been
 > pointed out, using the overlay arrow in error message lists is very
 > annoying on text consoles.  On text consoles, I think it would be
 > better to move to the top, even for RET or Mouse-2, than to display
 > the overlay arrow.

This patch does what you are suggesting. It doesn't do what Stefan suggested
(not because I don't think it is a good idea, but because I don't know how to
implement it cleanly). It might be better to change the condition for a text
terminal to one that checks for a left-hand fringe, if thats possible.


Nick


*** /home/nick/emacs/lisp/progmodes/compile.el.~1.352.~	2005-04-24 07:39:26.000000000 +1200
--- /home/nick/emacs/lisp/progmodes/compile.el	2005-04-30 00:15:11.000000000 +1200
***************
*** 1247,1253 ****
    (make-local-variable 'compilation-messages-start)
    (make-local-variable 'compilation-error-screen-columns)
    (make-local-variable 'overlay-arrow-position)
!   (set (make-local-variable 'overlay-arrow-string) "=>")
    (setq next-error-overlay-arrow-position nil)
    (add-hook 'kill-buffer-hook
  	    (lambda () (setq next-error-overlay-arrow-position nil)) nil t)
--- 1247,1253 ----
    (make-local-variable 'compilation-messages-start)
    (make-local-variable 'compilation-error-screen-columns)
    (make-local-variable 'overlay-arrow-position)
!   (set (make-local-variable 'overlay-arrow-string) "")
    (setq next-error-overlay-arrow-position nil)
    (add-hook 'kill-buffer-hook
  	    (lambda () (setq next-error-overlay-arrow-position nil)) nil t)
***************
*** 1576,1593 ****
  
  (defcustom compilation-context-lines 0
    "*Display this many lines of leading context before message.
! If nil, don't scroll the compilation output window."
    :type '(choice integer (const :tag "No window scrolling" nil))
    :group 'compilation
    :version "22.1")
  
  (defsubst compilation-set-window (w mk)
    "Align the compilation output window W with marker MK near top."
!   (if (integerp compilation-context-lines)
!       (set-window-start w (save-excursion
!                             (goto-char mk)
!                             (beginning-of-line (- 1 compilation-context-lines))
!                             (point))))
    (set-window-point w mk))
  
  (defvar next-error-highlight-timer)
--- 1576,1599 ----
  
  (defcustom compilation-context-lines 0
    "*Display this many lines of leading context before message.
! If nil, don't scroll the compilation output window. Text terminals never
! scroll because no overlay arrow is present."
    :type '(choice integer (const :tag "No window scrolling" nil))
    :group 'compilation
    :version "22.1")
  
  (defsubst compilation-set-window (w mk)
    "Align the compilation output window W with marker MK near top."
!   (if (display-graphic-p)
!       (if (integerp compilation-context-lines)
! 	  (set-window-start w (save-excursion
! 				(goto-char mk)
! 				(beginning-of-line (- 1 compilation-context-lines))
! 				(point))))
!     (set-window-start w (save-excursion
! 			  (goto-char mk)
! 			  (beginning-of-line 1)
! 			  (point))))
    (set-window-point w mk))
  
  (defvar next-error-highlight-timer)

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: eliz@gnu.org, juri@jurta.org, emacs-devel@gnu.org
> Date: Thu, 28 Apr 2005 07:01:00 -0400
> 
>     > Let me be sure I understand you correctly, Kim: are you actually
>     > saying that this is a good feature, the way it behaves in current CVS?
> 
>     No.
> 
> Why not do what was suggested?
> 
>     (unless (display-graphic-p)
>      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))

Since no one objected, I fixed it like this, but a little bit
differently: I set overlay-error-string to "" in compilation-setup, if
the terminal is not graphic.

Re: Overlay arrow in *compilation* and *grep* buffers

[Kim F. Storm]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> From: Richard Stallman <rms@gnu.org>
>> CC: eliz@gnu.org, juri@jurta.org, emacs-devel@gnu.org
>> Date: Thu, 28 Apr 2005 07:01:00 -0400
>> 
>>     > Let me be sure I understand you correctly, Kim: are you actually
>>     > saying that this is a good feature, the way it behaves in current CVS?
>> 
>>     No.
>> 
>> Why not do what was suggested?
>> 
>>     (unless (display-graphic-p)
>>      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))
>
> Since no one objected, I fixed it like this, but a little bit
> differently: I set overlay-error-string to "" in compilation-setup, if
> the terminal is not graphic.

IIRC, Richard installed the above change already.  Maybe that wasn't
necessary?

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > >> Why not do what was suggested?
 > >> 
 > >>     (unless (display-graphic-p)
 > >>      (put 'next-error-overlay-arrow-position 'overlay-arrow-string ""))
 > >
 > > Since no one objected, I fixed it like this, but a little bit
 > > differently: I set overlay-error-string to "" in compilation-setup, if
 > > the terminal is not graphic.

The thread moved on from there. You (= Eli) have done:

  (set (make-local-variable 'overlay-arrow-string)
       (if (display-graphic-p) "=>" ""))

I said that the condition (if (display-graphic-p)... was unnecessary and
unrequired. On a graphical display, when there is a left hand fringe
overlay-arrow-string isn't used and when there isn't one, you presumably want
overlay-arrow-string to be "=>".

 > IIRC, Richard installed the above change already.  Maybe that wasn't
 > necessary?

Richard installed a different change to simple.el.

(put 'next-error-overlay-arrow-position 'overlay-arrow-string "")

but this is a *different* arrow that displays in the *source* buffer.

I think this change is wrong.

I suggested a patch in this thread (Sat, 30 Apr 2005 00:19:43 +1200) which
also moves the current error to the top for the current error.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 8 May 2005 12:41:49 +1200
> Cc: Eli Zaretskii <eliz@gnu.org>, rms@gnu.org, emacs-devel@gnu.org
> 
> The thread moved on from there. You (= Eli) have done:
> 
>   (set (make-local-variable 'overlay-arrow-string)
>        (if (display-graphic-p) "=>" ""))
> 
> I said that the condition (if (display-graphic-p)... was unnecessary and
> unrequired. On a graphical display, when there is a left hand fringe
> overlay-arrow-string isn't used and when there isn't one, you presumably want
> overlay-arrow-string to be "=>".

Let me be sure I understand you: the above says that sometimes (e.g.,
when there's no left fringe), we _do_ want overlay-arrow-string to be
"=>" on a graphical display, right?  If so, why do you still say that
testing for display-graphic-p is unnecessary?

> Richard installed a different change to simple.el.
> 
> (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")
> 
> but this is a *different* arrow that displays in the *source* buffer.
> 
> I think this change is wrong.

Which change is wrong--Richard's or mine?

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

> > I said that the condition (if (display-graphic-p)... was unnecessary and
 > > unrequired. On a graphical display, when there is a left hand fringe
 > > overlay-arrow-string isn't used and when there isn't one, you presumably
 > > want overlay-arrow-string to be "=>".
 > 
 > Let me be sure I understand you: the above says that sometimes (e.g.,
 > when there's no left fringe), we _do_ want overlay-arrow-string to be
 > "=>" on a graphical display, right?  If so, why do you still say that
 > testing for display-graphic-p is unnecessary?

Sorry, I meant "and when there isn't one, you presumably want
overlay-arrow-string to be "" i.e a graphical display without a left fringe
should work like a text terminal.

 > > Richard installed a different change to simple.el.
 > > 
 > > (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")
 > > 
 > > but this is a *different* arrow that displays in the *source* buffer.
 > > 
 > > I think this change is wrong.
 > 
 > Which change is wrong--Richard's or mine?

Richard's. I think you're changing the behaviour of the right arrow i.e
the local variable, overlay-arrow-position, which is the one used in the
*compilation* buffer.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    > Richard installed a different change to simple.el.
    > 
    > (put 'next-error-overlay-arrow-position 'overlay-arrow-string "")
    > 
    > but this is a *different* arrow that displays in the *source* buffer.
    > 
    > I think this change is wrong.

    Which change is wrong--Richard's or mine?

It looks like I changed the wrong marker, and indeed it appears not to
work.  If your change does the job, please delete mine.

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: nickrob@snap.net.nz, emacs-devel@gnu.org
> Date: Sun, 08 May 2005 12:12:37 -0400
> 
> It looks like I changed the wrong marker, and indeed it appears not to
> work.  If your change does the job, please delete mine.

Done.

Re: Overlay arrow in *compilation* and *grep* buffers

[Juri Linkov]

> Currently simple.el has:
>
> (put 'next-error-overlay-arrow-position 'overlay-arrow-string "=>")
>
> but this could be removed as it is the default now anyway (I think Juri
> disappeared again before 2005-04-08).

Sorry.  I traveled to my friend.  She lives 1000km from me.

> I suggest this latter change until Juri (or maybe Stefan) comes up with a
> better solution that will work with "=>".

I suggest adding a new option defining how to highlight lines
in compilation/grep buffers, similar to `next-error-highlight'
which defines highlighting in source buffers.  It will replace
`compilation-context-lines':

(defcustom compilation-message-highlight 0
  "*Highlighting of locations in the compilation buffer.
If number, display this many lines of leading context before message.
If t, use persistent overlays fontified in next-error face.
If nil, don't highlight the locus in the compilation buffer.
If `fringe-arrow', indicate the locus by the fringe arrow."
  :type '(choice (number :tag "Context lines")
                 (const :tag "Persistent overlay" t)
                 (const :tag "No highlighting" nil)
                 (const :tag "Fringe arrow" 'fringe-arrow))
  :group 'compilation
  :version "22.1")

Since the number of context lines and fringe arrow are mutually exclusive
values, there will be no problem with an arrow overlapping a file name
in the compilation buffer, because by default there will be no arrow at all.
Users wishing to use arrows can set `compilation-message-highlight' to
`fringe-arrow' in .emacs conditionally depending on the presence of fringes.

BTW, I don't understand the reason of adding `(defvar next-error-highlight-timer)'
to compile.el on 2005-04-23.  There is already the same defvar in simple.el.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Overlay arrow in *compilation* and *grep* buffers

[David Kastrup]

Juri Linkov <juri@jurta.org> writes:

>> Currently simple.el has:
>>
>> (put 'next-error-overlay-arrow-position 'overlay-arrow-string "=>")
>>
>> but this could be removed as it is the default now anyway (I think
>> Juri disappeared again before 2005-04-08).
>
> Sorry.  I traveled to my friend.  She lives 1000km from me.

Time to invest into a good WLAN antenna.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > I suggest adding a new option defining how to highlight lines
 > in compilation/grep buffers, similar to `next-error-highlight'
 > which defines highlighting in source buffers.  It will replace
 > `compilation-context-lines':
 > 
 > (defcustom compilation-message-highlight 0
 >   "*Highlighting of locations in the compilation buffer.
 > If number, display this many lines of leading context before message.
 > If t, use persistent overlays fontified in next-error face.
 > If nil, don't highlight the locus in the compilation buffer.
 > If `fringe-arrow', indicate the locus by the fringe arrow."
 >   :type '(choice (number :tag "Context lines")
 >                  (const :tag "Persistent overlay" t)
 >                  (const :tag "No highlighting" nil)
 >                  (const :tag "Fringe arrow" 'fringe-arrow))
 >   :group 'compilation
 >   :version "22.1")
 > 
 > Since the number of context lines and fringe arrow are mutually exclusive
 > values, there will be no problem with an arrow overlapping a file name
 > in the compilation buffer, because by default there will be no arrow at all.
 > Users wishing to use arrows can set `compilation-message-highlight' to
 > `fringe-arrow' in .emacs conditionally depending on the presence of fringes.

I don't like this change because its not the case that one user wants an arrow
and another doesn't but rather one context may benefit from an arrow
(graphical display) while another may not (text terminal).

Previously I have suggested a change that gives compilation-context-lines a
context dependent values so that the error scrolls to the top of the window on
a text terminal. This or a similar change could be generalised to the case of
no left-fringe which I think could be tested by:

(if (or (not (display-graphic-p))
        (equal fringe-mode 0)
        (equal (car fringe-mode) 0))...

 > BTW, I don't understand the reason of adding `(defvar
 > next-error-highlight-timer)' to compile.el on 2005-04-23.  There is already
 > the same defvar in simple.el.

I would guess this was to remove a compile error. I don't think Richard
realised that the next-error code was defined in simple.el.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    I don't like this change because its not the case that one user wants an arrow
    and another doesn't but rather one context may benefit from an arrow
    (graphical display) while another may not (text terminal).

I agree.

And doesn't the current code do exactly this?

Re: Overlay arrow in *compilation* and *grep* buffers

[Juri Linkov]

> I don't like this change because its not the case that one user wants an arrow
> and another doesn't but rather one context may benefit from an arrow
> (graphical display) while another may not (text terminal).
>
> Previously I have suggested a change that gives compilation-context-lines a
> context dependent values so that the error scrolls to the top of the window on
> a text terminal.  This or a similar change could be generalised to the case of
> no left-fringe which I think could be tested by:
>
> (if (or (not (display-graphic-p))
>         (equal fringe-mode 0)
>         (equal (car fringe-mode) 0))...

Even on text terminals users might prefer not to scroll the window.
Since overlay arrows are problematic on text terminals, there should
be an alternative method of highlighting.  I propose to add one of the
methods of next-error highlighting for compilation messages, i.e. a
persistent overlay fontified in a special face.

The new option compilation-current-message-highlight will support
three values: `arrow', t for highlighting the whole current message
line, and nil to turn all indications off.

The default value can be selected based on the following conditions:

1. When left-fringe is available (tested with a condition like you proposed),
   use an arrow.
2. Otherwise, if compilation-context-lines is nil, highlight the
   current line in a special face with the persistent overlay.
3. If compilation-context-lines is not nil, don't highlight the current
   message since the current message is indicated by its location
   at the top of the window (by default).

There is a brief list of changes in the patch below:

1. New defcustom compilation-current-message-highlight.
2. compilation-context-lines moved up before compilation-context-lines.
3. compilation-highlight-overlay renamed to next-error-highlight-overlay,
   because this overlay is actually used for highlighting source lines
   visited by `next-error'.
4. compilation-highlight-overlay is used for highlighting lines
   in the compilation buffer.
6. The value `fringe-arrow' renamed to more general name `arrow'.

Index: lisp/progmodes/compile.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/compile.el,v
retrieving revision 1.353
diff -u -r1.353 compile.el
--- lisp/progmodes/compile.el	7 May 2005 16:18:36 -0000	1.353
+++ lisp/progmodes/compile.el	13 May 2005 04:59:43 -0000
@@ -383,12 +383,38 @@
    "Additional things to highlight in Compilation mode.
 This gets tacked on the end of the generated expressions.")
 
+(defcustom compilation-context-lines 0
+  "*Display this many lines of leading context before message.
+If nil, don't scroll the compilation output window."
+  :type '(choice integer (const :tag "No window scrolling" nil))
+  :group 'compilation
+  :version "22.1")
+
 (defvar compilation-highlight-regexp t
   "Regexp matching part of visited source lines to highlight temporarily.
 Highlight entire line if t; don't highlight source lines if nil.")
 
 (defvar compilation-highlight-overlay nil
-  "Overlay used to temporarily highlight compilation matches.")
+  "Overlay used to highlight the current message in the compilation buffer.")
+
+(defcustom compilation-current-message-highlight
+  (if (and (or (not (display-graphic-p))
+	       (equal fringe-mode 0)
+	       (equal (car-safe fringe-mode) 0)))
+      (not compilation-context-lines)
+    'arrow)
+  "*Highlighting of the current message in the compilation buffer.
+If t, highlight the current message with `compilation-current-message-face'.
+If nil, don't highlight the message in the compilation buffer.
+If `arrow', indicate the current message by the fringe or overlay arrow.
+
+The default is `arrow' when left fringe arrow is displayable.  Otherwise,
+the default is t when compilation-context-lines is nil."
+  :type '(choice (const :tag "Highligh whole line" t)
+                 (const :tag "No highlighting" nil)
+                 (const :tag "Arrow" 'arrow))
+  :group 'compilation
+  :version "22.1")
 
 (defcustom compilation-error-screen-columns t
   "*If non-nil, column numbers in error messages are screen columns.
@@ -491,6 +517,10 @@
 `compilation-info-face', `compilation-line-face' and
 `compilation-column-face' get prepended to this, when applicable.")
 
+(defvar compilation-current-message-face 'region
+  "Face name to use for the current message selected by `next-error'.
+Used when `compilation-current-message-highlight' is t.")
+
 (defvar compilation-error-face 'font-lock-warning-face
   "Face name to use for file name in error messages.")
 
@@ -1248,7 +1278,7 @@
   (make-local-variable 'compilation-error-screen-columns)
   (make-local-variable 'overlay-arrow-position)
   (set (make-local-variable 'overlay-arrow-string)
-       (if (display-graphic-p) "=>" ""))
+       (if (eq compilation-current-message-highlight 'arrow) "=>" ""))
   (setq next-error-overlay-arrow-position nil)
   (add-hook 'kill-buffer-hook
 	    (lambda () (setq next-error-overlay-arrow-position nil)) nil t)
@@ -1506,11 +1536,24 @@
 	 (end-loc (nth 2 loc))
 	 (marker (point-marker)))
     (setq compilation-current-error (point-marker)
-	  overlay-arrow-position
-	    (if (bolp)
-		compilation-current-error
-	      (copy-marker (line-beginning-position)))
 	  loc (car loc))
+    (cond ((eq compilation-current-message-highlight 'arrow)
+	   (setq overlay-arrow-position
+		 (if (bolp)
+		     compilation-current-error
+		   (copy-marker (line-beginning-position)))))
+	  ((eq compilation-current-message-highlight t)
+	   (unless compilation-highlight-overlay
+	     (setq compilation-highlight-overlay
+		   (make-overlay (point-min) (point-min)))
+	     (overlay-put compilation-highlight-overlay 'face 'match))
+	   (move-overlay compilation-highlight-overlay
+			 (copy-marker (line-beginning-position))
+			 (copy-marker (line-end-position))
+			 (current-buffer)))
+	  ((not compilation-current-message-highlight)
+	   (if compilation-highlight-overlay
+	       (delete-overlay compilation-highlight-overlay))))
     ;; If loc contains no marker, no error in that file has been visited.  If
     ;; the marker is invalid the buffer has been killed.  So, recalculate all
     ;; markers for that file.
@@ -1575,13 +1618,6 @@
       (setcdr loc (list line file marker)))
     loc))
 
-(defcustom compilation-context-lines 0
-  "*Display this many lines of leading context before message.
-If nil, don't scroll the compilation output window."
-  :type '(choice integer (const :tag "No window scrolling" nil))
-  :group 'compilation
-  :version "22.1")
-
 (defsubst compilation-set-window (w mk)
   "Align the compilation output window W with marker MK near top."
   (if (integerp compilation-context-lines)
@@ -1591,8 +1627,6 @@
                             (point))))
   (set-window-point w mk))
 
-(defvar next-error-highlight-timer)
-
 (defun compilation-goto-locus (msg mk end-mk)
   "Jump to an error corresponding to MSG at MK.
 All arguments are markers.  If END-MK is non-nil, mark is set there
@@ -1636,10 +1670,10 @@
     (when highlight-regexp
       (if (timerp next-error-highlight-timer)
 	  (cancel-timer next-error-highlight-timer))
-      (unless compilation-highlight-overlay
-	(setq compilation-highlight-overlay
+      (unless next-error-highlight-overlay
+	(setq next-error-highlight-overlay
 	      (make-overlay (point-min) (point-min)))
-	(overlay-put compilation-highlight-overlay 'face 'next-error))
+	(overlay-put next-error-highlight-overlay 'face 'next-error))
       (with-current-buffer (marker-buffer mk)
 	(save-excursion
 	  (if end-mk (goto-char end-mk) (end-of-line))
@@ -1649,19 +1683,19 @@
 		     (re-search-forward highlight-regexp end t))
 		(progn
 		  (goto-char (match-beginning 0))
-		  (move-overlay compilation-highlight-overlay
+		  (move-overlay next-error-highlight-overlay
 				(match-beginning 0) (match-end 0)
 				(current-buffer)))
-	      (move-overlay compilation-highlight-overlay
+	      (move-overlay next-error-highlight-overlay
 			    (point) end (current-buffer)))
 	    (if (numberp next-error-highlight)
 		(setq next-error-highlight-timer
 		      (run-at-time next-error-highlight nil 'delete-overlay
-				   compilation-highlight-overlay)))
+				   next-error-highlight-overlay)))
 	    (if (not (or (eq next-error-highlight t)
 			 (numberp next-error-highlight)))
-		(delete-overlay compilation-highlight-overlay))))))
-    (when (and (eq next-error-highlight 'fringe-arrow))
+		(delete-overlay next-error-highlight-overlay))))))
+    (when (and (eq next-error-highlight 'arrow))
       (setq next-error-overlay-arrow-position
 	    (copy-marker (line-beginning-position))))))

Index: lisp/simple.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/simple.el,v
retrieving revision 1.720
diff -u -r1.720 simple.el
--- lisp/simple.el	8 May 2005 19:33:14 -0000	1.720
+++ lisp/simple.el	13 May 2005 04:55:15 -0000
@@ -89,11 +89,11 @@
 If number, highlight the locus in next-error face for given time in seconds.
 If t, use persistent overlays fontified in next-error face.
 If nil, don't highlight the locus in the source buffer.
-If `fringe-arrow', indicate the locus by the fringe arrow."
+If `arrow', indicate the locus by the fringe or overlay arrow."
   :type '(choice (number :tag "Delay")
                  (const :tag "Persistent overlay" t)
                  (const :tag "No highlighting" nil)
-                 (const :tag "Fringe arrow" 'fringe-arrow))
+                 (const :tag "Arrow" 'arrow))
   :group 'next-error
   :version "22.1")
 
@@ -102,14 +102,17 @@
 If number, highlight the locus in next-error face for given time in seconds.
 If t, use persistent overlays fontified in next-error face.
 If nil, don't highlight the locus in the source buffer.
-If `fringe-arrow', indicate the locus by the fringe arrow."
+If `arrow', indicate the locus by the fringe or overlay arrow."
   :type '(choice (number :tag "Delay")
                  (const :tag "Persistent overlay" t)
                  (const :tag "No highlighting" nil)
-                 (const :tag "Fringe arrow" 'fringe-arrow))
+                 (const :tag "Arrow" 'arrow))
   :group 'next-error
   :version "22.1")
 
+(defvar next-error-highlight-overlay nil
+  "Overlay used to highlight next-error matches.")
+
 (defvar next-error-highlight-timer nil)
 
 (defvar next-error-overlay-arrow-position nil)

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Juri Linkov <juri@jurta.org>
> Date: Fri, 13 May 2005 08:03:54 +0300
> Cc: eliz@gnu.org, emacs-devel@gnu.org
> 
> The new option compilation-current-message-highlight will support
> three values: `arrow', t for highlighting the whole current message
> line, and nil to turn all indications off.

PLEASE do not propose new options until we cut the release branch and
go into pretest.  We will NEVER release another Emacs this way!

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > > The new option compilation-current-message-highlight will support
 > > three values: `arrow', t for highlighting the whole current message
 > > line, and nil to turn all indications off.
 > 
 > PLEASE do not propose new options until we cut the release branch and
 > go into pretest.  We will NEVER release another Emacs this way!

I don't follow this logic. Presumably the only thing that will bring the
pretest forward is completing the items in FOR-RELEASE. Small changes like
this aren't holding it up and may improve the final release.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 > Even on text terminals users might prefer not to scroll the window.
 > Since overlay arrows are problematic on text terminals, there should
 > be an alternative method of highlighting.  I propose to add one of the
 > methods of next-error highlighting for compilation messages, i.e. a
 > persistent overlay fontified in a special face.
 > 
 > The new option compilation-current-message-highlight will support
 > three values: `arrow', t for highlighting the whole current message
 > line, and nil to turn all indications off.
 > 
 > The default value can be selected based on the following conditions:
 > 
 > 1. When left-fringe is available (tested with a condition like you proposed),
 >    use an arrow.
 > 2. Otherwise, if compilation-context-lines is nil, highlight the
 >    current line in a special face with the persistent overlay.
 > 3. If compilation-context-lines is not nil, don't highlight the current
 >    message since the current message is indicated by its location
 >    at the top of the window (by default).
 > 
 > There is a brief list of changes in the patch below:
 > 
 > 1. New defcustom compilation-current-message-highlight.
This value needs to be recomputed at compile time.

 > 2. compilation-context-lines moved up before compilation-context-lines.
                                  before compilation-current-message-highlight?

 > 3. compilation-highlight-overlay renamed to next-error-highlight-overlay,
 >    because this overlay is actually used for highlighting source lines
 >    visited by `next-error'.
 > 4. compilation-highlight-overlay is used for highlighting lines
 >    in the compilation buffer.
 > 6. The value `fringe-arrow' renamed to more general name `arrow'.

Yes, it looks good. The default behaviour matches what was agreed in an earlier
thread.

On a related note, I think that the default value for next-error-highlight
should be nil, as the cursor already points to the source line of interest.

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Fri, 13 May 2005 19:18:37 +1200
> Cc: Juri Linkov <juri@jurta.org>, emacs-devel@gnu.org
> 
>  > PLEASE do not propose new options until we cut the release branch and
>  > go into pretest.  We will NEVER release another Emacs this way!
> 
> I don't follow this logic. Presumably the only thing that will bring the
> pretest forward is completing the items in FOR-RELEASE. Small changes like
> this aren't holding it up and may improve the final release.

The logic is that any work that is not necessary for finishing the
FOR-RELEASE items is wasting time of the core developers, and thus
postpones the pretest and the release, because those developers have
less time to work on FOR-RELEASE items.

If someone doesn't want to work on the manuals or on other FOR-RELEASE
items, fine; but at least they should not bring up new features,
especially those which affect widely used commands or packages,
because such changes tend to provoke long disputes, destabilize Emacs
and introduce bugs and misfeatures that drag many other developers
into discussions, suggestions for how to fix what became broken,
fixing them, debugging the fix for the fix, etc.

We've been through this time and again.  Some recent examples include
the overlay arrow in compilation buffers, font-lock changes wrt
comment delimiters, ispell vs aspell invocation, etc.  How many more
such incidents are needed before people will understand that THERE ARE
NO SMALL CHANGES before a release?

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

     > 1. New defcustom compilation-current-message-highlight.

Please don't add this option.  It isn't necessary to fix a problem.

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    We've been through this time and again.  Some recent examples include
    the overlay arrow in compilation buffers, font-lock changes wrt
    comment delimiters, ispell vs aspell invocation, etc.  How many more
    such incidents are needed before people will understand that THERE ARE
    NO SMALL CHANGES before a release?

Now is the right time to make some kinds of changes, those that correct
annoyances and wrinkles in the features we have added.

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> CC: nickrob@snap.net.nz, juri@jurta.org, emacs-devel@gnu.org
> Date: Fri, 13 May 2005 20:26:10 -0400
> 
> Now is the right time to make some kinds of changes, those that correct
> annoyances and wrinkles in the features we have added.

We clearly disagree on this.  I think that this is precisely the
reason why today, months since feature freeze, we have a less stable
Emacs than we had then.

Re: Overlay arrow in *compilation* and *grep* buffers

[Kim F. Storm]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> From: Richard Stallman <rms@gnu.org>
>> CC: nickrob@snap.net.nz, juri@jurta.org, emacs-devel@gnu.org
>> Date: Fri, 13 May 2005 20:26:10 -0400
>> 
>> Now is the right time to make some kinds of changes, those that correct
>> annoyances and wrinkles in the features we have added.
>
> We clearly disagree on this.  I think that this is precisely the
> reason why today, months since feature freeze, we have a less stable
> Emacs than we had then.

I mostly agree with Eli, but we should try to fix problems with
the new features added since 21.4 -- once 22.1 is out, it may
be too late to get things right.

But at this stage, if people had respected the feature freeze, the
new features in 22.1 would have stabilized by now ... it's the
constant addition of "tiny" things that keep messing things up
and postponing the release (e.g. the comment font lock thing).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

Richard Stallman writes:
 >      > 1. New defcustom compilation-current-message-highlight.
 > 
 > Please don't add this option.  It isn't necessary to fix a problem.

Here's a patch that will hopefully keep everyone happy ;-)

1) No new option.

1) Overlay arrow and no scrolling with left-hand fringe
   e.g. graphical terminal.

2) No overlay arrow and scrolling with no left-hand fringe 
   e.g. text terminal.

The best way to test for a left-hand fringe (absence of) seems to be:

  (if (equal (car (window-fringes)) 0)...

Its about the simplest patch I could think of that meets the requirements.
Shall I install it?

Nick


*** /home/nick/emacs/lisp/progmodes/compile.el.~1.353~	2005-05-15 14:40:42.000000000 +1200
--- /home/nick/emacs/lisp/progmodes/compile.el	2005-05-15 14:42:19.000000000 +1200
***************
*** 1247,1254 ****
    (make-local-variable 'compilation-messages-start)
    (make-local-variable 'compilation-error-screen-columns)
    (make-local-variable 'overlay-arrow-position)
!   (set (make-local-variable 'overlay-arrow-string)
!        (if (display-graphic-p) "=>" ""))
    (setq next-error-overlay-arrow-position nil)
    (add-hook 'kill-buffer-hook
  	    (lambda () (setq next-error-overlay-arrow-position nil)) nil t)
--- 1247,1253 ----
    (make-local-variable 'compilation-messages-start)
    (make-local-variable 'compilation-error-screen-columns)
    (make-local-variable 'overlay-arrow-position)
!   (set (make-local-variable 'overlay-arrow-string) "")
    (setq next-error-overlay-arrow-position nil)
    (add-hook 'kill-buffer-hook
  	    (lambda () (setq next-error-overlay-arrow-position nil)) nil t)
***************
*** 1575,1595 ****
        (setcdr loc (list line file marker)))
      loc))
  
! (defcustom compilation-context-lines 0
    "*Display this many lines of leading context before message.
! If nil, don't scroll the compilation output window."
    :type '(choice integer (const :tag "No window scrolling" nil))
    :group 'compilation
    :version "22.1")
  
  (defsubst compilation-set-window (w mk)
    "Align the compilation output window W with marker MK near top."
!   (if (integerp compilation-context-lines)
        (set-window-start w (save-excursion
!                             (goto-char mk)
!                             (beginning-of-line (- 1 compilation-context-lines))
!                             (point))))
!   (set-window-point w mk))
  
  (defvar next-error-highlight-timer)
  
--- 1574,1602 ----
        (setcdr loc (list line file marker)))
      loc))
  
! (defcustom compilation-context-lines nil
    "*Display this many lines of leading context before message.
! If nil, don't scroll the compilation output window unless there
! is no left-hand fringe. In this case, the message displays at the
! top of the window because no overlay arrow is present."
    :type '(choice integer (const :tag "No window scrolling" nil))
    :group 'compilation
    :version "22.1")
  
  (defsubst compilation-set-window (w mk)
    "Align the compilation output window W with marker MK near top."
!   (if (equal (car (window-fringes)) 0)
        (set-window-start w (save-excursion
! 			    (goto-char mk)
! 			    (beginning-of-line 1)
! 			    (point)))
!     (if (integerp compilation-context-lines)
! 	(set-window-start w (save-excursion
! 			      (goto-char mk)
! 			      (beginning-of-line
! 			       (- 1 compilation-context-lines))
! 			      (point)))))
!     (set-window-point w mk))
  
  (defvar next-error-highlight-timer)

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 15 May 2005 14:44:22 +1200
> Cc: juri@jurta.org, eliz@gnu.org, emacs-devel@gnu.org
> 
> The best way to test for a left-hand fringe (absence of) seems to be:
> 
>   (if (equal (car (window-fringes)) 0)...

I'd suggest to make this a function, e.g. on subr.el or perhaps
frame.el, in case some day this test needs to be modified, in which
case we do not want to go through dozens of places that use it.

> Its about the simplest patch I could think of that meets the requirements.
> Shall I install it?

FWIW, these changes are fine with me, but:

 . I think the manual and NEWS should mention it.

 . Please make sure it does TRT on systems that don't support
   start-process (say "M-: (fmakunbound 'start-process) RET" and see
   that "M-x compile" still works as you'd expect).

Thanks.

Re: Overlay arrow in *compilation* and *grep* buffers

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 15 May 2005 14:44:22 +1200
> Cc: juri@jurta.org, eliz@gnu.org, emacs-devel@gnu.org
> 
> ! (defcustom compilation-context-lines nil
>     "*Display this many lines of leading context before message.
> ! If nil, don't scroll the compilation output window unless there
> ! is no left-hand fringe. In this case, the message displays at the
> ! top of the window because no overlay arrow is present."

This text is hard to understand because of multiple negations (don't,
unless, no fringe).  I suggest to change it into something like this:

  If nil and the left fringe is displayed, don't scroll the
  compilation output window; the arrow in the left fringe indicates
  the current message.  If nil and there's no left fringe, always
  scroll and display the current message at the top of the compilation
  window.

Also, please use two spaces after the period that ends a sentence.

Thanks.

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    But at this stage, if people had respected the feature freeze, the
    new features in 22.1 would have stabilized by now ... it's the
    constant addition of "tiny" things that keep messing things up
    and postponing the release (e.g. the comment font lock thing).

The main cause of delay is that there are substantial jobs that need
to be done which people are not doing.  For instance, only two people
have worked on updating copyright years; they have done part of the
sources, but more remains.  And there is still work to be done on the
manual.

FOR-RELEASE shows we are not yet close to being ready.

I'm not criticizing you personally, since you have done a lot of
debugging of hard bugs, and that was also essential for the release.

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

Please install your patch, with Eli's doc string.

Please update etc/NEWS and the Emacs Manual too.

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

Richard Stallman writes:
 > Please install your patch, with Eli's doc string.
 > 
 > Please update etc/NEWS and the Emacs Manual too.

I've comitted these changes. The manual refers to "error messages" but these
can be "warning messages" too. As with compile.el, I've just called them
messages, I guess the correct term would be "diagnostic messages". All
the functions are next-error-* and changing them to next-diagnostic-*
wouldn't be practical. However, I do think it does make the manual unclear.

While debugging these lisp changes with Edebug, I noticed that `g' (top-level)
didn't clear Edebug's arrow. I don't know if this was due to the compilation
buffer having a local value but it doesn't happen on other functions when the
compilation buffer isn't present. It might be cleaner to give compile its
own arrow like GUD and next-error (the source buffer based arrow).

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    I've comitted these changes. The manual refers to "error messages" but these
    can be "warning messages" too.

I don't consider this a problem.  We need not be so pedantic.
People will understand.

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

    While debugging these lisp changes with Edebug, I noticed that `g' (top-level)
    didn't clear Edebug's arrow.

The natural way to do that is with an unwind-protect around the code
that reads the input.  Would you like to add one?

Re: Overlay arrow in *compilation* and *grep* buffers

[Nick Roberts]

 >     While debugging these lisp changes with Edebug, I noticed that `g'
 >     (top-level) didn't clear Edebug's arrow.
 > 
 > The natural way to do that is with an unwind-protect around the code
 > that reads the input.  Would you like to add one?

You will have to explain further, I don't understand what is causing the
problem.  The code around input to what?

Nick

Re: Overlay arrow in *compilation* and *grep* buffers

[Richard Stallman]

     >     While debugging these lisp changes with Edebug, I noticed that `g'
     >     (top-level) didn't clear Edebug's arrow.
     > 
     > The natural way to do that is with an unwind-protect around the code
     > that reads the input.  Would you like to add one?

    You will have to explain further, I don't understand what is causing the
    problem.  The code around input to what?

I meant the code in edebug to read input.

But now I think I misunderstood before.
The g key can run a command that clears the arrow
and then calls top-level.