bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Gustavo Barros]

Hi All,

`pulse-reset-face' in `pulse.el' "resets" the `:extend' attribute of 
`pulse-highlight-face' only when it is called with an argument. This is 
done by:

#+begin_src emacs-lisp
(and face
     (set-face-extend 'pulse-highlight-face
                      (face-extend-p face nil t)))
#+end_src

Considering the role `pulse-highlight-face' plays in the way `pulse' 
works, it would be expected that this resetting should also be done for 
the `:extend' attribute when the function is called without an argument, 
otherwise, that face attribute is left in an arbitrary state when one of 
the `pulse-momentary-highlight-...` user facing functions is called 
without an argument.  That state is "the extend attribute of whatever 
face was last passed as an argument to one of the pulsing functions".

An ECM to reproduce the effects of this is the following.  Start `emacs 
-Q', and set `pulse's default face `:extend' to t.

#+begin_src emacs-lisp
(load-library "pulse")
(set-face-extend 'pulse-highlight-start-face t)
#+end_src

Examination of `pulse-highlight-start-face' with `describe-face' shows 
the attribute in place. And, if we evaluate (make sure it is not the 
last line of the buffer when calling `C-x C-e'):

#+begin_src emacs-lisp
(pulse-momentary-highlight-one-line (point))

#+end_src

And the `:extend' does not work. Now eval:

#+begin_src emacs-lisp
(pulse-momentary-highlight-one-line (point) 'pulse-highlight-start-face)

#+end_src

And the pulse face is indeed extended this time. And then, if we eval 
again:

#+begin_src emacs-lisp
(pulse-momentary-highlight-one-line (point))

#+end_src

Now the extend works for this one.

If we want to "revert" the state, it is enough that we call, for 
example:

#+begin_src emacs-lisp
(pulse-momentary-highlight-one-line (point) 'highlight)

#+end_src

(`hightlight' is the face used for this purpose by 
`flymake-show-diagnostic', but any other face with no `extend' attribute 
called by any other user of `pulse.el' infrastructure would do the 
same).

And now,

#+begin_src emacs-lisp
(pulse-momentary-highlight-one-line (point))

#+end_src

no longer works as expected (again).

My suggested fix is that `pulse-reset-face' do the same for the `extend' 
attribute as it does for `background', namely: if it is called with a 
`face' argument, use the extend attribute of that face; otherwise, use 
the extend attribute of `pulse's default face 
`pulse-highlight-start-face'.

Best regards,
Gustavo.




In GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, 
cairo version 1.16.0)
 of 2021-03-25 built on gusbrs-laptop
Windowing system distributor 'The X.Org Foundation', version 
11.0.12009000
System Description: Linux Mint 20.1

Recent messages:
Loading pulse...done
t
nil
[nil 24768 52127 785690 0.03 pulse-tick 
((11422590749233444063540913557203 . 7036874417766400000000)) nil 
395000]
[nil 24768 52130 912738 0.03 pulse-tick 
((57112953856190749572945063578143 . 35184372088832000000000)) nil 
808000]
[nil 24768 52132 913706 0.03 pulse-tick 
((57112953926593103094281339013663 . 35184372088832000000000)) nil 
41000]
[nil 24768 52135 937847 0.03 pulse-tick 
((57112954032995665628571964672543 . 35184372088832000000000)) nil 
679000]
[nil 24768 52139 121924 0.03 pulse-tick 
((11422590829005067355650957168339 . 7036874417766400000000)) nil 
746000]
Making completion list...
command-execute: Command attempted to use minibuffer while in minibuffer
Quit
Configured using:
 'configure --with-mailutils --with-xwidgets --with-modules
 --with-cairo'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS XWIDGETS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LC_MONETARY: pt_BR.UTF-8
  value of $LC_NUMERIC: pt_BR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search seq
byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils pulse time-date subr-x
cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
xwidget-internal cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 45282 7952)
 (symbols 48 6067 2)
 (strings 32 15716 2078)
 (string-bytes 1 510505)
 (vectors 16 9328)
 (vector-slots 8 125104 10630)
 (floats 8 22 48)
 (intervals 56 279 7)
 (buffers 1000 12))

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Lars Ingebrigtsen]

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> Considering the role `pulse-highlight-face' plays in the way `pulse' 
> works, it would be expected that this resetting should also be done for 
> the `:extend' attribute when the function is called without an argument, 

I think that sounds correct, so I've now made this change in Emacs 28.

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

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Gustavo Barros]

Hi Lars,

On Sun, 13 Jun 2021 at 09:49, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Gustavo Barros <gusbrs.2016@gmail.com> writes:
>
>> Considering the role `pulse-highlight-face' plays in the way `pulse' 
>> works, it would be expected that this resetting should also be done 
>> for 
>> the `:extend' attribute when the function is called without an 
>> argument, 
>
> I think that sounds correct, so I've now made this change in Emacs 28.

Thank you for looking into this.

But my suggestion was more the one to use the `:extend' attribute of 
`pulse-highlight-start-face' to reset `pulse-highlight-face'. Like:

