patch for next-error highlighting

patch for next-error highlighting

[Drew Adams]

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

The attached patches of `simple.el' and `compile.el' add and respect a new
value, `until-move', for options `next-error-highlight' and
`next-error-highlight-no-select'.

The value of `t' is not adequate in a one-buffer-per-frame environment (mine, at
least). The problem is that the overlay gets deleted upon the next command, and
a switch-frame event can act as the next command. Much of the time, the
highlighting is never seen, because a switch-frame event (whether from the user
or the code) causes it to disappear too quickly.

Regardless of whether you are convinced that that is a problem, I would argue
that a user should be given the option of showing the highlighting until the
locus changes, regardless of the next command.

The new value `until-move' does that: it moves the overlay when you change
locus, but it does not delete the overlay upon the next command. IOW, it does
not put `compilation-goto-locus-delete-o' on `pre-command-hook'.

(Personally, I'd prefer that `until-move' be the default value, because I think
it makes more sense for others too...)

You may argue that if the value is `until-move', the overlay is never deleted.
Correct. So what? A user should still have this option.

In practice, it doesn't bother me at all that the overlay remains, and I've been
using this for years (long before Emacs itself had such highlighting). 

If you really want a way for the user to get rid of the overlay, then let `C-u
C-u C-x `' do that, by adding this to the beginning of the definition of
`next-error':

(if (and (consp arg) (= 16 (prefix-numeric-value arg)) 
         compilation-highlight-overlay)
    (delete-overlay compilation-highlight-overlay)
  ...)


[-- Attachment #2: simple-2009-02-22.patch --]
[-- Type: application/octet-stream, Size: 3769 bytes --]

diff -c -w "simple.el" "simple-patched-2009-02-22.el"
*** simple.el	Sun Feb 22 13:20:10 2009
--- simple-patched-2009-02-22.el	Sun Feb 22 13:26:12 2009
***************
*** 121,150 ****
    :version "22.1")
  
  (defcustom next-error-highlight 0.5
!   "Highlighting of locations in selected source buffers.
! If a number, highlight the locus in `next-error' face for the given time
! in seconds, or until the next command is executed.
! If t, highlight the locus until the next command is executed, or until
! some other locus replaces it.
! If nil, don't highlight the locus in the source buffer.
! If `fringe-arrow', indicate the locus by the fringe arrow."
!   :type '(choice (number :tag "Highlight for specified time")
!                  (const :tag "Semipermanent highlighting" t)
!                  (const :tag "No highlighting" nil)
!                  (const :tag "Fringe arrow" fringe-arrow))
    :group 'next-error
    :version "22.1")
  
  (defcustom next-error-highlight-no-select 0.5
    "Highlighting of locations in `next-error-no-select'.
! If number, highlight the locus in `next-error' face for given time in seconds.
! If t, highlight the locus indefinitely until some other locus replaces it.
! If nil, don't highlight the locus in the source buffer.
! If `fringe-arrow', indicate the locus by the fringe arrow."
!   :type '(choice (number :tag "Highlight for specified time")
!                  (const :tag "Semipermanent highlighting" t)
!                  (const :tag "No highlighting" nil)
!                  (const :tag "Fringe arrow" fringe-arrow))
    :group 'next-error
    :version "22.1")
  
