unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#14645: Keep highlighting face foreground
@ 2013-06-17 22:55 Juri Linkov
  2013-06-18 23:31 ` Juri Linkov
  2020-08-13 11:27 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2013-06-17 22:55 UTC (permalink / raw)
  To: 14645

Now that we have a new function `add-face-text-property' it would be
very useful for features that add faces with a different background
to highlight matches.  For instance, `occur' uses `add-text-properties'
to add the `match' face, but currently it removes foreground colors
from the text copied from the fontified buffer.

`add-face-text-property' can help to keep the original foreground colors
in the text copied from fontified buffers.  And if someone still wants
to always use the black foreground, this is easy to do by customizing
the `match' face and explicitly selecting a foreground color.

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2013-06-13 22:08:45 +0000
+++ lisp/replace.el	2013-06-17 22:51:03 +0000
@@ -1442,13 +1460,12 @@ (defun occur-engine (regexp buffers out-
 			(setq matches (1+ matches))
 			(add-text-properties
 			 (match-beginning 0) (match-end 0)
-			 (append
-			  `(occur-match t)
-			  (when match-face
-			    ;; Use `face' rather than `font-lock-face' here
-			    ;; so as to override faces copied from the buffer.
-			    `(face ,match-face)))
-			 curstring)
+			 '(occur-match t) curstring)
+			(when match-face
+			  ;; Add `match-face' to faces copied from the buffer.
+			  (add-face-text-property
+			   (match-beginning 0) (match-end 0)
+			   match-face nil curstring))
 			(setq start (match-end 0))))
 		    ;; Generate the string to insert for this match
 		    (let* ((match-prefix






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-17 22:55 bug#14645: Keep highlighting face foreground Juri Linkov
@ 2013-06-18 23:31 ` Juri Linkov
  2013-06-19 23:14   ` Juri Linkov
                     ` (2 more replies)
  2020-08-13 11:27 ` Lars Ingebrigtsen
  1 sibling, 3 replies; 10+ messages in thread
From: Juri Linkov @ 2013-06-18 23:31 UTC (permalink / raw)
  To: 14645

Another place where `add-face-text-property' could help is for the
recently added face `info-index-match'.  Currently it looks very ugly
when the `info-index-match' face is added in the middle of a link
that breaks its continuity.  A test case is: `C-h r I match RET'.

This is because `Info-index' adds the `info-index-match' face to the
strings of the found index entries.  Later `Info-virtual-index-find-node'
inserts strings to the Info buffer.  And finally `Info-fontify-node'
puts the `font-lock-face' property with `info-xref' on links.
The `face info-index-match' takes precedence over `font-lock-face info-xref'.

This patch uses `add-face-text-property' to merge the `info-index-match'
face with the `info-xref' face.  I think it's not a problem that
it will use the text property `face' instead of `font-lock-face'
since more strategically advantageous direction of the development
for the Info reader would be to move from font-lock-mode to more
browser-like design like in eww.

However, the problem is that usually a highlighting feature puts
a highlighting face on the already fontified string, but this code
reverses the order and first it highlights matches and later puts
more regular text properties on it (so this reversal requires the non-nil
APPEND arg of `add-face-text-property').  Unfortunately, I have no better idea.

=== modified file 'lisp/info.el'
--- lisp/info.el	2013-05-27 22:42:11 +0000
+++ lisp/info.el	2013-06-18 22:57:07 +0000
@@ -4874,9 +4894,8 @@ (defun Info-fontify-node ()
 			       "mouse-2: go to this node")
 		  'mouse-face 'highlight)))
 	      (when (or not-fontified-p fontify-visited-p)
-		(put-text-property
+		(add-face-text-property
 		 (match-beginning 1) (match-end 1)
-                 'font-lock-face
                  ;; Display visited menu items in a different face
                  (if (and Info-fontify-visited-nodes
                           (save-match-data
@@ -4905,7 +4924,7 @@ (defun Info-fontify-node ()
 						  (caar hl))))
 				    (setq res (car hl) hl nil)
 				  (setq hl (cdr hl))))
-                              res))) 'info-xref-visited 'info-xref)))
+                              res))) 'info-xref-visited 'info-xref) 'append))
 	      (when (and not-fontified-p
 			 (memq Info-hide-note-references '(t hide))
 			 (not (Info-index-node)))






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-18 23:31 ` Juri Linkov
@ 2013-06-19 23:14   ` Juri Linkov
  2020-08-13 11:35     ` Lars Ingebrigtsen
  2013-06-20 23:03   ` Juri Linkov
  2020-08-13 11:32   ` Lars Ingebrigtsen
  2 siblings, 1 reply; 10+ messages in thread
From: Juri Linkov @ 2013-06-19 23:14 UTC (permalink / raw)
  To: 14645

I was going to propose a similar change to use `add-face-text-property'
in hi-lock, but it turned out to be much simpler because hi-lock
is based on font-lock that already has `font-lock-prepend-text-property'
that does the same:

=== modified file 'lisp/hi-lock.el'
--- lisp/hi-lock.el	2013-06-03 08:51:50 +0000
+++ lisp/hi-lock.el	2013-06-19 23:12:05 +0000
@@ -715,7 +690,7 @@ (defun hi-lock-set-pattern (regexp face)
   "Highlight REGEXP with face FACE."
   ;; Hashcons the regexp, so it can be passed to remove-overlays later.
   (setq regexp (hi-lock--hashcons regexp))
-  (let ((pattern (list regexp (list 0 (list 'quote face) t))))
+  (let ((pattern (list regexp (list 0 (list 'quote face) 'prepend))))
     ;; Refuse to highlight a text that is already highlighted.
     (unless (assoc regexp hi-lock-interactive-patterns)
       (push pattern hi-lock-interactive-patterns)






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-18 23:31 ` Juri Linkov
  2013-06-19 23:14   ` Juri Linkov
@ 2013-06-20 23:03   ` Juri Linkov
  2020-08-13 11:32   ` Lars Ingebrigtsen
  2 siblings, 0 replies; 10+ messages in thread
From: Juri Linkov @ 2013-06-20 23:03 UTC (permalink / raw)
  To: 14645

> This patch uses `add-face-text-property' to merge the `info-index-match'
> face with the `info-xref' face.

Using the text property `face' instead of `font-lock-face'
might break something, so a better patch below removes
the text properties `face info-index-match' from the Info buffer
and replaces them with `font-lock-face info-index-match'
in `Info-virtual-index-find-node'.  And `Info-fontify-node'
merges them with its `font-lock-face' text properties using
`font-lock-prepend-text-property'.  Here is a summary:

1. `Info-index' puts `info-index-match face' on index strings.
2. `Info-virtual-index-find-node' inserts them into the Info buffer.
3. It also converts text properties `face info-index-match' to
   `font-lock-face info-index-match' in the Info buffer.
4. `Info-fontify-node' prepends `font-lock-face info-xref' to links.

=== modified file 'lisp/info.el'
--- lisp/info.el	2013-05-27 22:42:11 +0000
+++ lisp/info.el	2013-06-20 22:59:01 +0000
@@ -3417,7 +3439,19 @@ (defun Info-virtual-index-find-node (fil
 			    (nth 1 entry)
 			    (if (nth 3 entry)
 				(format " (line %s)" (nth 3 entry))
-			      ""))))))
+			      ""))))
+	  ;; Convert text properties `face info-index-match' to
+	  ;; `font-lock-face info-index-match'.
+	  (save-excursion
+	    (let ((beg (text-property-any (point-min) (point-max) 'face 'info-index-match))
+		  (end nil)
+		  (inhibit-read-only t))
+	      (while beg
+		(goto-char beg)
+		(when (setq end (next-single-property-change beg 'face))
+		  (font-lock-prepend-text-property beg end 'font-lock-face 'info-index-match)
+		  (remove-text-properties beg end '(face info-index-match)))
+		(setq beg (text-property-any end (point-max) 'face 'info-index-match)))))))
     ;; Else, Generate a list of previous search results
     (let ((nodes (reverse Info-virtual-index-nodes)))
       (insert (format "\n\^_\nFile: %s,  Node: %s,  Up: Top\n\n"
@@ -3532,7 +3567,19 @@ (defun Info-apropos-find-node (_filename
 			    (nth 2 entry)
 			    (if (nth 3 entry)
 				(format " (line %s)" (nth 3 entry))
-			      "")))))))))
+			      ""))))
+	  ;; Convert text properties `face info-index-match' to
+	  ;; `font-lock-face info-index-match'.
+	  (save-excursion
+	    (let ((beg (text-property-any (point-min) (point-max) 'face 'info-index-match))
+		  (end nil)
+		  (inhibit-read-only t))
+	      (while beg
+		(goto-char beg)
+		(when (setq end (next-single-property-change beg 'face))
+		  (font-lock-prepend-text-property beg end 'font-lock-face 'info-index-match)
+		  (remove-text-properties beg end '(face info-index-match)))
+		(setq beg (text-property-any end (point-max) 'face 'info-index-match))))))))))
 
 (defun Info-apropos-matches (string)
   "Collect STRING matches from all known Info files on your system.
@@ -4755,7 +4811,7 @@ (defun Info-fontify-node ()
             (when (or not-fontified-p fontify-visited-p)
               (setq rbeg (match-beginning 2)
                     rend (match-end 2))
-              (put-text-property
+              (font-lock-prepend-text-property
                rbeg rend
                'font-lock-face
                ;; Display visited nodes in a different face
@@ -4874,7 +4930,7 @@ (defun Info-fontify-node ()
 			       "mouse-2: go to this node")
 		  'mouse-face 'highlight)))
 	      (when (or not-fontified-p fontify-visited-p)
-		(put-text-property
+		(font-lock-prepend-text-property
 		 (match-beginning 1) (match-end 1)
                  'font-lock-face
                  ;; Display visited menu items in a different face






^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-17 22:55 bug#14645: Keep highlighting face foreground Juri Linkov
  2013-06-18 23:31 ` Juri Linkov
@ 2020-08-13 11:27 ` Lars Ingebrigtsen
  1 sibling, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-13 11:27 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 14645

Juri Linkov <juri@jurta.org> writes:

> Now that we have a new function `add-face-text-property' it would be
> very useful for features that add faces with a different background
> to highlight matches.  For instance, `occur' uses `add-text-properties'
> to add the `match' face, but currently it removes foreground colors
> from the text copied from the fontified buffer.

[...]

> +			 '(occur-match t) curstring)
> +			(when match-face
> +			  ;; Add `match-face' to faces copied from the buffer.
> +			  (add-face-text-property
> +			   (match-beginning 0) (match-end 0)
> +			   match-face nil curstring))

Looks like a variation of this has been applied already.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-18 23:31 ` Juri Linkov
  2013-06-19 23:14   ` Juri Linkov
  2013-06-20 23:03   ` Juri Linkov
@ 2020-08-13 11:32   ` Lars Ingebrigtsen
  2020-08-13 23:44     ` Juri Linkov
  2 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-13 11:32 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 14645

Juri Linkov <juri@jurta.org> writes:

> Another place where `add-face-text-property' could help is for the
> recently added face `info-index-match'.  Currently it looks very ugly
> when the `info-index-match' face is added in the middle of a link
> that breaks its continuity.  A test case is: `C-h r I match RET'.
>
> This is because `Info-index' adds the `info-index-match' face to the
> strings of the found index entries.  Later `Info-virtual-index-find-node'
> inserts strings to the Info buffer.  And finally `Info-fontify-node'
> puts the `font-lock-face' property with `info-xref' on links.
> The `face info-index-match' takes precedence over `font-lock-face info-xref'.

This hadn't been fixed, so I applied your patch to Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2013-06-19 23:14   ` Juri Linkov
@ 2020-08-13 11:35     ` Lars Ingebrigtsen
  0 siblings, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-13 11:35 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 14645

Juri Linkov <juri@jurta.org> writes:

> I was going to propose a similar change to use `add-face-text-property'
> in hi-lock, but it turned out to be much simpler because hi-lock
> is based on font-lock that already has `font-lock-prepend-text-property'
> that does the same:
>
> === modified file 'lisp/hi-lock.el'
> --- lisp/hi-lock.el	2013-06-03 08:51:50 +0000
> +++ lisp/hi-lock.el	2013-06-19 23:12:05 +0000
> @@ -715,7 +690,7 @@ (defun hi-lock-set-pattern (regexp face)
>    "Highlight REGEXP with face FACE."
>    ;; Hashcons the regexp, so it can be passed to remove-overlays later.
>    (setq regexp (hi-lock--hashcons regexp))
> -  (let ((pattern (list regexp (list 0 (list 'quote face) t))))
> +  (let ((pattern (list regexp (list 0 (list 'quote face) 'prepend))))

If I'm reading this correctly, something similar has been added in the
intervening years:

  (let ((pattern (list (lambda (limit)
                         (let ((case-fold-search case-fold)
                               (search-spaces-regexp spaces-regexp))
                           (re-search-forward regexp limit t)))
                       (list subexp (list 'quote face) 'prepend)))

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2020-08-13 11:32   ` Lars Ingebrigtsen
@ 2020-08-13 23:44     ` Juri Linkov
  2020-08-14  6:20       ` Eli Zaretskii
  2020-08-14  9:50       ` Lars Ingebrigtsen
  0 siblings, 2 replies; 10+ messages in thread
From: Juri Linkov @ 2020-08-13 23:44 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 14645

>> Another place where `add-face-text-property' could help is for the
>> recently added face `info-index-match'.  Currently it looks very ugly
>> when the `info-index-match' face is added in the middle of a link
>> that breaks its continuity.  A test case is: `C-h r I match RET'.
>>
>> This is because `Info-index' adds the `info-index-match' face to the
>> strings of the found index entries.  Later `Info-virtual-index-find-node'
>> inserts strings to the Info buffer.  And finally `Info-fontify-node'
>> puts the `font-lock-face' property with `info-xref' on links.
>> The `face info-index-match' takes precedence over `font-lock-face info-xref'.
>
> This hadn't been fixed, so I applied your patch to Emacs 28.

This patch breaks Info fontification, please revert it.
Here is what I said in the message sent later with another patch at
https://debbugs.gnu.org/14645#14

  Using the text property `face' instead of `font-lock-face'
  might break something, so a better patch below removes
  the text properties `face info-index-match' from the Info buffer

This is why I posted a better patch.  But please give me more time
to test the second better patch, I'm not yet sure how well it works now.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2020-08-13 23:44     ` Juri Linkov
@ 2020-08-14  6:20       ` Eli Zaretskii
  2020-08-14  9:50       ` Lars Ingebrigtsen
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-08-14  6:20 UTC (permalink / raw)
  To: Juri Linkov; +Cc: larsi, 14645

> From: Juri Linkov <juri@jurta.org>
> Date: Fri, 14 Aug 2020 02:44:43 +0300
> Cc: 14645@debbugs.gnu.org
> 
> > This hadn't been fixed, so I applied your patch to Emacs 28.
> 
> This patch breaks Info fontification, please revert it.

> Here is what I said in the message sent later with another patch at
> https://debbugs.gnu.org/14645#14
> 
>   Using the text property `face' instead of `font-lock-face'
>   might break something, so a better patch below removes
>   the text properties `face info-index-match' from the Info buffer

Thanks for alerting us to the breakage, but would it be possible to
have a test which would make sure the fontification functionality in
Info is not broken, so that we don't have to rely on the memory and
attention of those who are experts on that?





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#14645: Keep highlighting face foreground
  2020-08-13 23:44     ` Juri Linkov
  2020-08-14  6:20       ` Eli Zaretskii
@ 2020-08-14  9:50       ` Lars Ingebrigtsen
  1 sibling, 0 replies; 10+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-14  9:50 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 14645

Juri Linkov <juri@jurta.org> writes:

> This patch breaks Info fontification, please revert it.

OK, reverted.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-14  9:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-17 22:55 bug#14645: Keep highlighting face foreground Juri Linkov
2013-06-18 23:31 ` Juri Linkov
2013-06-19 23:14   ` Juri Linkov
2020-08-13 11:35     ` Lars Ingebrigtsen
2013-06-20 23:03   ` Juri Linkov
2020-08-13 11:32   ` Lars Ingebrigtsen
2020-08-13 23:44     ` Juri Linkov
2020-08-14  6:20       ` Eli Zaretskii
2020-08-14  9:50       ` Lars Ingebrigtsen
2020-08-13 11:27 ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).