GNU bug report logs - #62675
29.0.60; emoji-zoom-increase interacts badly with faces

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62675 in the body.
You can then email your comments to 62675 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #3 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 09:38:32 +0200

--begin--
emacs -Q
C-p C-p
C-x 8 e + + + +
C-u C-x =
--end--

Visually, this does the right thing: the ';' has its height increased,
but the text properties look like this:

  face                 ((:height 1.1) font-lock-comment-delimiter-face :height 1.3310000000000004)
  fontified            t
  rear-nonsticky       t

Thatʼs not a valid 'face property, unless Iʼm misreading (info
"(elisp) Special Properties"), plus the ':height 1.331' bit is wrong,
thatʼs (expt 1.1 3), not (expt 1.1 4). Yet it works, the height has
been increased.

Is this a bug in our interpretation of 'face properties?

I can fix the code in `emoji-zoom-increase' to produce something like

((:height 1.4641000000000004) font-lock-comment-delimiter-face)

instead, but first Iʼd like to understand whatʼs going on.

In GNU Emacs 29.0.60 (build 15, x86_64-pc-linux-gnu, GTK+ Version
 3.24.24, cairo version 1.16.0) of 2023-03-14 built on rltb
Repository revision: 4b6f2a7028b91128934a19f83572f24106782225
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Debian GNU/Linux 11 (bullseye)

Robert
-- 

Message #6 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 11:45:35 +0300

> From: Robert Pluim <rpluim <at> gmail.com>
> Date: Wed, 05 Apr 2023 09:38:32 +0200
> 
> 
> --begin--
> emacs -Q
> C-p C-p
> C-x 8 e + + + +
> C-u C-x =
> --end--
> 
> Visually, this does the right thing: the ';' has its height increased,
> but the text properties look like this:
> 
>   face                 ((:height 1.1) font-lock-comment-delimiter-face :height 1.3310000000000004)
>   fontified            t
>   rear-nonsticky       t
> 
> Thatʼs not a valid 'face property, unless Iʼm misreading (info
> "(elisp) Special Properties"), plus the ':height 1.331' bit is wrong,
> thatʼs (expt 1.1 3), not (expt 1.1 4). Yet it works, the height has
> been increased.

The first "+" doesn't increase the height, though.

> Is this a bug in our interpretation of 'face properties?

Face properties can be specified in many strange ways, and the fact
that some invalid specifications seem to "work" doesn't mean we have a
bug, it just means some invalid faces fall through the cracks.

> I can fix the code in `emoji-zoom-increase' to produce something like
> 
> ((:height 1.4641000000000004) font-lock-comment-delimiter-face)
> 
> instead

Please do, and thanks.  We don't need to understand why some bug seems
to work, it is enough to fix the bug.

Message #9 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 11:12:31 +0200

>>>>> On Wed, 05 Apr 2023 11:45:35 +0300, Eli Zaretskii <eliz <at> gnu.org> said:

    >> Thatʼs not a valid 'face property, unless Iʼm misreading (info
    >> "(elisp) Special Properties"), plus the ':height 1.331' bit is wrong,
    >> thatʼs (expt 1.1 3), not (expt 1.1 4). Yet it works, the height has
    >> been increased.

    Eli> The first "+" doesn't increase the height, though.

Hmm, youʼre right. Although if I then move to a different place in the
buffer, and do 'C-x 8 e +' it does increase the height first time. Is
this an interaction with font-lock?

Oh, and it doesnʼt work at eob. And increase followed by decrease
doesnʼt get you back to :height 1.0. And thereʼs no reset. I think
Iʼll only fix the first one on emacs-29 :-)

    >> Is this a bug in our interpretation of 'face properties?

    Eli> Face properties can be specified in many strange ways, and the fact
    Eli> that some invalid specifications seem to "work" doesn't mean we have a
    Eli> bug, it just means some invalid faces fall through the cracks.

