why does occur depropertize matching lines?

why does occur depropertize matching lines?

[Drew Adams]

Don't want to start a holy war. Just a question.

Occur now depropertizes the matching lines it copies to the occur buffer.
Why?

Previously (at least in Emacs 20), this wasn't done, and font-lock
highlighting from the source buffer could often provide visual info in the
occur buffer. For example, with font-lock on in Emacs Lisp mode, it's a lot
easier to see that a line is commented-out by its color than by checking to
see if the line starts with ";".

Imagine several similar lines, some of which are commented out. I wager that
with the depropertized occur buffer you will spend more time clicking and
checking which line is which -- or at least spend more time checking for the
";" at the beginning of a line. If the highlighting is transferred to the
occur buffer, you have a lot more visual feedback to go on.

My 2 cents. I'm sure there are good reasons for the change.

 - Drew

RE: why does occur depropertize matching lines?

[Drew Adams]

I got no response to this question (10/05), and I'm still curious:

    Occur now depropertizes the matching lines it copies to the
    occur buffer. Why?

    Previously, this wasn't done, and font-lock highlighting from the source
    buffer could often provide useful visual info in the occur buffer.

    For example, in Emacs Lisp mode, it's easier to see that a line is
    commented-out by its color than by checking to see if it starts with
";".

Re: why does occur depropertize matching lines?

[Richard Stallman]

	Occur now depropertizes the matching lines it copies to the
	occur buffer. Why?

I think it was because some properties would cause trouble and
confusion (such as keymaps, mouse face).

RE: why does occur depropertize matching lines?

[Drew Adams]

    	Occur now depropertizes the matching lines it copies to the
    	occur buffer. Why?

    I think it was because some properties would cause trouble and
    confusion (such as keymaps, mouse face).

That makes sense.

It's too bad, however, if plain old faces (font-lock highlighting) can't be
preserved. As I said in my original email:

        Imagine several similar lines, some of which are commented out.
        I wager that with the depropertized occur buffer you will spend
        more time clicking and checking which line is which -- or at least
        spend more time checking for the ";" at the beginning of a line.
        If the highlighting is transferred to the occur buffer, you have a
        lot more visual feedback to go on.

Is there no (simple) way to distinguish plain highlighting text properties
from the other stuff?

I think this is a considerable setback wrt usability: it represents an
information loss for users. I don't know how much of the pb was program
"trouble" and how much was user "confusion", but I'm one user who would be
willing to trade, say, the confusion of some extra, ineffective mouse-face
highlighting for the benefit of informative syntactic highlighting. Such
highlighting is all the more helpful when lines have been taken out of
context (as in occur).

Re: why does occur depropertize matching lines?

[Juri Linkov]

"Drew Adams" <drew.adams@oracle.com> writes:
> I think this is a considerable setback wrt usability: it represents an
> information loss for users. I don't know how much of the pb was program
> "trouble" and how much was user "confusion", but I'm one user who would be
> willing to trade, say, the confusion of some extra, ineffective mouse-face
> highlighting for the benefit of informative syntactic highlighting. Such
> highlighting is all the more helpful when lines have been taken out of
> context (as in occur).

Even now occur partly preserves highlighting for some lines: when
called with a numeric prefix argument occur displays context lines
highlighted with their original faces whereas highlighting from matching
lines is removed.  This is too weird.  I can't find a reason for that.

I think occur should preserve highlighting of matching lines as well
(or at least preserve face properties if other properties are too confusing).

However, I see another problem: matches are not highlighted anymore
with `list-matching-lines-face' because when a character has two properties

  font-lock-face [bold]                   ;; from `list-matching-lines-face'
  face           [font-lock-keyword-face] ;; from source buffer

`face' property takes precedence over `font-lock-face'.

If it would be possible to merge these two properties somehow...

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

Re: why does occur depropertize matching lines?

[Richard Stallman]

    Even now occur partly preserves highlighting for some lines: when
    called with a numeric prefix argument occur displays context lines
    highlighted with their original faces whereas highlighting from matching
    lines is removed.  This is too weird.  I can't find a reason for that.

I discovered that changing occur-1 to pass t for the keep-props
argument to occur-engine inverts this: it preserves the highlighting
for the actual target line, and discards it for the context lines.

That seems like a good idea to me; what do you think?

Here's the patch that does it.

*** replace.el	20 Nov 2004 16:58:11 -0500	1.190
--- replace.el	24 Nov 2004 20:51:24 -0500	
***************
*** 915,921 ****
  		    (and case-fold-search
  			 (isearch-no-upper-case-p regexp t))
  		    list-matching-lines-buffer-name-face
