* bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property @ 2016-01-07 16:52 Clément Pit--Claudel 2016-01-07 18:22 ` Eli Zaretskii 0 siblings, 1 reply; 4+ messages in thread From: Clément Pit--Claudel @ 2016-01-07 16:52 UTC (permalink / raw) To: 22323 [-- Attachment #1: Type: text/plain, Size: 1573 bytes --] Hi all, Font fallback seems to break face stacking for invisible overlays: This works fine: (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed") (erase-buffer) (fundamental-mode) (add-to-invisibility-spec '(outline . t)) (insert "abc!!def!!ghi") (let ((ov (make-overlay 6 9))) (overlay-put ov 'invisible 'outline)) (let ((ov (make-overlay (point-min) (point-max)))) (overlay-put ov 'face 'region)) (pop-to-buffer (current-buffer))) This doesn't work (assuming that ℙ is not in your usual font): (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed") (erase-buffer) (fundamental-mode) (add-to-invisibility-spec '(outline . t)) (insert "abcℙℙdefℙℙghi") (let ((ov (make-overlay 6 9))) (overlay-put ov 'invisible 'outline)) (let ((ov (make-overlay (point-min) (point-max)))) (overlay-put ov 'face 'region)) (pop-to-buffer (current-buffer))) I came across this while using prettify-symbols-mode: (with-current-buffer (get-buffer-create "With prettification to common character: ellispis is highlighed") (erase-buffer) (fundamental-mode) (add-to-invisibility-spec '(outline . t)) (insert "abc!!def!!ghi") (setq prettify-symbols-alist '(("!!" . ?ℙ))) (prettify-symbols-mode) (let ((ov (make-overlay 6 9))) (overlay-put ov 'invisible 'outline)) (let ((ov (make-overlay (point-min) (point-max)))) (overlay-put ov 'face 'region)) (pop-to-buffer (current-buffer))) Cheers, Clément. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property 2016-01-07 16:52 bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property Clément Pit--Claudel @ 2016-01-07 18:22 ` Eli Zaretskii 2016-01-08 1:19 ` Clément Pit--Claudel 0 siblings, 1 reply; 4+ messages in thread From: Eli Zaretskii @ 2016-01-07 18:22 UTC (permalink / raw) To: Clément Pit--Claudel; +Cc: 22323 > From: Clément Pit--Claudel <clement.pitclaudel@live.com> > Date: Thu, 7 Jan 2016 11:52:55 -0500 > > Font fallback seems to break face stacking for invisible overlays: > > This works fine: > > (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed") > (erase-buffer) > (fundamental-mode) > (add-to-invisibility-spec '(outline . t)) > (insert "abc!!def!!ghi") > (let ((ov (make-overlay 6 9))) > (overlay-put ov 'invisible 'outline)) > (let ((ov (make-overlay (point-min) (point-max)))) > (overlay-put ov 'face 'region)) > (pop-to-buffer (current-buffer))) > > This doesn't work (assuming that ℙ is not in your usual font): > > (with-current-buffer (get-buffer-create "No prettification: ellispis is highlighed") > (erase-buffer) > (fundamental-mode) > (add-to-invisibility-spec '(outline . t)) > (insert "abcℙℙdefℙℙghi") > (let ((ov (make-overlay 6 9))) > (overlay-put ov 'invisible 'outline)) > (let ((ov (make-overlay (point-min) (point-max)))) > (overlay-put ov 'face 'region)) > (pop-to-buffer (current-buffer))) This is indeed the same issue as bug#22320. Using a different font requires a different face, so you have here exactly the same situation as in #22320: the last visible character ℙ has a face that is different from the first invisible character d. ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property 2016-01-07 18:22 ` Eli Zaretskii @ 2016-01-08 1:19 ` Clément Pit--Claudel 2016-01-08 10:22 ` Eli Zaretskii 0 siblings, 1 reply; 4+ messages in thread From: Clément Pit--Claudel @ 2016-01-08 1:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 22323 [-- Attachment #1: Type: text/plain, Size: 3073 bytes --] On 01/07/2016 04:02 PM, Eli Zaretskii wrote: >> Date: Thu, 07 Jan 2016 22:12:42 +0200 >> From: Eli Zaretskii <eliz@gnu.org> >> Cc: 22320@debbugs.gnu.org >> >>> The same problems exist for composition, but keeping the properties of the first character seems to work well there; maybe we could consider harmonizing both behaviors? >> >> I'm not sure I understand what exactly are you proposing to do. We >> cannot treat invisible text like we treat character compositions, each >> one invokes a very different machinery with distinct and very >> different features. > > Here's a suggestion: ignore the face of the invisible text altogether, > and instead always use the face of the last visible character. The > patch to do that is below; it fixes all of your test cases. But since > you would like to see the face of the invisible text show through, I'm > not sure you will like it... > > WDYT? I like it :) I find it much more consistent that what we currently have. Plus, has a desirable property that you pointed out and that I agree with, that the properties of the invisible text do not affect the display of the ellipsis. The current implementation doesn't have that property (adding a face to a hidden character will cause the ellipsis to change to the default face), so I think it's a net gain. (In fact, I'm even a bit surprised that it handles the selection and font fallback cases properly. It's great that it does though; but did I apply the patch incorrectly?) Clément. > diff --git a/src/xdisp.c b/src/xdisp.c > index 87a92fc..7e5f7df 100644 > --- a/src/xdisp.c > +++ b/src/xdisp.c > @@ -4583,14 +4583,15 @@ setup_for_ellipsis (struct it *it, int len) > it->current.dpvec_index = 0; > it->dpvec_face_id = -1; > > - /* Reset the current face ID to default if the last visible > - character and the first invisible character have different faces. > - IT->saved_face_id was set in handle_stop to the face of the > - preceding character, and will be different from IT->face_id only > - if the invisible text skipped in handle_invisible_prop has some > - non-default face. IT's face is restored in set_iterator_to_next. */ > - if (it->saved_face_id < 0 || it->saved_face_id != it->face_id) > - it->saved_face_id = it->face_id = DEFAULT_FACE_ID; > + /* Use IT->saved_face_id for the ellipsis, so that it has the same > + face as the preceding text. IT->saved_face_id was set in > + handle_stop to the face of the preceding character, and will be > + different from IT->face_id only if the invisible text skipped in > + handle_invisible_prop has some non-default face. We thus ignore > + the face of the invisible text when we display the ellipsis. > + IT's face is restored in set_iterator_to_next. */ > + if (it->saved_face_id >= 0) > + it->face_id = it->saved_face_id; > > /* If the ellipsis represents buffer text, it means we advanced in > the buffer, so we should no longer ignore overlay strings. */ > > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property 2016-01-08 1:19 ` Clément Pit--Claudel @ 2016-01-08 10:22 ` Eli Zaretskii 0 siblings, 0 replies; 4+ messages in thread From: Eli Zaretskii @ 2016-01-08 10:22 UTC (permalink / raw) To: Clément Pit--Claudel; +Cc: 22323-done > Cc: 22323@debbugs.gnu.org > From: Clément Pit--Claudel <clement.pitclaudel@live.com> > Date: Thu, 7 Jan 2016 20:19:25 -0500 > > > Here's a suggestion: ignore the face of the invisible text altogether, > > and instead always use the face of the last visible character. The > > patch to do that is below; it fixes all of your test cases. But since > > you would like to see the face of the invisible text show through, I'm > > not sure you will like it... > > > > WDYT? > > I like it :) I find it much more consistent that what we currently have. Plus, > has a desirable property that you pointed out and that I agree with, that the > properties of the invisible text do not affect the display of the ellipsis. The > current implementation doesn't have that property (adding a face to a hidden > character will cause the ellipsis to change to the default face), so I think > it's a net gain. Thanks, I pushed it, and I'm marking these 2 bugs as done. > (In fact, I'm even a bit surprised that it handles the selection and font > fallback cases properly. It's great that it does though; but did I apply the > patch incorrectly?) Yes, you applied correctly. The default ellipsis uses ASCII characters, so I think the fallback font doesn't affect them. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-08 10:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-07 16:52 bug#22323: Font fallback causes inconsistent stacking of faces in overlays with invisible property Clément Pit--Claudel 2016-01-07 18:22 ` Eli Zaretskii 2016-01-08 1:19 ` Clément Pit--Claudel 2016-01-08 10:22 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.