#+begin_src emacs-lisp
(set-face-extend 'pulse-highlight-face
                 (if face 
                     (face-extend-p face nil t)
                   (face-extend-p 'pulse-highlight-start-face)))
#+end_src

Because `pulse-highlight-start-face' is the default face `pulse' uses 
when the `pulse-momentary-highlight-...' functions are called without 
argument.  This makes `pulse-highlight-start-face' the only possible 
handle for setting the `:extend' attribute for calls of `pulse' 
functions without argument. If you just set it to nil when a call with 
no argument is made, we can still only get a pulse to work with 
`:extend' by explicitly passing the argument.

Btw, this is the same treatment `:background' receives. Why should we 
treat both face attributes differently?

Best regards,
Gustavo.

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Lars Ingebrigtsen]

Gustavo Barros <gusbrs.2016@gmail.com> writes:

> But my suggestion was more the one to use the `:extend' attribute of 
> `pulse-highlight-start-face' to reset `pulse-highlight-face'. Like:
>
> #+begin_src emacs-lisp
> (set-face-extend 'pulse-highlight-face
>                  (if face 
>                      (face-extend-p face nil t)
>                    (face-extend-p 'pulse-highlight-start-face)))
> #+end_src

Oh, right.  Now pushed something similar.

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

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Gustavo Barros]

Hi Lars,

On Sun, 13 Jun 2021 at 10:39, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Gustavo Barros <gusbrs.2016@gmail.com> writes:
>
>> But my suggestion was more the one to use the `:extend' attribute of 
>> `pulse-highlight-start-face' to reset `pulse-highlight-face'. Like:
>>
>> #+begin_src emacs-lisp
>> (set-face-extend 'pulse-highlight-face
>>                  (if face 
>>                      (face-extend-p face nil t)
>>                    (face-extend-p 'pulse-highlight-start-face)))
>> #+end_src
>
> Oh, right.  Now pushed something similar.

Thank you very much.

And, if you allow me a minor related "side report" (just noticed it 
now).  The `face-extend-p' function in the Elisp manual is missing the 
`inherit' argument at:

http://git.savannah.gnu.org/cgit/emacs.git/tree/doc/lispref/display.texi?id=00802c0cfeea4218802509693ca0f5d2593f8a7c#n3010

Best regards,
Gustavo.

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Eli Zaretskii]

> From: Gustavo Barros <gusbrs.2016@gmail.com>
> Date: Sun, 13 Jun 2021 10:45:57 -0300
> Cc: 48936@debbugs.gnu.org
> 
> And, if you allow me a minor related "side report" (just noticed it 
> now).  The `face-extend-p' function in the Elisp manual is missing the 
> `inherit' argument at:

Thanks, fixed.

bug#48936: 27.2; pulse-reset-face and extend attribute of pulse-highligh-face

[Gustavo Barros]

On Sun, 13 Jun 2021 at 11:12, Eli Zaretskii <eliz@gnu.org> wrote:

>> And, if you allow me a minor related "side report" (just noticed it 
>> now).  The `face-extend-p' function in the Elisp manual is missing 
>> the 
>> `inherit' argument at:
>
> Thanks, fixed.

Thank you, Eli.

Best,
Gustavo.