OK. Iʼll make the code match the documented behaviour, and not worry
about it too much 😀

    >> I can fix the code in `emoji-zoom-increase' to produce something like
    >> 
    >> ((:height 1.4641000000000004) font-lock-comment-delimiter-face)
    >> 
    >> instead

    Eli> Please do, and thanks.  We don't need to understand why some bug seems
    Eli> to work, it is enough to fix the bug.

This seems to work:

diff --git a/lisp/international/emoji.el b/lisp/international/emoji.el
index bcd4aac4f29..a9ed6e79cc9 100644
--- a/lisp/international/emoji.el
+++ b/lisp/international/emoji.el
@@ -708,22 +708,24 @@ emoji-zoom-increase
 FACTOR is the multiplication factor for the size."
   (interactive)
   (set-transient-map emoji-zoom-map t nil "Zoom with %k")
-  (let* ((factor (or factor 1.1))
-         (old (get-text-property (point) 'face))
-         (height (or (and (consp old)
-                          (plist-get old :height))
-                     1.0))
-         (inhibit-read-only t))
-    (with-silent-modifications
-      (if (consp old)
-          (add-text-properties
-           (point) (1+ (point))
-           (list 'face (plist-put (copy-sequence old) :height (* height factor))
-                 'rear-nonsticky t))
-        (add-face-text-property (point) (1+ (point))
-                                (list :height (* height factor)))
-        (put-text-property (point) (1+ (point))
-                           'rear-nonsticky t)))))
+  (unless (eobp)
+    (let* ((factor (or factor 1.1))
+           (old (get-text-property (point) 'face))
+           (height (or (and (consp old)
+                            (plist-get (car old) :height))
+                       1.0))
+           (inhibit-read-only t))
+      (with-silent-modifications
+        (if (consp old)
+            (add-text-properties
+             (point) (1+ (point))
+             (list 'face (cons (plist-put (car old) :height (* height factor))
+                               (cdr old))
+                   'rear-nonsticky t))
+          (add-face-text-property (point) (1+ (point))
+                                  (list :height (* height factor)))
+          (put-text-property (point) (1+ (point))
+                             'rear-nonsticky t))))))
 
 ;;;###autoload
 (defun emoji-zoom-decrease ()

Robert
-- 

Message #12 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 12:54:27 +0300

> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: 62675 <at> debbugs.gnu.org
> Date: Wed, 05 Apr 2023 11:12:31 +0200
> 
>     Eli> Please do, and thanks.  We don't need to understand why some bug seems
>     Eli> to work, it is enough to fix the bug.
> 
> This seems to work:

Looks reasonable, thanks.  Please try both with font-lock-mode ON and
OFF, and in the latter case both with some non-default face at point
and without.

Message #15 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 13:55:22 +0200

>>>>> On Wed, 05 Apr 2023 12:54:27 +0300, Eli Zaretskii <eliz <at> gnu.org> said:

    >> From: Robert Pluim <rpluim <at> gmail.com>
    >> Cc: 62675 <at> debbugs.gnu.org
    >> Date: Wed, 05 Apr 2023 11:12:31 +0200
    >> 
    Eli> Please do, and thanks.  We don't need to understand why some bug seems
    Eli> to work, it is enough to fix the bug.
    >> 
    >> This seems to work:

    Eli> Looks reasonable, thanks.  Please try both with font-lock-mode ON and
    Eli> OFF, and in the latter case both with some non-default face at point
    Eli> and without.

Ah yes, font-lock off makes it all more complex. Tested in those 3
cases. Is this too much for emacs-29?

diff --git a/lisp/international/emoji.el b/lisp/international/emoji.el
index bcd4aac4f29..ff55170875e 100644
--- a/lisp/international/emoji.el
+++ b/lisp/international/emoji.el
@@ -708,22 +708,32 @@ emoji-zoom-increase
 FACTOR is the multiplication factor for the size."
   (interactive)
   (set-transient-map emoji-zoom-map t nil "Zoom with %k")
-  (let* ((factor (or factor 1.1))
-         (old (get-text-property (point) 'face))
-         (height (or (and (consp old)
-                          (plist-get old :height))
-                     1.0))
-         (inhibit-read-only t))
-    (with-silent-modifications
-      (if (consp old)
-          (add-text-properties
-           (point) (1+ (point))
-           (list 'face (plist-put (copy-sequence old) :height (* height factor))
-                 'rear-nonsticky t))
-        (add-face-text-property (point) (1+ (point))
-                                (list :height (* height factor)))
-        (put-text-property (point) (1+ (point))
-                           'rear-nonsticky t)))))
+  (unless (eobp)
+    (let* ((factor (or factor 1.1))
+           (old (get-text-property (point) 'face))
+           ;; The text property is either a named face, or a plist
+           ;; with :height, or a list starting with such a plist,
+           ;; followed by one or more faces.
+           (newheight (* (or (and (consp old)
+                                  (or (plist-get (car old) :height)
+                                      (plist-get old :height)))
+                             1.0)
+                         factor))
+           (inhibit-read-only t))
+      (with-silent-modifications
+        (if (consp old)
+            (add-text-properties
+             (point) (1+ (point))
+             (list 'face
+                   (if (eq (car old) :height)
+                       (plist-put old :height newheight)
+                     (cons (plist-put (car old) :height newheight)
+                           (cdr old)))
+                   'rear-nonsticky t))
+          (add-face-text-property (point) (1+ (point))
+                                  (list :height newheight))
+          (put-text-property (point) (1+ (point))
+                             'rear-nonsticky t))))))
 
 ;;;###autoload
 (defun emoji-zoom-decrease ()

Robert
-- 

Message #18 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 15:08:49 +0300

> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: 62675 <at> debbugs.gnu.org
> Date: Wed, 05 Apr 2023 13:55:22 +0200
> 
>     Eli> Looks reasonable, thanks.  Please try both with font-lock-mode ON and
>     Eli> OFF, and in the latter case both with some non-default face at point
>     Eli> and without.
> 
> Ah yes, font-lock off makes it all more complex. Tested in those 3
> cases. Is this too much for emacs-29?

Not too much, because this code is new in Emacs 29.

Thanks.

Message #21 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Wed, 05 Apr 2023 15:53:00 +0200

>>>>> On Wed, 05 Apr 2023 15:08:49 +0300, Eli Zaretskii <eliz <at> gnu.org> said:

    >> From: Robert Pluim <rpluim <at> gmail.com>
    >> Cc: 62675 <at> debbugs.gnu.org
    >> Date: Wed, 05 Apr 2023 13:55:22 +0200
    >> 
    Eli> Looks reasonable, thanks.  Please try both with font-lock-mode ON and
    Eli> OFF, and in the latter case both with some non-default face at point
    Eli> and without.
    >> 
    >> Ah yes, font-lock off makes it all more complex. Tested in those 3
    >> cases. Is this too much for emacs-29?

    Eli> Not too much, because this code is new in Emacs 29.

OK. I guess that means I can add `emoji-zoom-reset' and fix the
increase/decrease assymetry there as well.

