next-error-highlight = t - highlighting doesn't stay until you move to the next locus

next-error-highlight = t - highlighting doesn't stay until you move to the next locus

[Drew Adams]

The doc string says:

"If t, highlight the locus indefinitely until some
 other locus replaces it."

I wish it were so. However, the code says this:

(if (or (eq next-error-highlight t)
        (numberp next-error-highlight))
    ;; We want highlighting:
    ;; delete overlay on next input.  <---------- NOT GOOD
    (add-hook 'pre-command-hook 'compilation-goto-locus-delete-o)
...

What I want: Leave the highlighting in place, until I ask to move to another
locus (and then highlight that one) - just as the doc says.

What happens: The highlighting disappears completely upon the next event.
For example, if I use pop-up-frames, and I click in the source buffer
(frame), the highlighting disappears. If the source-buffer frame is slightly
behind the *grep* frame, then I can't see where the hit is - as soon as I
click the frame, the highlighting disappears.

IOW, why add the deletion to pre-command-hook? It should, as the doc says,
be associated with the request to move to another hit.

Re: next-error-highlight = t - highlighting doesn't stay until you move to the next locus

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Date: Tue, 20 Mar 2007 16:20:33 -0700
> 
> The doc string says:
> 
> "If t, highlight the locus indefinitely until some
>  other locus replaces it."
> 
> I wish it were so. However, the code says this:
> 
> (if (or (eq next-error-highlight t)
>         (numberp next-error-highlight))
>     ;; We want highlighting:
>     ;; delete overlay on next input.  <---------- NOT GOOD
>     (add-hook 'pre-command-hook 'compilation-goto-locus-delete-o)
> ...

I cannot find this code in the current CVS version of simple.el
(assuming that this where you take it from).  Please see if the latest
code still has the problem.

RE: next-error-highlight = t - highlighting doesn't stay until you moveto the next locus

[Drew Adams]

> > The doc string says:
> >
> > "If t, highlight the locus indefinitely until some
> >  other locus replaces it."
> >
> > I wish it were so. However, the code says this:
> >
> > (if (or (eq next-error-highlight t)
> >         (numberp next-error-highlight))
> >     ;; We want highlighting:
> >     ;; delete overlay on next input.  <---------- NOT GOOD
> >     (add-hook 'pre-command-hook 'compilation-goto-locus-delete-o)
> > ...
>
> I cannot find this code in the current CVS version of simple.el
> (assuming that this where you take it from).  Please see if the latest
> code still has the problem.

Yes, the latest code in CVS still has the problem. See the definition of
`compilation-goto-locus' in compile.el. Thanks for looking into this.

Re: next-error-highlight = t - highlighting doesn't stay until you moveto the next locus

[Eli Zaretskii]

> From: "Drew Adams" <drew.adams@oracle.com>
> Cc: <emacs-devel@gnu.org>
> Date: Sat, 24 Mar 2007 09:50:25 -0700
> 
> > > The doc string says:
> > >
> > > "If t, highlight the locus indefinitely until some
> > >  other locus replaces it."
> > >
> > > I wish it were so. However, the code says this:
> > >
> > > (if (or (eq next-error-highlight t)
> > >         (numberp next-error-highlight))
> > >     ;; We want highlighting:
> > >     ;; delete overlay on next input.  <---------- NOT GOOD
> > >     (add-hook 'pre-command-hook 'compilation-goto-locus-delete-o)
> > > ...
> >
> > I cannot find this code in the current CVS version of simple.el
> > (assuming that this where you take it from).  Please see if the latest
> > code still has the problem.
> 
> Yes, the latest code in CVS still has the problem. See the definition of
> `compilation-goto-locus' in compile.el. Thanks for looking into this.

If I understand correctly, the problem is that `next-error-highlight's
documentation on simple.el is inconsistent with what
`compilation-goto-locus' (from compile.el) actually does.  I don't
know how to fix that, since next-error-highlight' is a general-purpose
variable, whose use is not limited to compile.el.  This is certainly
not a simple doc string problem, so perhaps we should leave this until
after the release.

RE: next-error-highlight = t - highlighting doesn't stay until you moveto the next locus

[Drew Adams]

> If I understand correctly, the problem is that `next-error-highlight's
> documentation on simple.el is inconsistent with what
> `compilation-goto-locus' (from compile.el) actually does.  I don't
> know how to fix that, since next-error-highlight' is a general-purpose
> variable, whose use is not limited to compile.el.  This is certainly
> not a simple doc string problem, so perhaps we should leave this until
> after the release.

Yes, product /= doc.

I would like to see the product follow the doc, in this case. It is quite
annoying to have the highlighting disappear upon the next event, instead of
upon the next use of `next-error'. To me, this a bug. IIUC, some bugs are
being fixed before the release. Could this please be one of them?

Though `next-error-highlight' might be general, `compilation-goto-locus' is
not so general. It is the latter, IMO, that needs to be fixed so that it
doesn't remove the highlighting prematurely.

Re: next-error-highlight = t - highlighting doesn't stay until you moveto the next locus

[Richard Stallman]

    Though `next-error-highlight' might be general, `compilation-goto-locus' is
    not so general.

Actually, `compilation-goto-locus' is the only place that does
anything with `next-error-highlight'.  So this is a simple mismatch
between the code in `compilation-goto-locus' and the doc string of
`next-error-highlight'.

The current behavior could be desirable, and so could the documented
behavior for `next-error-highlight' = t.  So how about implementing
the behavior of "until next time" as another option, and assigning two
values for `next-error-highlight' to stand for the two?

RE: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Drew Adams]

> Actually, `compilation-goto-locus' is the only place that does
> anything with `next-error-highlight'.  So this is a simple mismatch
> between the code in `compilation-goto-locus' and the doc string of
> `next-error-highlight'.

Yes.

> The current behavior could be desirable, and so could the documented
> behavior for `next-error-highlight' = t.  So how about implementing
> the behavior of "until next time" as another option, and assigning two
> values for `next-error-highlight' to stand for the two?

Just as long as I can choose some value to make it do what the doc string
says, I'll be happy. Thanks.

[However, I cannot imagine anyone preferring the current behavior (for a
value of t). Click the mouse or move the cursor and the highlighting
disappears? How could that be desirable?]

Re: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Miles Bader]

"Drew Adams" <drew.adams@oracle.com> writes:
> [However, I cannot imagine anyone preferring the current behavior (for a
> value of t). Click the mouse or move the cursor and the highlighting
> disappears? How could that be desirable?]

Seems perfectly reasonable to me -- it's supposed to be quick flash to
draw your eye, not a permanent annotation.

-Miles

-- 
"Nah, there's no bigger atheist than me.  Well, I take that back.
I'm a cancer screening away from going agnostic and a biopsy away
from full-fledged Christian."  [Adam Carolla]

Re: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Stefan Monnier]

