bug#34022: 26.1; Region highlight has lower priority than flymake overlays

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[Johann Klähn]

I just noticed a bug (?) that region becomes de-facto "invisible" due to its highlight overlay having a lower priority than the flymake overlays. This only happens when both region and flymake diagnostics are indicated using the same face property (background in my case). It becomes more apparent for larger flymake overlays.

Steps to reproduce, starting from emacs -Q:

• M-: (set-face-background flymake-note "yellow green")
• M-x find-library flymake (or any file where there will be flymake diagnostics)
• M-x flymake-mode
• Jump to a docstring/line/paragraph highlighted using e.g. the `flymake-note' face.
• With transient-mark-mode enabled select regions that overlap with the
  flymake overlay. ⇒ The region is not highlighted since the background
 face property given by the flymake overlay takes precedence.

Relevant extract from simple.el.gz, where the secondary priority for
the region highlight overlay is set to 100:

> (defvar redisplay-highlight-region-function
>   (lambda (start end window rol)
>     (if (not (overlayp rol))
>         (let ((nrol (make-overlay start end)))
>           (funcall redisplay-unhighlight-region-function rol)
>           (overlay-put nrol 'window window)
>           (overlay-put nrol 'face 'region)
>           ;; Normal priority so that a large region doesn't hide all the
>           ;; overlays within it, but high secondary priority so that if it
>           ;; ends/starts in the middle of a small overlay, that small overlay
>           ;; won't hide the region's boundaries.
>           (overlay-put nrol 'priority '(nil . 100))
>           nrol)
>       (unless (and (eq (overlay-buffer rol) (current-buffer))
>                    (eq (overlay-start rol) start)
>                    (eq (overlay-end rol) end))
>         (move-overlay rol start end (current-buffer)))
>       rol)))

Flymake uses 100 as the offset to compute the default priority for its overlays:

> (defun flymake--highlight-line (diagnostic)
>   "Highlight buffer with info in DIAGNOSTIC."
>   (when-let* ((ov (make-overlay
>                    (flymake--diag-beg diagnostic)
>                    (flymake--diag-end diagnostic))))
>     ;; First set `category' in the overlay, then copy over every other
>     ;; property.
>     ;;
>     (let ((alist (assoc-default (flymake--diag-type diagnostic)
>                                 flymake-diagnostic-types-alist)))
>       (overlay-put ov 'category (assoc-default 'flymake-category alist))
>       (cl-loop for (k . v) in alist
>                unless (eq k 'category)
>                do (overlay-put ov k v)))
>     ;; Now ensure some essential defaults are set
>     ;;
>     (cl-flet ((default-maybe
>                 (prop value)
>                 (unless (or (plist-member (overlay-properties ov) prop)
>                             (let ((cat (overlay-get ov
>                                                     'flymake-category)))
>                               (and cat
>                                    (plist-member (symbol-plist cat) prop))))
>                   (overlay-put ov prop value))))
>       (default-maybe 'bitmap 'flymake-error-bitmap)
>       (default-maybe 'face 'flymake-error)
>       (default-maybe 'before-string
>         (flymake--fringe-overlay-spec
>          (overlay-get ov 'bitmap)))
>       (default-maybe 'help-echo
>         (lambda (window _ov pos)
>           (with-selected-window window
>             (mapconcat
>              #'flymake--diag-text
>              (flymake-diagnostics pos)
>              "\n"))))
>       (default-maybe 'severity (warning-numeric-level :error))
>       (default-maybe 'priority (+ 100 (overlay-get ov 'severity))))
>     ;; Some properties can't be overridden.
>     ;;
>     (overlay-put ov 'evaporate t)
>     (overlay-put ov 'flymake-diagnostic diagnostic)))

Is it intentional that 100 is used as the offset here? This has the effect that most priorities will be above that of the region overlay.

---

In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.30)
 of 2018-07-05 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.12003000
Recent messages:

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 MODULES THREADS LIBSYSTEMD LCMS2

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LC_MESSAGES: C
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  flymake-mode: t
  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
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq dired
dired-loaddefs format-spec rfc822 mml mml-sec password-cache epa derived
epg epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-extra
help-fns radix-tree cl-print byte-opt bytecomp byte-compile cconv debug
face-remap misearch multi-isearch noutline outline easy-mmode lisp-mnt
checkdoc cl-macs gv cl-seq flymake-proc flymake compile comint
ansi-color ring warnings help-mode easymenu cl-loaddefs cl-lib jka-compr
thingatpt find-func elec-pair time-date mule-util 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 menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame 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 minibuffer 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 dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 123857 7080)
 (symbols 48 22067 1)
 (miscs 40 144 289)
 (strings 32 35534 1170)
 (string-bytes 1 922022)
 (vectors 16 17358)
 (vector-slots 8 524383 14202)
 (floats 8 67 298)
 (intervals 56 2057 393)
 (buffers 992 18))

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[João Távora]

Johann Klähn <johann@jklaehn.de> writes:

> I just noticed a bug (?) that region becomes de-facto "invisible" due
> to its highlight overlay having a lower priority than the flymake
> overlays. This only happens when both region and flymake diagnostics
> are indicated using the same face property (background in my case). It
> becomes more apparent for larger flymake overlays.
>
> Steps to reproduce, starting from emacs -Q:
>
> • M-: (set-face-background flymake-note "yellow green")

You probably mean 

(set-face-background 'flymake-note "yellow green")

> Flymake uses 100 as the offset to compute the default priority for its
> overlays:
> Is it intentional that 100 is used as the offset here? This has the
> effect that most priorities will be above that of the region overlay.

Yes, it was intentional.  Doesn't mean that it's correct :-) Perhaps it
should be reduced to the 50's region and also use the (nil . <value>),
form which I didn't know about (so thanks for showing me this).

I don't know if there are any guide-lines for interoperation in this
front, so if someone more knowledgeable than me want to chime in...

Anyway, for now, you can probably work around this with

   (push '(priority . (nil . 50)) (get :note 'flymake-overlay-control))

João

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[Johann Klähn]

(Sorry, I initially forgot to CC the bug tracker.)

Hi João,

thanks for your quick response! I did not see `flymake-overlay-control' because I was still using
the version of flymake shipped with Emacs 26.1. (I really like the options introduced with the new
version allows, so I will probably switch to using it.)

On Wed, Jan 9, 2019, at 21:52, João Távora wrote:
> Johann Klähn <johann@jklaehn.de> writes:
> > • M-: (set-face-background flymake-note "yellow green")
> 
> You probably mean 
> 
> (set-face-background 'flymake-note "yellow green")
Exactly :)

> Yes, it was intentional.  Doesn't mean that it's correct :-) Perhaps it
> should be reduced to the 50's region and also use the (nil . <value>),
> form which I didn't know about (so thanks for showing me this).

Wrt. to using non-numeric priority values maybe Eli Zaretskii could chime in, because there is the
following section ("Overlay Properties") in the Emacs Lisp manual, which he wrote (see Bug#20253):

‖ ‘priority’
‖      This property’s value determines the priority of the overlay.  If
‖      you want to specify a priority value, use either ‘nil’ (or zero),
‖      or a positive integer.  Any other value has undefined behavior.
‖      [...]
‖      Note that Emacs sometimes uses non-numeric priority values for some
‖      of its internal overlays, so do not try to do arithmetic on the
‖      priority of an overlay (unless it is one that you created).  In
‖      particular, the overlay used for showing the region uses a priority
‖      value of the form ‘(PRIMARY . SECONDARY)’, where the PRIMARY value
‖      is used as described above, and SECONDARY is the fallback value
‖      used when PRIMARY and the nesting considerations fail to resolve
‖      the precedence between overlays.  However, you are advised not to
‖      design Lisp programs based on this implementation detail; if you
‖      need to put overlays in priority order, use the SORTED argument of
‖      ‘overlays-at’.  *Note Finding Overlays::.

Even though it's shipped with Emacs, flymake is probably not "internal" enough to use those?

...looking around further I just found Bug#16192, where a similar problem was fixed for hl-line-mode
by using a negative priority.  And then there is also Bug#15899, which discusses the use of overlays
to highlight region in the first place.

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[João Távora]

Johann Klähn <johann@jklaehn.de> writes:

> thanks for your quick response! I did not see
> `flymake-overlay-control' because I was still using the version of
> flymake shipped with Emacs 26.1. (I really like the options introduced
> with the new version allows, so I will probably switch to using it.)

Flymake is now also distributed through ELPA, so you can use the most
recent version even on Emacs 26.1

> ‖ ‘priority’
> ‖      This property’s value determines the priority of the overlay.  If
> ‖      you want to specify a priority value, use either ‘nil’ (or zero),
> ‖      or a positive integer.  Any other value has undefined behavior.
> ‖      [...]
> ‖      Note that Emacs sometimes uses non-numeric priority values for some
> ‖      of its internal overlays, so do not try to do arithmetic on the
> ‖      priority of an overlay (unless it is one that you created).  In
> ‖      particular, the overlay used for showing the region uses a priority
> ‖      value of the form ‘(PRIMARY . SECONDARY)’, where the PRIMARY value
> ‖      is used as described above, and SECONDARY is the fallback value
> ‖      used when PRIMARY and the nesting considerations fail to resolve
> ‖      the precedence between overlays.  However, you are advised not to
> ‖      design Lisp programs based on this implementation detail; if you
> ‖      need to put overlays in priority order, use the SORTED argument of
> ‖      ‘overlays-at’.  *Note Finding Overlays::.
>
> Even though it's shipped with Emacs, flymake is probably not
> "internal" enough to use those?

Good point.  Don't know how to fix it then.

> ...looking around further I just found Bug#16192, where a similar
> problem was fixed for hl-line-mode by using a negative priority.  And
> then there is also Bug#15899, which discusses the use of overlays to
> highlight region in the first place.
>
I remember reading somewhere negative priorites were a nono, maybe that
has changed.

João

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[João Távora]

[BTW Johann: to CC the bug tracker, use 34022@debbugs.gnu.org, not
bug-gnu-emacs@gnu.org, since this will create a new bug, and has the
potential to annoy maintainers :-).  I'm guilty of that too, since I
replied to all in the email you did that (and so created another bug).]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure what I'm supposed to chime in about.  Are you asking
> whether it's okay for Flymake to use the same technique as the
> region's overlay?

I guess he is, yes.  It's a problem I didn't encounter during
development because the face properties of the Flymake overlays and the
region don't clash when using the default settings.

> Or are you asking something else?  If the former,
> I'm not sure I understand the problem well enough to answer the
> question.

In Flymake multiple overlays (error, warning, note) can overlap and
priorities are used so that an error is never hidden by a note.  I based
thosed priorities in the 100's range (so note = 101, warning=102,
error=103, sth like that). 100 was a number chosen quite arbitrarily
which worked fine until Johann noticed the bug (I do think it's a bug).

>In particular, how is Flymake different from any other package which
>uses overlays to mark portions of text?

Perhaps it isn't.  Can you point me to other packages that leave
persistent region-resisting overlays in the buffer? (or that "play nice"
with the region, for some meaning of "nice").

João

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: Johann Klähn <johann@jklaehn.de>, 34022@debbugs.gnu.org
> Date: Fri, 11 Jan 2019 09:33:33 +0000
> 
> In Flymake multiple overlays (error, warning, note) can overlap and
> priorities are used so that an error is never hidden by a note.  I based
> thosed priorities in the 100's range (so note = 101, warning=102,
> error=103, sth like that). 100 was a number chosen quite arbitrarily
> which worked fine until Johann noticed the bug (I do think it's a bug).

Would it still DTRT if you use priorities around 50?

> >In particular, how is Flymake different from any other package which
> >uses overlays to mark portions of text?
> 
> Perhaps it isn't.  Can you point me to other packages that leave
> persistent region-resisting overlays in the buffer?

Wht do you mean by "region-resisting"?

> (or that "play nice" with the region, for some meaning of "nice").

What is the meaning of "playing nice" in this context?

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Cc: Johann Klähn <johann@jklaehn.de>, 34022@debbugs.gnu.org
>> Date: Fri, 11 Jan 2019 09:33:33 +0000
>> 
>> In Flymake multiple overlays (error, warning, note) can overlap and
>> priorities are used so that an error is never hidden by a note.  I based
>> thosed priorities in the 100's range (so note = 101, warning=102,
>> error=103, sth like that). 100 was a number chosen quite arbitrarily
>> which worked fine until Johann noticed the bug (I do think it's a bug).
>
> Would it still DTRT if you use priorities around 50?

No.  You can see the problem for yourself (Johann had already pasted a
slightly harder to follow recipe):

  Emacs -Q
  M-x flymake-mode RET
  M-: (set-face-background 'flymake-note "yellow green") RET
  C-x C-f lisp/progmodes/flymake.el RET
  M-g M-g 1057 RET

Case A: Start marking a region from the beginning of the line.
Case B: Start marking a region from the middle of the line

With stock flymake.el, in both cases the region is invisible in the
parts where it overlaps the green diagnostic.  Actually you could sort
of say that in case A it's doing the right thing.  But in case B it
clearly isn't.

Also if I make Flymake use priorities around 50 as you suggest (actually
41 in this case).

But if I make Flymake use priorities of the form (nil . 41), it is
possible to see both the region and the diagnostic in both A and B.  I
think it's the way this should behave.

>> Perhaps it isn't.  Can you point me to other packages that leave
>> persistent region-resisting overlays in the buffer?
> Wht do you mean by "region-resisting"?
> What is the meaning of "playing nice" in this context?

I hope I've clarified above!

João

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: johann@jklaehn.de,  34022@debbugs.gnu.org
> Date: Fri, 11 Jan 2019 20:04:05 +0000
> 
> But if I make Flymake use priorities of the form (nil . 41), it is
> possible to see both the region and the diagnostic in both A and B.  I
> think it's the way this should behave.

Then feel free to go ahead and use that.

bug#34022: 26.1; Region highlight has lower priority than flymake overlays

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Cc: johann@jklaehn.de,  34022@debbugs.gnu.org
>> Date: Fri, 11 Jan 2019 20:04:05 +0000
>> 
>> But if I make Flymake use priorities of the form (nil . 41), it is
>> possible to see both the region and the diagnostic in both A and B.  I
>> think it's the way this should behave.
>
> Then feel free to go ahead and use that.

Done in 455e753a2b8fa2e83d23be1771f3371ba90b85f9 (after a very long
time).

Closing this.

João