--- 121,162 ----
    :version "22.1")
  
  (defcustom next-error-highlight 0.5
!   "Highlighting of the current locus in selected source buffer.
! Highlighting means use face `next-error' or show a fringe arrow.
! Value:
! * `fringe-arrow' means indicate the locus using a fringe arrow.
! * `until-move' means highlight the locus until it is moved.
! * A number means highlight the locus for that many seconds, or until
!   the next command is executed.
! * t means highlight the locus until it is moved or the next command is
!   executed.
! * nil means do not highlight the locus at all."
!   :type '(choice
!           (number :tag "Highlight locus for specified time")
!           (const :tag  "Highlight locus until move"                 until-move)
!           (const :tag  "Highlight locus until move or next command" t)
!           (const :tag  "Do not highlight locus"                     nil)
!           (const :tag  "Indicate locus using fringe arrow"          fringe-arrow))
    :group 'next-error
    :version "22.1")
  
  (defcustom next-error-highlight-no-select 0.5
    "Highlighting of locations in `next-error-no-select'.
! Highlighting means use face `next-error' or show a fringe arrow.
! Value:
! * `fringe-arrow' means indicate the locus using a fringe arrow.
! * `until-move' means highlight the locus until it is moved.
! * A number means highlight the locus for that many seconds, or until
!   the next command is executed.
! * t means highlight the locus until it is moved or the next command is
!   executed.
! * nil means do not highlight the locus at all."
!   :type '(choice
!           (number :tag "Highlight locus for specified time")
!           (const :tag  "Highlight locus until move"                 until-move)
!           (const :tag  "Highlight locus until move or next command" t)
!           (const :tag  "Do not highlight locus"                     nil)
!           (const :tag  "Indicate locus using fringe arrow"          fringe-arrow))
    :group 'next-error
    :version "22.1")
  

Diff finished at Sun Feb 22 13:27:04

[-- Attachment #3: compile-2009-02-22.patch --]
[-- Type: application/octet-stream, Size: 1879 bytes --]

diff -c -w "compile.el" "compile-patched-2009-02-22.el"
*** compile.el	Sun Feb 22 13:20:56 2009
--- compile-patched-2009-02-22.el	Sun Feb 22 13:23:56 2009
***************
*** 2085,2098 ****
  		;; We want highlighting: delete overlay on next input.
  		(add-hook 'pre-command-hook
  			  'compilation-goto-locus-delete-o)
  	      ;; We don't want highlighting: delete overlay now.
! 	      (delete-overlay compilation-highlight-overlay))
  	    ;; We want highlighting for a limited time:
  	    ;; set up a timer to delete it.
  	    (when (numberp next-error-highlight)
  	      (setq next-error-highlight-timer
  		    (run-at-time next-error-highlight nil
! 				 'compilation-goto-locus-delete-o)))))))
      (when (and (eq next-error-highlight 'fringe-arrow))
        ;; We want a fringe arrow (instead of highlighting).
        (setq next-error-overlay-arrow-position
--- 2085,2100 ----
                  ;; We want highlighting: delete overlay on next input.
                  (add-hook 'pre-command-hook
                            'compilation-goto-locus-delete-o)
+               (unless (eq next-error-highlight 'until-move)
                  ;; We don't want highlighting: delete overlay now.
!                 (delete-overlay compilation-highlight-overlay)))
              ;; We want highlighting for a limited time:
              ;; set up a timer to delete it.
              (when (numberp next-error-highlight)
                (setq next-error-highlight-timer
                      (run-at-time next-error-highlight nil
!                                  'compilation-goto-locus-delete-o)))))
!         (raise-frame)))
      (when (and (eq next-error-highlight 'fringe-arrow))
        ;; We want a fringe arrow (instead of highlighting).
        (setq next-error-overlay-arrow-position

Diff finished at Sun Feb 22 13:29:41

Re: patch for next-error highlighting

[Miles Bader]

"Drew Adams" <drew.adams@oracle.com> writes:
> You may argue that if the value is `until-move', the overlay is never deleted.
> Correct. So what? A user should still have this option.

The name `until-move' sounds like it means the overlay gets deleted when
the user issues a move command (C-n, C-f ...).

Something like `persistent' seems more clear.

-Miles

-- 
"1971 pickup truck; will trade for guns"

RE: patch for next-error highlighting

[Drew Adams]

> The name `until-move' sounds like it means the overlay gets 
> deleted when the user issues a move command (C-n, C-f ...).
> 
> Something like `persistent' seems more clear.

I'm OK with whatever name you want.

FWIW, the rationale for `until-move' is that it is consistent with its tag, and
the set of tags is consistent. Each tag describes when the current locus is
highlighted/unhighlighted.

IOW, it is not about the overlay being persistent; it is about the
(un)highlighting of the current locus, whether and when that happens.

These are the descriptions I sent:

(number :tag "Highlight locus for specified time")
(const :tag  "Highlight locus until move"  until-move)
(const :tag  "Highlight locus until move or next command"  t)
(const :tag  "Do not highlight locus"  nil)
(const :tag  "Indicate locus using fringe arrow"  fringe-arrow))

Re: patch for next-error highlighting

[Miles Bader]

"Drew Adams" <drew.adams@oracle.com> writes:
> IOW, it is not about the overlay being persistent; it is about the
> (un)highlighting of the current locus, whether and when that happens.

It looks like the effect of your change is that the highlighting becomes
persistent.

-Miles

-- 
Logic, n. The art of thinking and reasoning in strict accordance with the
limitations and incapacities of the human misunderstanding.

RE: patch for next-error highlighting

[Drew Adams]

Any news on this patch?

>---------8<------------
> From: Drew Adams Sent: Sunday, February 22, 2009 3:17 PM
> The attached patches of `simple.el' and `compile.el' add and 
> respect a new value, `until-move', for options
> `next-error-highlight' and `next-error-highlight-no-select'.
> 
> The value of `t' is not adequate in a one-buffer-per-frame 
> environment (mine, at least). The problem is that the overlay
> gets deleted upon the next command, and a switch-frame event
> can act as the next command. Much of the time, the
> highlighting is never seen, because a switch-frame event 
> (whether from the user or the code) causes it to disappear too quickly.
> 
> Regardless of whether you are convinced that that is a problem,
> I would argue that a user should be given the option of showing
> the highlighting until the locus changes, regardless of the next command.
> 
> The new value `until-move' does that: it moves the overlay 
> when you change locus, but it does not delete the overlay upon
> the next command. IOW, it does not put
> `compilation-goto-locus-delete-o' on `pre-command-hook'.
> 
> (Personally, I'd prefer that `until-move' be the default 
> value, because I think it makes more sense for others too...)
> 
> You may argue that if the value is `until-move', the overlay 
> is never deleted. Correct. So what? A user should still have
> this option.
> 
> In practice, it doesn't bother me at all that the overlay 
> remains, and I've been using this for years (long before
> Emacs itself had such highlighting). 
> 
> If you really want a way for the user to get rid of the 
> overlay, then let `C-u C-u C-x `' do that, by adding this to
> the beginning of the definition of `next-error':
> 
> (if (and (consp arg) (= 16 (prefix-numeric-value arg)) 
>          compilation-highlight-overlay)
>     (delete-overlay compilation-highlight-overlay)
>   ...)