bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Sebastian Urban]

This is follow up (part two of solution) to this bug#35044:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-03/msg01011.html
and then (two months split)
https://lists.gnu.org/archive/html/bug-gnu-emacs/2019-04/msg00739.html
or at debbugs
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=35044

In short "table.el" uses text property 'face' (table-cell) which causes
losing of cell highlight when user switches from modes like "Text" or
"Fundamental" to modes that use "Font lock", like "Outline".  Also when
user switches back, 'face' is not applied immediately in every cell, but
one by one, when cursor is moved from cell to cell.

For both problems a solution seems to be an update to "table.el", which
will change 'face' to 'font-lock-face'.

If it helps, I found 4 places in code that probably need to be changed:
- 'defface table-cell' at line 680,
- 'defun table--put-cell-face-property' at line 5172,
- 'defun table--remove-cell-properties' at line 5189,
- 'defun table--update-cell-face' at line 5209.

---

In GNU Emacs 25.2.1 (i686-w64-mingw32)
  of 2017-04-24 built on LAPHROAIG
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
  'configure --host=i686-w64-mingw32 --without-dbus
  --without-compress-install 'CFLAGS=-static -O2 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
   value of $LANG: PLK
   locale-coding-system: cp1250

Major mode: Fundamental

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-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

Recent messages:
Preparing diary...
No diary entries for Niedziela, 28 Kwiecień 2019
Preparing diary...done
For information about GNU Emacs and the GNU system, type C-h C-a.
next-line: End of buffer

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg gnus-util mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils cal-china
lunar solar cal-dst cal-bahai cal-islam cal-hebrew holidays hol-loaddefs
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs finder-inf
package epg-config seq byte-opt gv bytecomp byte-compile cl-extra
help-mode easymenu cconv cl-loaddefs pcase cl-lib time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel dos-w32 ls-lisp disp-table w32-win w32-vars term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
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 charscript
case-table epa-hook jka-cmpr-hook help simple abbrev 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 w32notify w32 multi-tty
make-network-process emacs)

Memory information:
((conses 8 114624 7479)
  (symbols 32 22205 0)
  (miscs 32 94 203)
  (strings 16 23812 4853)
  (string-bytes 1 741124)
  (vectors 8 15933)
  (vector-slots 4 485829 3692)
  (floats 8 712 215)
  (intervals 28 263 31)
  (buffers 520 21))

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Noam Postavsky]

severity 35481 minor
tags 35481 + easy confirmed
quit

Sebastian Urban <mrsebastianurban@gmail.com> writes:

> For both problems a solution seems to be an update to "table.el", which
> will change 'face' to 'font-lock-face'.
>
> If it helps, I found 4 places in code that probably need to be changed:
> - 'defface table-cell' at line 680,

The defface can be left alone.  It's only the use of the 'face' text
property that can be a problem.

> - 'defun table--put-cell-face-property' at line 5172,
> - 'defun table--remove-cell-properties' at line 5189,
> - 'defun table--update-cell-face' at line 5209.

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Sebastian Urban]

This is just a reminder, because "This bug report was last modified 13
days ago."

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Sebastian Urban]

This is just a reminder, because "This bug report was last modified 80
days ago."

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Lars Ingebrigtsen]

Sebastian Urban <mrsebastianurban@gmail.com> writes:

> In short "table.el" uses text property 'face' (table-cell) which causes
> losing of cell highlight when user switches from modes like "Text" or
> "Fundamental" to modes that use "Font lock", like "Outline".  Also when
> user switches back, 'face' is not applied immediately in every cell, but
> one by one, when cursor is moved from cell to cell.
>
> For both problems a solution seems to be an update to "table.el", which
> will change 'face' to 'font-lock-face'.
>
> If it helps, I found 4 places in code that probably need to be changed:
> - 'defface table-cell' at line 680,
> - 'defun table--put-cell-face-property' at line 5172,
> - 'defun table--remove-cell-properties' at line 5189,
> - 'defun table--update-cell-face' at line 5209.

The patch below does this, but I'm not sure it's right.  When we're
using font-lock-face instead of face, then things work as normal when
we're using global-font-lock-mode, but if that's off, then we won't get
any faces at all.

Which would be bad.

I grepped around, but other modes don't seem to care about that.  But I
guess it's somewhat unusual for a minor mode to do stuff like this?

I think putting both font-lock-face and face on the text would do the
trick here, but is that the way to go?

diff --git a/lisp/textmodes/table.el b/lisp/textmodes/table.el
index 839df035d2..92aaa46ad6 100644
--- a/lisp/textmodes/table.el
+++ b/lisp/textmodes/table.el
@@ -5151,7 +5151,7 @@ table--put-cell-indicator-property
 
 (defun table--put-cell-face-property (beg end &optional object)
   "Put cell face property."
-  (put-text-property beg end 'face 'table-cell object))
+  (put-text-property beg end 'font-lock-face 'table-cell object))
 
 (defun table--put-cell-keymap-property (beg end &optional object)
   "Put cell keymap property."
@@ -5178,7 +5178,7 @@ table--remove-cell-properties
 				   'table-cell nil
 				   'table-justify nil
 				   'table-valign nil
-				   'face nil
+				   'font-lock-face nil
 				   'rear-nonsticky nil
 				   'cursor-sensor-functions nil
 				   'keymap nil)

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

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I grepped around, but other modes don't seem to care about that.  But I
> guess it's somewhat unusual for a minor mode to do stuff like this?
>
> I think putting both font-lock-face and face on the text would do the
> trick here, but is that the way to go?

There weren't any comments in two weeks, and this change does fix the
problem (if inelegantly), so I've applied it.

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

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Noam Postavsky]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The patch below does this, but I'm not sure it's right.  When we're
> using font-lock-face instead of face, then things work as normal when
> we're using global-font-lock-mode, but if that's off, then we won't get
> any faces at all.
>
> Which would be bad.

I missed this the first time around, but, why would that be bad?  If the
user turns off font-lock-mode then not getting faces should be expected,
no?

bug#35481: 25.2; table.el should use 'font-lock-face' instead of 'face'

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> The patch below does this, but I'm not sure it's right.  When we're
>> using font-lock-face instead of face, then things work as normal when
>> we're using global-font-lock-mode, but if that's off, then we won't get
>> any faces at all.
>>
>> Which would be bad.
>
> I missed this the first time around, but, why would that be bad?  If the
> user turns off font-lock-mode then not getting faces should be expected,
> no?

Font locking is about adding decorative highlights.  The background face
table.el adds is functional -- it says what cell is selected.

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