> [However, I cannot imagine anyone preferring the current behavior (for a
> value of t). Click the mouse or move the cursor and the highlighting
> disappears? How could that be desirable?]

I, for one, cannot imagine anyone preferring the behavior where the
highlighting lives on for ever.  To make it bearable, you'd need at the very
least a special way to make it go away when you don't want it any more.


        Stefan

RE: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Drew Adams]

> I, for one, cannot imagine anyone preferring the behavior where the
> highlighting lives on for ever.

The doc string description of the value t, and the behavior I prefer, is
that the highlighting goes away when it is replaced by the next hit visit
(`next-error'). Yes, this does always leave one occurrence highlighted. That
doesn't bother me. If it did, I would make it go away by re-fontifying the
buffer.

I've been using a similar highlighting in my own code for years, and I in
fact leave all of the visited occurences highlighted, "forever". That makes
it very easy to compare bits of code - a bit like `occur' in context.

> To make it bearable, you'd need at the very
> least a special way to make it go away when you don't want it any more.

Yes, I have a key that removes such highlighting (in the case of my own
highlighting, which is of each visited hit). I prefer to remove the
highlighting only upon demand. Since Emacs 22 only ever leaves one hit
highlighted, I never feel the need to remove that highlighting.

Different strokes for different folks.

Re: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Stefan Monnier]

> The doc string description of the value t, and the behavior I prefer, is
> that the highlighting goes away when it is replaced by the next hit visit
> (`next-error').  Yes, this does always leave one occurrence highlighted.
> That doesn't bother me. If it did, I would make it go away by
> re-fontifying the buffer.

Since next-error uses an overlay, re-fontifying wouldn't help.

> I've been using a similar highlighting in my own code for years, and I in
> fact leave all of the visited occurences highlighted, "forever".
> That makes it very easy to compare bits of code - a bit like `occur'
> in context.

>> To make it bearable, you'd need at the very least a special way to make
>> it go away when you don't want it any more.

> Yes, I have a key that removes such highlighting (in the case of my own
> highlighting, which is of each visited hit).

But then it looks more and more like a new feature, i.e. not fit for Emacs-22.


        Stefan

RE: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Drew Adams]

> > The doc string description of the value t, and the behavior I prefer, is
> > that the highlighting goes away when it is replaced by the next
> > hit visit (`next-error').  Yes, this does always leave one
> > occurrence highlighted. That doesn't bother me. If it did, I would
> > make it go away by re-fontifying the buffer.
>
> Since next-error uses an overlay, re-fontifying wouldn't help.

As I said, leaving the last occurrence highlighted doesn't bother me.
Different people use Emacs differently and have different preferences.

As I said in my initial post:

> For example, if I use pop-up-frames, and I click in the source buffer
> (frame), the highlighting disappears. If the source-buffer frame
> is slightly behind the *grep* frame, then I can't see where the hit
> is - as soon as I click the frame, the highlighting disappears.

Your eyes might be quicker or better than mine, but highlighting that
disappears before I can even notice it is useless to me.

> > I've been using a similar highlighting in my own code for
> > years, and I in fact leave all of the visited occurences
> > highlighted, "forever". That makes it very easy to compare
> > bits of code - a bit like `occur' in context.
>
> >> To make it bearable, you'd need at the very least a special way to make
> >> it go away when you don't want it any more.
>
> > Yes, I have a key that removes such highlighting (in the case of my own
> > highlighting, which is of each visited hit).
>
> But then it looks more and more like a new feature, i.e. not fit
> for Emacs-22.

I'm not asking that multiple occurrences remain highlighted as in my own
code. I'm asking only that the implementation respect the doc string for
`t', that users have an option value that will leave the occurrence
highlighted until `next-error'.

Re: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

>> But then it looks more and more like a new feature, i.e. not fit
>> for Emacs-22.
>
> I'm not asking that multiple occurrences remain highlighted as in my own
> code. I'm asking only that the implementation respect the doc string for
> `t', that users have an option value that will leave the occurrence
> highlighted until `next-error'.

I think we should simply fix the docstring and leave the rest for
after the release.

RE: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Drew Adams]