Robert
-- 

Message #24 received at 62675 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 62675 <at> debbugs.gnu.org
Subject: Re: bug#62675: 29.0.60; emoji-zoom-increase interacts badly with faces
Date: Thu, 06 Apr 2023 13:38:58 +0200

tags 62675 fixed
close 62675 29.1
quit

>>>>> On Wed, 05 Apr 2023 15:53:00 +0200, Robert Pluim <rpluim <at> gmail.com> said:

>>>>> On Wed, 05 Apr 2023 15:08:49 +0300, Eli Zaretskii <eliz <at> gnu.org> said:
    >>> From: Robert Pluim <rpluim <at> gmail.com>
    >>> Cc: 62675 <at> debbugs.gnu.org
    >>> Date: Wed, 05 Apr 2023 13:55:22 +0200
    >>> 
    Eli> Looks reasonable, thanks.  Please try both with font-lock-mode ON and
    Eli> OFF, and in the latter case both with some non-default face at point
    Eli> and without.
    >>> 
    >>> Ah yes, font-lock off makes it all more complex. Tested in those 3
    >>> cases. Is this too much for emacs-29?

    Eli> Not too much, because this code is new in Emacs 29.

    Robert> OK. I guess that means I can add `emoji-zoom-reset' and fix the
    Robert> increase/decrease assymetry there as well.

Iʼve added `emoji-zoom-reset' as well as fixing this bug.

Closing.
Committed as 470d269ec1f

Robert
-- 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.