! 		    nil list-matching-lines-face nil)))
  	(let* ((bufcount (length active-bufs))
  	       (diff (- (length bufs) bufcount)))
  	  (message "Searched %d buffer%s%s; %s match%s for `%s'"
--- 915,921 ----
  		    (and case-fold-search
  			 (isearch-no-upper-case-p regexp t))
  		    list-matching-lines-buffer-name-face
! 		    nil list-matching-lines-face t)))
  	(let* ((bufcount (length active-bufs))
  	       (diff (- (length bufs) bufcount)))
  	  (message "Searched %d buffer%s%s; %s match%s for `%s'"


    However, I see another problem: matches are not highlighted anymore
    with `list-matching-lines-face' because when a character has two properties

      font-lock-face [bold]                   ;; from `list-matching-lines-face'
      face           [font-lock-keyword-face] ;; from source buffer

    `face' property takes precedence over `font-lock-face'.

I wonder why occur-engine uses font-lock-face.  It is not font lock
mode, after all.  Changing it to use `face' instead makes it seem
to do the right thing.  Does it cause any problem?

*** replace.el	20 Nov 2004 16:58:11 -0500	1.190
--- replace.el	24 Nov 2004 20:53:47 -0500	
***************
*** 998,1004 ****
  					     (append
  					      `(occur-match t)
  					      (when match-face
! 						`(font-lock-face ,match-face)))
  					     curstring)
  			(setq start (match-end 0))))
  		    ;; Generate the string to insert for this match
--- 998,1004 ----
  					     (append
  					      `(occur-match t)
  					      (when match-face
! 						`(face ,match-face)))
  					     curstring)
  			(setq start (match-end 0))))
  		    ;; Generate the string to insert for this match

Re: why does occur depropertize matching lines?

[Juri Linkov]

Richard Stallman <rms@gnu.org> writes:
> I discovered that changing occur-1 to pass t for the keep-props
> argument to occur-engine inverts this: it preserves the highlighting
> for the actual target line, and discards it for the context lines.

Inverting the meaning of the `keep-props' argument for context lines is
too unnatural thing to do.  It's more natural to expect that users
might want to highlight either everything (matching and context lines)
or nothing.  So I think the argument of `occur-accumulate-lines'
should be reversed.

> That seems like a good idea to me; what do you think?

I suppose someone might not like to keep highlighting at all.
How about a user option for it?

Index: lisp/replace.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/replace.el,v
retrieving revision 1.190
diff -u -r1.190 replace.el
--- lisp/replace.el	20 Nov 2004 22:09:50 -0000	1.190
+++ lisp/replace.el	25 Nov 2004 07:48:48 -0000
@@ -636,6 +636,11 @@
   :type 'hook
   :group 'matching)
 
+(defcustom occur-keep-props nil
+  "Non-nil keeps text properties in *Occur* buffer."
+  :type 'boolean
+  :group 'matching)
+
 (put 'occur-mode 'mode-class 'special)
 (defun occur-mode ()
   "Major mode for output from \\[occur].
@@ -768,7 +774,7 @@
   :type 'face
   :group 'matching)
 
-(defun occur-accumulate-lines (count &optional no-props)
+(defun occur-accumulate-lines (count &optional keep-props)
   (save-excursion
     (let ((forwardp (> count 0))
 	  (result nil))
@@ -778,9 +784,9 @@
 			(bobp))))
 	(setq count (+ count (if forwardp -1 1)))
 	(push
-	 (funcall (if no-props
-		      #'buffer-substring-no-properties
-		    #'buffer-substring)
+	 (funcall (if keep-props
+		      #'buffer-substring
+		    #'buffer-substring-no-properties)
 	  (line-beginning-position)
 	  (line-end-position))
 	 result)
@@ -915,7 +921,7 @@
 		    (and case-fold-search
 			 (isearch-no-upper-case-p regexp t))
 		    list-matching-lines-buffer-name-face
-		    nil list-matching-lines-face nil)))
+		    nil list-matching-lines-face occur-keep-props)))
 	(let* ((bufcount (length active-bufs))
 	       (diff (- (length bufs) bufcount)))
 	  (message "Searched %d buffer%s%s; %s match%s for `%s'"



> I wonder why occur-engine uses font-lock-face.  It is not font lock
> mode, after all.  Changing it to use `face' instead makes it seem
> to do the right thing.  Does it cause any problem?

Perhaps, yes.

The default value of `occur-mode-hook' is '(turn-on-font-lock),
and `occur-mode' also adds `font-lock-defontify' hook to
`change-major-mode-hook'.

I don't know for what purpose these font-lock related settings were
added, so I can't tell what kinds of problems removing `font-lock-face'
property might cause.

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

Re: why does occur depropertize matching lines?

[Richard Stallman]

    Inverting the meaning of the `keep-props' argument for context lines is
    too unnatural thing to do.  It's more natural to expect that users
    might want to highlight either everything (matching and context lines)
    or nothing.

I think users will like the behavior that results from the changes I
wrote.  I like it, and it seems natural to me.  Why wouldn't other
people like it?

Re: why does occur depropertize matching lines?

[Juri Linkov]

Richard Stallman <rms@gnu.org> writes:
>     Inverting the meaning of the `keep-props' argument for context lines is
>     too unnatural thing to do.  It's more natural to expect that users
>     might want to highlight either everything (matching and context lines)
>     or nothing.
>
> I think users will like the behavior that results from the changes I
> wrote.  I like it, and it seems natural to me.  Why wouldn't other
> people like it?

One of the reasons why people wouldn't like it, is the fact that
when the source buffer is not yet completely fontified by background
fontification, some lines copied to the Occur buffer are not fontified.
Such party fontified Occur buffer looks too ugly.  Perhaps, the right
thing to do is to call (font-lock-fontify-region (point-min) (point-max))
on the source buffer from `occur-engine' before it starts to copy
source lines to the Occur buffer?

There is another problem which arises when all font properties are
copied to the Occur buffer: matches fontified in `list-matching-lines-face'
(which is `bold' by default) don't stand out as clearly as they were
when the Occur buffer had no fontified lines at all.

It seems there is a need in a new face to display matching strings.
Such a face could be used also in other packages: to highlight matches
in *grep* buffer and apropos documentation.

A good choice for properties of such face is still unclear.
One variant is to use bold red (like grep --color uses with ANSI
sequences).  However, bold red is too loud color, it is used mostly
for error messages.  Another variant is to set some background color
like in `highlight', `isearch-lazy-highlight-face', `region' or
`secondary-selection'.

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

Re: why does occur depropertize matching lines?

[Richard Stallman]

I noticed an apparent confusion in the code that was inverting the meaning
of the KEEP-PROPS parameter; so I fixed that.  The current code
should keep the fontification of ALL the lines from the buffer.

    It seems there is a need in a new face to display matching strings.
    Such a face could be used also in other packages: to highlight matches
    in *grep* buffer and apropos documentation.

I don't entirely understand this point.  Are you saying that bold
isn't sufficient to stand out?  If so, we could change the value of
list-matching-lines-face.

Re: why does occur depropertize matching lines?

[Juri Linkov]

Richard Stallman <rms@gnu.org> writes:
> I noticed an apparent confusion in the code that was inverting the meaning
> of the KEEP-PROPS parameter; so I fixed that.  The current code
> should keep the fontification of ALL the lines from the buffer.

It keeps the fontification of all copied lines, but not all lines in
the source buffer might be fontified at the moment of calling `occur',
since background font-lock fontification might not yet fontify some
portions of the source buffer which are not visible in the window.

When such unfontified lines are copied from the partly fontified
source buffer to the Occur buffer, the result looks very bad.

What I propose is to force font-lock fontification of the whole source
buffer before starting to copy its lines to the Occur buffer.
Alternatively, it could fontify only unfontified matched lines in the
source buffer before copying them.

>     It seems there is a need in a new face to display matching strings.
>     Such a face could be used also in other packages: to highlight matches
>     in *grep* buffer and apropos documentation.
>
> I don't entirely understand this point.  Are you saying that bold
> isn't sufficient to stand out?

Yes, in heavily highlighted text bold doesn't stand out sufficiently.

> If so, we could change the value of list-matching-lines-face.

Of course, but I can't find a good face to put into this face variable.
It is desirable for such face to have a background color different
from default.  The most suitable faces are `isearch-lazy-highlight-face',
`region' and `secondary-selection', but when users will use isearch on
the Occur buffer, or select a region on it, then text highlighted by them
will become indistinguishable from matching lines with the same color.

So it makes sense to add a new `match' face for matches in the Occur
buffer as well as for matches in grep output which is very similar by
functionality to occur.

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

Re: why does occur depropertize matching lines?

[Stefan Monnier]

> Yes, in heavily highlighted text bold doesn't stand out sufficiently.

That's the problem: the highlight we copy from the source buffer might take
*any* form.  It could very well be the exact same highlight used for by
occur itself.

What we really want is to copy the face info from the source but "dim" it
a bit so it doesn't overshadow occur's own highlighting.


        Stefan

Re: why does occur depropertize matching lines?

[Juri Linkov]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Yes, in heavily highlighted text bold doesn't stand out sufficiently.
>
> That's the problem: the highlight we copy from the source buffer
> might take *any* form.  It could very well be the exact same
> highlight used for by occur itself.

Most faces don't change background color.  Only very special faces
(that are very rarely used in buffers on which occur is usually
called) have background different from default.  So any face with
a different background would be visible among highlighted text with
no background.

> What we really want is to copy the face info from the source but "dim" it
> a bit so it doesn't overshadow occur's own highlighting.

There is one problem with dimmed faces: they might be not readable,
especially for visually impaired users.

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

Re: why does occur depropertize matching lines?

[Richard Stallman]

    Most faces don't change background color.  Only very special faces
    (that are very rarely used in buffers on which occur is usually
    called) have background different from default.

Yes, most such faces are used for temporary highlighting.

    > What we really want is to copy the face info from the source but "dim" it
    > a bit so it doesn't overshadow occur's own highlighting.

    There is one problem with dimmed faces: they might be not readable,
    especially for visually impaired users.

On some terminals, such dimming is not even possible, due to the limited
number of supported colors.

So I think you're right that we can simply change that face.
How about if you define a new face for the purpose, and set it
up in a way you think is good?  Please give pay attention to both
the light-background case and the dark-background case.

RE: why does occur depropertize matching lines?

[Drew Adams]

        Most faces don't change background color.  Only very special faces
        (that are very rarely used in buffers on which occur is usually
        called) have background different from default.

    Yes, most such faces are used for temporary highlighting.

I don't think these assumptions should be made. Users can define faces (for
font-lock or anything else) any way they want, including changing the
background.

        > What we really want is to copy the face info from the source
        > but "dim" it a bit so it doesn't overshadow occur's own
        > highlighting.

        There is one problem with dimmed faces: they might be not readable,
        especially for visually impaired users.

    On some terminals, such dimming is not even possible, due to the limited
    number of supported colors.

I wouldn't want dimming, anyway.

Any highlighting that `occur' does should just be done without regard to the
highlighting already there. If it turns out that `occur's highlighting uses
face `foobar', and that is confusing in my context because I use `foobar'
for font-locking, I can change the face used for `occur' highlighting
(assuming it's not hard-coded!).

Re: why does occur depropertize matching lines?

[Josh Varner]

On Wed, 8 Dec 2004 11:06:30 -0800, Drew Adams <drew.adams@oracle.com> wrote:
>         Most faces don't change background color.  Only very special faces
>         (that are very rarely used in buffers on which occur is usually
>         called) have background different from default.
> 
>     Yes, most such faces are used for temporary highlighting.
> 
> I don't think these assumptions should be made. Users can define faces (for
> font-lock or anything else) any way they want, including changing the
> background.
> 

If you don't want the face changed why not add an overlay arrow like
in gdb mode?

Josh

RE: why does occur depropertize matching lines?

[Drew Adams]

    >         Most faces don't change background color.  Only very
    special faces
    >         (that are very rarely used in buffers on which occur
    is usually
    >         called) have background different from default.
    >
    >     Yes, most such faces are used for temporary highlighting.
    >
    > I don't think these assumptions should be made. Users can
    define faces (for
    > font-lock or anything else) any way they want, including changing the
    > background.
    >

    If you don't want the face changed why not add an overlay arrow like
    in gdb mode?

Sorry, I don't understand what you are suggesting. Don't want what face
changed?

My point was that we should not assume that most faces do not change the
background color ("very rarely") and that most such faces are only for
temporary highlighting (vs, for instance, font-locking).

Re: why does occur depropertize matching lines?

[Richard Stallman]

    My point was that we should not assume that most faces do not change the
    background color ("very rarely") and that most such faces are only for
    temporary highlighting (vs, for instance, font-locking).

This is an unnecessary argument, so let's let it drop.

The default definition of list-matching-lines-face does not have
to be good for all possible situations.  It does not have to be
good in customized Emacses.  It only needs to be good in the usual
and unmodified situation.

Re: why does occur depropertize matching lines?

[Stefan Monnier]

>> What we really want is to copy the face info from the source but "dim" it
>> a bit so it doesn't overshadow occur's own highlighting.

> There is one problem with dimmed faces: they might be not readable,
> especially for visually impaired users.

I don't mean a face that makes the text "dim" (i.e. potentially unreadable).

A face mostly defines changes w.r.t the default, so by "to dim a face"
I meant to tone down those changes.  E.g. if the face was bold, use
half-bold instead; if it was bright red (with a default of black),
use a darker red; if the face was double height, use 1.5 height; ...

Of course, any change in color might lead to less readable text, so such
a feature would be tricky to make robust.  In any case we don't have any
such feature right now.  I was just mentioning it in terms of "what would be
the ideal behavior IMO", to have something to compare to.


        Stefan

Re: why does occur depropertize matching lines?

[Richard Stallman]

    What I propose is to force font-lock fontification of the whole source
    buffer before starting to copy its lines to the Occur buffer.

No thanks.

    Alternatively, it could fontify only unfontified matched lines in the
    source buffer before copying them.

Would you like to implement that?