Underlining in compile.el

Underlining in compile.el

[Richard M. Stallman]

Does anyone want to write a cleaner implementation of
the feature of underlining the link areas in compile.el?

*** compile.el	12 Jun 2005 06:10:07 -0400	1.362
--- compile.el	16 Jun 2005 22:04:13 -0400	
***************
*** 493,517 ****
  ;; backward-compatibility alias
  (put 'compilation-info-face 'face-alias 'compilation-info)
  
  (defvar compilation-message-face nil
    "Face name to use for whole messages.
  Faces `compilation-error-face', `compilation-warning-face',
  `compilation-info-face', `compilation-line-face' and
  `compilation-column-face' get prepended to this, when applicable.")
  
! (defvar compilation-error-face 'font-lock-warning-face
    "Face name to use for file name in error messages.")
  
! (defvar compilation-warning-face 'compilation-warning
    "Face name to use for file name in warning messages.")
  
! (defvar compilation-info-face 'compilation-info
    "Face name to use for file name in informational messages.")
  
! (defvar compilation-line-face 'font-lock-variable-name-face
    "Face name to use for line number in message.")
  
! (defvar compilation-column-face 'font-lock-type-face
    "Face name to use for column number in message.")
  
  ;; same faces as dired uses
--- 493,552 ----
  ;; backward-compatibility alias
  (put 'compilation-info-face 'face-alias 'compilation-info)
  
+ (defface compilation-error-file-name
+   '((default :inherit font-lock-warning-face)
+     (((supports :underline t)) :underline t))
+   "Face for displaying file names in compilation errors."
+   :group 'font-lock-highlighting-faces
+   :version "22.1")
+ 
+ (defface compilation-warning-file-name
+   '((default :inherit font-lock-warning-face)
+     (((supports :underline t)) :underline t))
+   "Face for displaying file names in compilation errors."
+   :group 'font-lock-highlighting-faces
+   :version "22.1")
+ 
+ (defface compilation-info-file-name
+   '((default :inherit compilation-info)
+     (((supports :underline t)) :underline t))
+   "Face for displaying file names in compilation errors."
+   :group 'font-lock-highlighting-faces
+   :version "22.1")
+ 
+ (defface compilation-line-number
+   '((default :inherit font-lock-variable-name-face)
+     (((supports :underline t)) :underline t))
+   "Face for displaying file names in compilation errors."
+   :group 'font-lock-highlighting-faces
+   :version "22.1")
+ 
+ (defface compilation-column-number
+   '((default :inherit font-lock-type-face)
+     (((supports :underline t)) :underline t))
+   "Face for displaying file names in compilation errors."
+   :group 'font-lock-highlighting-faces
+   :version "22.1")
+ 
  (defvar compilation-message-face nil
    "Face name to use for whole messages.
  Faces `compilation-error-face', `compilation-warning-face',
  `compilation-info-face', `compilation-line-face' and
  `compilation-column-face' get prepended to this, when applicable.")
  
! (defvar compilation-error-face 'compilation-error-file-name
    "Face name to use for file name in error messages.")
  
! (defvar compilation-warning-face 'compilation-warning-file-name
    "Face name to use for file name in warning messages.")
  
! (defvar compilation-info-face 'compilation-info-file-name
    "Face name to use for file name in informational messages.")
  
! (defvar compilation-line-face 'compilation-line-number
    "Face name to use for line number in message.")
  
! (defvar compilation-column-face 'compilation-column-number
    "Face name to use for column number in message.")
  
  ;; same faces as dired uses

Re: Underlining in compile.el

[Kim F. Storm]

"Richard M. Stallman" <rms@gnu.org> writes:

> Does anyone want to write a cleaner implementation of
> the feature of underlining the link areas in compile.el?

Did anyone else actually express any support for that change?

To me, it still seems like the entirely wrong approach to add new
faces everywhere something may look like a link.

IMO, we should let the issue rest until after the (re|p)lease!!!

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

Re: Underlining in compile.el

[Richard M. Stallman]

    To me, it still seems like the entirely wrong approach to add new
    faces everywhere something may look like a link.

The idea that beginners will be less confused if links are underlined
seems inherently plausible.  As for adding new faces, that's the
implementation I saw how to write, but I agree it is ugly.  I'm
suggesting that someone else implement this feature without
defining any new faces, by changing the font-lock specs.

However, ISTR that this complaint was made when there were bugs
causing too much of the line to be highlighted.  If fixing those bugs
was enough to solve the problem, we don't need to do any more.  Would
those that found the situation confusing before please speak up now?

RE: Underlining in compile.el

[Drew Adams]

        To me, it still seems like the entirely wrong approach to add new
        faces everywhere something may look like a link.

    The idea that beginners will be less confused if links are underlined
    seems inherently plausible.

A common practice used on Web pages with tables or lists that are dense with
similar links in similar places is to _not_ use underlining.

That is, in a long list where each list item is a link, and where the list
is the main thing on the page, underlining is often foregone because it is
unnecessary and would be distracting. Users can easily figure out that each
line is a link - mouseover anywhere within the list (the page) highlights a
link.

If compile and grep buffers had full-line links, the links would be easier
to access, they would help with visual alignment (like using a ruler in a
parts catalog), and there would be no need to underline them. Mouseover
highlighting would suffice, and the mouseover highlighting could itself be
underlining, to reduce noise.

Re: Underlining in compile.el

[Richard M. Stallman]

    If compile and grep buffers had full-line links, the links would be easier
    to access, they would help with visual alignment (like using a ruler in a
    parts catalog), and there would be no need to underline them.

I see.  However, we don't want mouse-1 to be active on the whole line,
for other reasons.  I think the current set of places where mouse-1 is
active is the right set.  The question that remains is only whether
to underline those places.

Re: Underlining in compile.el

[Nick Roberts]

 > However, ISTR that this complaint was made when there were bugs
 > causing too much of the line to be highlighted.  If fixing those bugs
 > was enough to solve the problem, we don't need to do any more.  Would
 > those that found the situation confusing before please speak up now?

My preference was for just using mouse-1  where text is generally underlined
already e.g info, help, customize (not underlined) etc.

I think you/others said it should apply to all buffers.

My next preference would be to restrict the area where mouse-1 worked e.g in
grep just on file and line number while mouse-2 could still work over the
whole line.  I would not know how to do this, but I am prepared to work on it
after the release.

I stopped following the thread, but I don't like the idea of adding underlining
as links already have mouse-face and a hand pointer.  What next, a little
jingle that plays when the mouse is over the link?

Nick

Re: Underlining in compile.el

[Richard M. Stallman]

    My next preference would be to restrict the area where mouse-1 worked e.g in
    grep just on file and line number while mouse-2 could still work over the
    whole line.  I would not know how to do this, but I am prepared to work on it
    after the release.

This is already the case when I try it.  Do you see something
different?  If so, would you please provide a precise self-contained
test case?

Re: Underlining in compile.el

[Nick Roberts]

 >     My next preference would be to restrict the area where mouse-1 worked
 >     e.g in grep just on file and line number while mouse-2 could still work
 >     over the whole line.  I would not know how to do this, but I am
 >     prepared to work on it after the release.
 > 
 > This is already the case when I try it.  Do you see something
 > different?  If so, would you please provide a precise self-contained
 > test case?

emacs -Q
cd emacs/lisp
M-x grep
Run grep (like this): grep -nH -e hello *el

The mouse-face and mouse-1 binding lasts until the end of the first match
(shown by carets):

newcomment.el:279:	       (insert cs " hello " ce)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I think it should be:

newcomment.el:279:	       (insert cs " hello " ce)
^^^^^^^^^^^^^^^^^

If there is a variable to set this behaviour, then I think the latter should
be the default.

Nick

Re: Underlining in compile.el

[Juri Linkov]

> The mouse-face and mouse-1 binding lasts until the end of the first
> match (shown by carets):
>
> newcomment.el:279:	       (insert cs " hello " ce)
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> I think it should be:
>
> newcomment.el:279:	       (insert cs " hello " ce)
> ^^^^^^^^^^^^^^^^^

Have you tried this with the latest CVS?  This was fixed two weeks ago.

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

Re: Underlining in compile.el

[Nick Roberts]

Juri Linkov writes:
 > > The mouse-face and mouse-1 binding lasts until the end of the first
 > > match (shown by carets):
 > >
 > > newcomment.el:279:	       (insert cs " hello " ce)
 > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 > >
 > > I think it should be:
 > >
 > > newcomment.el:279:	       (insert cs " hello " ce)
 > > ^^^^^^^^^^^^^^^^^
 > 
 > Have you tried this with the latest CVS?  This was fixed two weeks ago.

I have (+1200):

-rw-r--r--    1 nickrob  nick        23639 Jun 19 09:50 grep.el
-rw-r--r--    1 nickrob  nick        20544 Jun 24 01:33 grep.elc

and

-rwxr-xr-x   2 nickrob  nick   17668441 Jun 27 12:31 /home/nick/emacs/src/emacs

I've just recompiled the lisp files and I can now see underlining under the
filename and line number, but mouse-face and mouse-1 binding extends to the
first match as before.  Also the phrase "matches found" is underlined,
presumably because it has the same face as actual matches.

Nick

Re: Underlining in compile.el

[Nick Roberts]

 > Have you tried this with the latest CVS?  This was fixed two weeks ago.

Sorry, this was my mistake. I hadn't updated my loaddefs.el.

I still get a slight discrepancy with byte compiling:

textmodes/org.el:7542:12:Warning: `make-local-hook' is an obsolete function
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
although errors are correctly highlighted.  But maybe its another autoload
problem.  I don't like updating autoloads for the whole directory because all
my local copies get added.  Does it help to check in a new ldefs-boot.el?  I'm
not sure how that works.

Nick

Re: Underlining in compile.el

[Juri Linkov]

>  > Have you tried this with the latest CVS?  This was fixed two weeks ago.
>
> Sorry, this was my mistake. I hadn't updated my loaddefs.el.

After CVS update I always run:

make; make -C lisp EMACS=../src/emacs recompile updates; make; make tags; make info
                                                =======

to get reliable results without making complete bootstrap.
Maybe there should be a special make target that combines
both `recompile' and `updates'?

Also INSTALL.CVS could suggest using `update' in the normal updating
procedure, because errors caused by not updating loaddefs.el might be
too subtle to indicate the need to update loaddefs.el.  This means
to suggest users always run

    $ make recompile update EMACS=../src/emacs

> I still get a slight discrepancy with byte compiling:
>
> textmodes/org.el:7542:12:Warning: `make-local-hook' is an obsolete function
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This highlighting is intentional.  The `gnu' rule in
`compilation-error-regexp-alist-alist' matches `[Ww]arning\\|'.
I don't know why this was added.

BTW, there is also a similar problem in grep matches.  When a matched
line starts with a number and colon, then it becomes highlighted as
a column number.  Maybe it helps to split the grep rules into smaller
parts as Richard suggested, so if the used grep doesn't support output
of column number then grep.el won't interpret such numbers as
column numbers.

> although errors are correctly highlighted.  But maybe its another
> autoload problem.  I don't like updating autoloads for the whole
> directory because all my local copies get added.

I don't understand your problem.  Why don't you like to add your
local copies?

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

Re: Underlining in compile.el

[Juri Linkov]

I just noticed that there is a much simpler and more correct solution
for adding underline attributes to links in the compilation and grep
buffers than using new special faces with the underline attribute
(currently fontification with them is still wrong: some parts of links
are not underlined while some non-link parts are underlined).

There is already a face variable `compilation-message-face' with
`nil' value by default.  compile.el puts the face from this variable
exactly on the link areas active for navigation (together with
text properties `message', `help-echo' and `mouse-face'=`highlight').

So changing its default value to `underline' will do the right thing:
it will underline exactly the link areas, no more, no less.

The patch below removes the new compilation faces with underline
attributes, and changes the default value of `compilation-message-face'
to `underline'.

Index: lisp/progmodes/compile.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/compile.el,v
retrieving revision 1.369
diff -c -r1.369 compile.el
*** lisp/progmodes/compile.el	6 Aug 2005 01:10:09 -0000	1.369
--- lisp/progmodes/compile.el	6 Aug 2005 18:31:58 -0000
***************
*** 377,389 ****
     '(;; configure output lines.
       ("^[Cc]hecking \\(?:[Ff]or \\|[Ii]f \\|[Ww]hether \\(?:to \\)?\\)?\\(.+\\)\\.\\.\\. *\\(?:(cached) *\\)?\\(\\(yes\\(?: .+\\)?\\)\\|no\\|\\(.*\\)\\)$"
        (1 font-lock-variable-name-face)
!       (2 (compilation-text-face '(4 . 3))))
       ;; Command output lines.  Recognize `make[n]:' lines too.
       ("^\\([[:alnum:]_/.+-]+\\)\\(\\[\\([0-9]+\\)\\]\\)?[ \t]*:"
        (1 font-lock-function-name-face) (3 compilation-line-face nil t))
       (" --?o\\(?:utfile\\|utput\\)?[= ]?\\(\\S +\\)" . 1)
!      ("^Compilation finished" . compilation-info-text-face)
!      ("^Compilation exited abnormally" . compilation-error-text-face))
     "Additional things to highlight in Compilation mode.
  This gets tacked on the end of the generated expressions.")
  
--- 377,389 ----
     '(;; configure output lines.
       ("^[Cc]hecking \\(?:[Ff]or \\|[Ii]f \\|[Ww]hether \\(?:to \\)?\\)?\\(.+\\)\\.\\.\\. *\\(?:(cached) *\\)?\\(\\(yes\\(?: .+\\)?\\)\\|no\\|\\(.*\\)\\)$"
        (1 font-lock-variable-name-face)
!       (2 (compilation-face '(4 . 3))))
       ;; Command output lines.  Recognize `make[n]:' lines too.
       ("^\\([[:alnum:]_/.+-]+\\)\\(\\[\\([0-9]+\\)\\]\\)?[ \t]*:"
        (1 font-lock-function-name-face) (3 compilation-line-face nil t))
       (" --?o\\(?:utfile\\|utput\\)?[= ]?\\(\\S +\\)" . 1)
!      ("^Compilation finished" . compilation-info-face)
!      ("^Compilation exited abnormally" . compilation-error-face))
     "Additional things to highlight in Compilation mode.
  This gets tacked on the end of the generated expressions.")
  
***************
*** 499,563 ****
  ;; backward-compatibility alias
  (put 'compilation-info-face 'face-alias 'compilation-info)
  
- (defface compilation-error-file-name
-   '((default :inherit compilation-error)
-     (((supports :underline t)) :underline t))
-   "Face for displaying file names in error messages."
-   :group 'font-lock-highlighting-faces
-   :version "22.1")
- 
- (defface compilation-warning-file-name
-   '((default :inherit compilation-warning)
-     (((supports :underline t)) :underline t))
-   "Face for displaying file names in warning messages."
-   :group 'font-lock-highlighting-faces
-   :version "22.1")
- 
- (defface compilation-info-file-name
-   '((default :inherit compilation-info)
-     (((supports :underline t)) :underline t))
-   "Face for displaying file names in informational messages."
-   :group 'font-lock-highlighting-faces
-   :version "22.1")
- 
  (defface compilation-line-number
!   '((default :inherit font-lock-variable-name-face)
!     (((supports :underline t)) :underline t))
    "Face for displaying line numbers in compiler messages."
    :group 'font-lock-highlighting-faces
    :version "22.1")
  
  (defface compilation-column-number
!   '((default :inherit font-lock-type-face)
!     (((supports :underline t)) :underline t))
    "Face for displaying column numbers in compiler messages."
    :group 'font-lock-highlighting-faces
    :version "22.1")
  
! (defvar compilation-message-face nil
    "Face name to use for whole messages.
  Faces `compilation-error-face', `compilation-warning-face',
  `compilation-info-face', `compilation-line-face' and
  `compilation-column-face' get prepended to this, when applicable.")
  
! (defvar compilation-error-face 'compilation-error-file-name
    "Face name to use for file name in error messages.")
  
! (defvar compilation-error-text-face 'compilation-error
!   "Face name to use for text of error messages.")
! 
! (defvar compilation-warning-face 'compilation-warning-file-name
    "Face name to use for file name in warning messages.")
  
! (defvar compilation-warning-text-face 'compilation-warning
!   "Face name to use for text of warning messages.")
! 
! (defvar compilation-info-face 'compilation-info-file-name
    "Face name to use for file name in informational messages.")
  
- (defvar compilation-info-text-face 'compilation-info
-   "Face name to use for text of informational messages.")
- 
  (defvar compilation-line-face 'compilation-line-number
    "Face name to use for line numbers in compiler message.")
  
--- 499,531 ----
  ;; backward-compatibility alias
  (put 'compilation-info-face 'face-alias 'compilation-info)
  
  (defface compilation-line-number
!   '((t :inherit font-lock-variable-name-face))
    "Face for displaying line numbers in compiler messages."
    :group 'font-lock-highlighting-faces
    :version "22.1")
  
  (defface compilation-column-number
!   '((t :inherit font-lock-type-face))
    "Face for displaying column numbers in compiler messages."
    :group 'font-lock-highlighting-faces
    :version "22.1")
  
! (defvar compilation-message-face 'underline
    "Face name to use for whole messages.
  Faces `compilation-error-face', `compilation-warning-face',
  `compilation-info-face', `compilation-line-face' and
  `compilation-column-face' get prepended to this, when applicable.")
  
! (defvar compilation-error-face 'compilation-error
    "Face name to use for file name in error messages.")
  
! (defvar compilation-warning-face 'compilation-warning
    "Face name to use for file name in warning messages.")
  
! (defvar compilation-info-face 'compilation-info
    "Face name to use for file name in informational messages.")
  
  (defvar compilation-line-face 'compilation-line-number
    "Face name to use for line numbers in compiler message.")
  
***************
*** 584,594 ****
    (or (and (car type) (match-end (car type)) compilation-warning-face)
        (and (cdr type) (match-end (cdr type)) compilation-info-face)
        compilation-error-face))
- 
- (defun compilation-text-face (type)
-   (or (and (car type) (match-end (car type)) compilation-warning-text-face)
-       (and (cdr type) (match-end (cdr type)) compilation-info-text-face)
-       compilation-error-text-face))
  
  ;; Internal function for calculating the text properties of a directory
  ;; change message.  The directory property is important, because it is
--- 552,557 ----

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

Re: Underlining in compile.el

[Richard M. Stallman]

It's a nice simplification, so please do install it.