> > I'm not asking that multiple occurrences remain highlighted as in my own
> > code. I'm asking only that the implementation respect the doc string for
> > `t', that users have an option value that will leave the occurrence
> > highlighted until `next-error'.
>
> I think we should simply fix the docstring and leave the rest for
> after the release.

What's wrong with Richard's suggestion of:

> implementing the behavior of "until next time" as another
> option, and assigning two values for `next-error-highlight'
> to stand for the two?

Re: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

>> > I'm not asking that multiple occurrences remain highlighted as in my own
>> > code. I'm asking only that the implementation respect the doc string for
>> > `t', that users have an option value that will leave the occurrence
>> > highlighted until `next-error'.
>>
>> I think we should simply fix the docstring and leave the rest for
>> after the release.
>
> What's wrong with Richard's suggestion of:
>
>> implementing the behavior of "until next time" as another
>> option, and assigning two values for `next-error-highlight'
>> to stand for the two?

That can probably wait till after the release; it is, after all, a new
feature.

Re: next-error-highlight = t - highlighting doesn't stay until youmoveto the next locus

[Richard Stallman]

    [However, I cannot imagine anyone preferring the current behavior (for a
    value of t). Click the mouse or move the cursor and the highlighting
    disappears? How could that be desirable?]

It is not obvious to me that this is bad.
Anyway, this option is already implemented, so it takes
essentially no work to keep it as an option.
There is no point eliminating it.

Would someone please try implementing the other behavior
that you've asked for?

Re: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Richard Stallman]

    >> implementing the behavior of "until next time" as another
    >> option, and assigning two values for `next-error-highlight'
    >> to stand for the two?

    That can probably wait till after the release; it is, after all, a new
    feature.

I would call it a bug fix, since the code does not match the
documentation.  However, most people seem to prefer the current
behavior.  So I agree that changing the doc is a good fix
for the bug.

Would someone please do that?

RE: next-error-highlight = t - highlighting doesn't stayuntilyoumoveto the next locus

[Drew Adams]

>     >> implementing the behavior of "until next time" as another
>     >> option, and assigning two values for `next-error-highlight'
>     >> to stand for the two?
>
>     That can probably wait till after the release; it is,
>     after all, a new feature.
>
> I would call it a bug fix, since the code does not match the
> documentation.  However, most people seem to prefer the current
> behavior.  So I agree that changing the doc is a good fix
> for the bug.
>
> Would someone please do that?

Is the intention nevertheless to fix this as you suggested after the
release? That is, will a value be provided after the release to allow the
highlighting to stay until the next `next-error'? Or are you considering
that making the doc fit the current implementation is an adequate fix for
the bug?

Just asking for clarification. If there is no intention to fix this as I
requested, then I'll try to update my own highlighting code from Emacs 20 to
fit Emacs 22 also. If this will be available in Emacs 22 sometime after the
release, then I won't bother.

Re: next-error-highlight = t - highlighting doesn't stay untilyoumoveto the next locus

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     >> implementing the behavior of "until next time" as another
>     >> option, and assigning two values for `next-error-highlight'
>     >> to stand for the two?
>
>     That can probably wait till after the release; it is, after all, a new
>     feature.
>
> I would call it a bug fix, since the code does not match the
> documentation.  However, most people seem to prefer the current
> behavior.  So I agree that changing the doc is a good fix
> for the bug.
>
> Would someone please do that?

Done.

Re: next-error-highlight = t - highlighting doesn't stayuntilyoumoveto the next locus

[Chong Yidong]

> Is the intention nevertheless to fix this as you suggested after the
> release? That is, will a value be provided after the release to allow the
> highlighting to stay until the next `next-error'? Or are you considering
> that making the doc fit the current implementation is an adequate fix for
> the bug?
>
> Just asking for clarification. If there is no intention to fix this as I
> requested, then I'll try to update my own highlighting code from Emacs 20 to
> fit Emacs 22 also. If this will be available in Emacs 22 sometime after the
> release, then I won't bother.

I can look into implementing this feature --- after Emacs 22.1 is out.

Re: next-error-highlight = t - highlighting doesn't stayuntilyoumoveto the next locus

[Richard Stallman]

    Is the intention nevertheless to fix this as you suggested after the
    release? That is, will a value be provided after the release to allow the
    highlighting to stay until the next `next-error'?

If someone wants to implement the other option then, that is fine with me.