bug#32344: 26.1; Option to avoid comint highlighting input

bug#32344: 26.1; Option to avoid comint highlighting input

[Carlos Pita]

Some modes (at least python.el) do their own input coloring according to
the major mode font locking rules. It's unfortunate that comint then
overwrites this coloring with comint-highlight-input face. Now, avoding
this is as simple as removing

    (add-text-properties beg end
                         '(front-sticky t
                           font-lock-face comint-highlight-input))

from comint-send-input. The problem is that these 3 lines are buried
down into a long function and it's not possible to cleanly disable them,
except maybe by an around advice that redefines add-text-properties.

There is an even more unfortunate consequence of this: for multiline
inputs all but the last line will be fontified by
comint-highlight-input. I had reported that in
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=32315 which now I'm going
to close since it's obviously a particular manifestation of comint
forcing the recoloring of previous inputs.

Please add a customization variable to make this behavior optional so
that modes that colorize input can keep their colors in previous inputs
and in previous lines of the current input.


---


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.12000000
Recent messages:
Note: file is write protected
Mark saved where search started [2 times]
Mark set
kill-region: Buffer is read-only: #<buffer comint.el.gz>
Mark set [2 times]
Saved text until "'comint-output-filter-functions "")))))
"
Mark set [2 times]
comint-send-input
Mark activated

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 $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  display-line-numbers-mode: t
  pdf-occur-global-minor-mode: t
  flymake-mode: t
  diff-auto-refine-mode: t
  pyvenv-mode: t
  shell-dirtrack-mode: t
  xclip-mode: t
  winner-mode: t
  show-paren-mode: t
  ido-ubiquitous-mode: t
  ido-everywhere: t
  global-company-mode: t
  company-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny rfc822 mml mml-sec epa
derived epg mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail crm edebug completion
semantic/wisent/python semantic/db-mode semantic/decorate/include
semantic/db-find semantic/db-ref semantic/db eieio-base
semantic/decorate/mode semantic/decorate semantic/format semantic/tag-ls
semantic/ctxt semantic/dep semantic/find semantic/wisent/python-wy rx
rect eieio-opt speedbar sb-image ezimage dframe autoload lisp-mnt
dabbrev misearch multi-isearch cl-print debug tabify pulse vc-git
whitespace server display-line-numbers checkdoc pdf-occur ibuf-ext
ibuffer ibuffer-loaddefs tablist tablist-filter semantic/wisent/comp
semantic/wisent semantic/wisent/wisent semantic/util-modes semantic/util
semantic semantic/tag semantic/lex semantic/fw mode-local cedet dired
dired-loaddefs pdf-isearch let-alist pdf-misc imenu pdf-tools pdf-view
bookmark pp jka-compr pdf-cache pdf-info tq pdf-util image-mode
org-protocol org-element avl-tree generator org org-macro org-footnote
org-pcomplete org-list org-faces org-entities noutline outline
org-version ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table
ob-keys ob-exp ob-comint ob-core ob-eval org-compat org-macs
org-loaddefs find-func cal-menu calendar cal-loaddefs cl-extra yasnippet
elec-pair highlight-indentation flymake-proc flymake warnings help-fns
radix-tree help-mode elpy find-file-in-project ivy delsel colir color
ivy-overlay ffap thingatpt windmove diff-mode easy-mmode elpy-shell
pyvenv esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg
esh-groups eshell esh-module esh-mode esh-util elpy-profile elpy-django
elpy-refactor subr-x python tramp-sh tramp tramp-compat tramp-loaddefs
trampver ucs-normalize shell pcomplete parse-time format-spec advice
json map grep compile comint ansi-color files-x doom-themes-org
doom-tomorrow-night-theme doom-themes doom-themes-common company-oddmuse
company-keywords company-etags etags xref project company-gtags
company-dabbrev-code company-dabbrev company-files company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-bbdb xclip winner ring paren
ido-completing-read+ memoize s cus-edit minibuf-eldef ido gnus nnheader
gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums time-date
mail-utils mm-util mail-prsvr wid-edit company edmacro kmacro pcase
cus-start cus-load finder-inf info package easymenu epg-config
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib 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 1243861 76924)
 (symbols 48 47660 1)
 (miscs 40 15325 1716)
 (strings 32 140100 3027)
 (string-bytes 1 4040347)
 (vectors 16 82572)
 (vector-slots 8 2118926 159486)
 (floats 8 586 699)
 (intervals 56 11147 366)
 (buffers 992 53))

bug#32344:

[Carlos Pita]

Here is a workaround:

(advice-add 'comint-send-input
            :around (lambda (f &rest args)
                      (when (eq major-mode 'inferior-python-mode)
                        (cl-letf (((symbol-function 'g)
                                   (symbol-function 'add-text-properties))
                                  ((symbol-function 'add-text-properties)
                                   (lambda (start end properties
&optional object)
                                     (unless (eq (nth 3 properties)
'comint-highlight-input)
                                       (g start end properties object)))))
                          (apply f args)))))

bug#32344:

[Carlos Pita]

Recently I've had some exchange with elpy maintainer and he said he
would be glad to support full history fontification for python mode
but he didn't want to add my advice (its final version is quoted
below) to elpy in order to instrument comint-send-input, so for now we
just have a wiki-level DIY solution. Now, I fully understand his
concern, so is there any interest in adding this option to comint? It
seems quite simple a change and it will allow inferior modes to fully
colorize their history, which is a rather nice feature. In case there
is any interest I could submit a patch.

---

(advice-add 'comint-send-input
            :around (lambda (f &rest args)
                      (if (eq major-mode 'inferior-python-mode)
                          (cl-letf ((g (symbol-function 'add-text-properties))
                                    ((symbol-function 'add-text-properties)
                                     (lambda (start end properties
&optional object)
                                       (unless (eq (nth 3 properties)
'comint-highlight-input)
                                         (funcall g start end
properties object)))))
                            (apply f args))
                        (apply f args))))

bug#32344: (no subject)

[Jörg Behrmann]

I have just tested your advice and, for me, it works wonderfully; a great
quality of life improvement. I do hope you prepare a patch for upstream.

If you have a final patch I will happily test it for you, but so far I haven't
found any issues (in my usage).

Could a general version of the advice check for a customisable group of inferior
modes to do the proper fontification?

bug#32344:

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

Hi Jorg,

I somehow totally missed your answer. Of course I'm glad to provide a
proper patch (and get rid of those awful advices in my init.el ;) ).
Indeed I was searching through my old reports at debbugs mainly in
order to push this one ahead, so it's nice to see you interested
(although a bit late from my side...).

I'm attaching a patch adding a new customization option to suppress
highlighting. As soon as you apply it I will be submitting the
corresponding PR to elpy in order to have full python highlighting all
along the line. The patch is against emacs 27, please let me know if
you have any problem applying it on top 26.

Best regards
--
Carlos

[-- Attachment #2: 0001-Option-for-comint-to-honour-original-input-highlight.patch --]
[-- Type: text/x-patch, Size: 1782 bytes --]

From 1f2392f09b9c0193e79382512a8a30388ce54bf8 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Sun, 13 Oct 2019 13:57:11 -0300
Subject: [PATCH] Option for comint to honour original input highlighting

* comint.el (comint-send-input): add option to let major modes that
colorize their input (for example, python-mode) stop comint from
overriding that specialized colorization.
---
 lisp/comint.el | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 4bb4367..b8f5205 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -223,6 +223,12 @@ comint-input-autoexpand
 		 (other :tag "on" t))
   :group 'comint)
 
+(defcustom comint-highlight-input t
+  "If non-nil, highlight input with `comint-highlight-input' face.
+Otherwise keep the original highlighting untouched."
+  :type 'boolean
+  :group 'comint)
+
 (defface comint-highlight-input '((t (:weight bold)))
   "Face to use to highlight user input."
   :group 'comint)
@@ -1881,9 +1887,10 @@ comint-send-input
               (end (if no-newline (point) (1- (point)))))
           (with-silent-modifications
             (when (> end beg)
-              (add-text-properties beg end
-                                   '(front-sticky t
-                                     font-lock-face comint-highlight-input))
+              (when comint-highlight-input
+               (add-text-properties beg end
+                                    '(front-sticky t
+                                      font-lock-face comint-highlight-input)))
               (unless comint-use-prompt-regexp
                 ;; Give old user input a field property of `input', to
                 ;; distinguish it from both process output and unsent
-- 
2.20.1

bug#32344:

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]

I can confirm that the patch cleanly applies on top of emacs-26 branch
too, but in any case here is a version rebased on top of that branch.

[-- Attachment #2: 0001-Option-for-comint-to-honour-original-input-highlight.patch --]
[-- Type: text/x-patch, Size: 1782 bytes --]

From 1f2392f09b9c0193e79382512a8a30388ce54bf8 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Sun, 13 Oct 2019 13:57:11 -0300
Subject: [PATCH] Option for comint to honour original input highlighting

* comint.el (comint-send-input): add option to let major modes that
colorize their input (for example, python-mode) stop comint from
overriding that specialized colorization.
---
 lisp/comint.el | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 4bb4367..b8f5205 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -223,6 +223,12 @@ comint-input-autoexpand
 		 (other :tag "on" t))
   :group 'comint)
 
+(defcustom comint-highlight-input t
+  "If non-nil, highlight input with `comint-highlight-input' face.
+Otherwise keep the original highlighting untouched."
+  :type 'boolean
+  :group 'comint)
+
 (defface comint-highlight-input '((t (:weight bold)))
   "Face to use to highlight user input."
   :group 'comint)
@@ -1881,9 +1887,10 @@ comint-send-input
               (end (if no-newline (point) (1- (point)))))
           (with-silent-modifications
             (when (> end beg)
-              (add-text-properties beg end
-                                   '(front-sticky t
-                                     font-lock-face comint-highlight-input))
+              (when comint-highlight-input
+               (add-text-properties beg end
+                                    '(front-sticky t
+                                      font-lock-face comint-highlight-input)))
               (unless comint-use-prompt-regexp
                 ;; Give old user input a field property of `input', to
                 ;; distinguish it from both process output and unsent
-- 
2.20.1

bug#32344:

[Carlos Pita]

Hi Paul, I'm CC'ing you because:

1. This issue has been long open
2. I'm providing a very simple patch now
3. You seem to actively contribute to comint.el
4. IMO, despite its simplicity, this patch enables major modes to
provide a quite better interactive experience.
5. The current behavior is too invasive from the part of comint.

For further discussion of the possibilities open by this change,
please read https://github.com/jorgenschaefer/elpy/issues/1428

I'm also providing a couple of screenshots for you to compare.

If you prefer so, I could make the new option to be automatically buffer local.

Best regards
--
Carlos

bug#32344:

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 24 bytes --]

Forgot the screenshots.

[-- Attachment #2: Screenshot from 2019-10-13 16-16-25.png --]
[-- Type: image/png, Size: 16434 bytes --]

[-- Attachment #3: Screenshot from 2019-10-13 16-21-40.png --]
[-- Type: image/png, Size: 19457 bytes --]

bug#32344: [PATCH] 26.3/27 Option to avoid comint override colorization provided by major modes

[Carlos Pita]

[-- Attachment #1: Type: text/plain, Size: 168 bytes --]

Here is another simple patch that takes advantage of the change in
comint to actually colorize python output. This should be applied
after the previous one, of course.

[-- Attachment #2: 0001-Keep-python-shell-input-colorization-in-comint.patch --]
[-- Type: text/x-patch, Size: 1021 bytes --]

From 1f20366f37e32e7abfea66e4c8e3e565714701c0 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Sun, 13 Oct 2019 18:42:37 -0300
Subject: [PATCH] Keep python shell input colorization in comint

* progmodes/python.el (inferior-python-mode): use the new comint
`comit-highlight-input' customization option in order to keep
colorization done by python.el.
---
 lisp/progmodes/python.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 4e0a73c..0f366c0 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2755,6 +2755,7 @@ inferior-python-mode
          python-pdbtrack-comint-output-filter-function
          python-comint-postoutput-scroll-to-bottom
          comint-watch-for-password-prompt))
+  (set (make-local-variable 'comint-highlight-input) nil)
   (set (make-local-variable 'compilation-error-regexp-alist)
        python-shell-compilation-regexp-alist)
   (add-hook 'completion-at-point-functions
-- 
2.20.1

bug#32344: [PATCH] 26.3/27 Option to avoid comint override colorization provided by major modes

[Carlos Pita]

I've merged both patches since the python.el one was trivial and
dependent on the other and the main target of the new option.

I've also rewritten the commit message following the conventions
described in CONTRIBUTE.

Here is the commit so that you can easily comment online if you so
prefer: https://github.com/memeplex/emacs/commit/27acd97dfec7ecd9667a032e3a94650cb01244c1

Adding the .patch suffix to the same URL gives you a nicely formatted
patch, ready to be applied.

Best regards
--
Carlos

bug#32344: [PATCH] 26.3/27 Option to avoid comint override colorization provided by major modes

[Lars Ingebrigtsen]

Carlos Pita <carlosjosepita@gmail.com> writes:

> I've merged both patches since the python.el one was trivial and
> dependent on the other and the main target of the new option.
>
> I've also rewritten the commit message following the conventions
> described in CONTRIBUTE.
>
> Here is the commit so that you can easily comment online if you so
> prefer:
> https://github.com/memeplex/emacs/commit/27acd97dfec7ecd9667a032e3a94650cb01244c1

Could you post the patch here instead?  It's nice to have the proposed
patches in the bug tracker.

Anyway, while Juri's suggestion makes sense (merging faces) in general,
I think the mechanism here (making comint not do any face mangling for
the modes that do it themselves) is cleaner.  So I think Carlos' patch
should be applied to Emacs.  

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

bug#32344: [PATCH] 26.3/27 Option to avoid comint override colorization provided by major modes

[Lars Ingebrigtsen]

[-- Attachment #1: Type: text/plain, Size: 442 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Could you post the patch here instead?  It's nice to have the proposed
> patches in the bug tracker.

(Included below.)

> Anyway, while Juri's suggestion makes sense (merging faces) in general,
> I think the mechanism here (making comint not do any face mangling for
> the modes that do it themselves) is cleaner.  So I think Carlos' patch
> should be applied to Emacs.  

Which I'm doing now.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pita.patch --]
[-- Type: text/x-diff, Size: 2567 bytes --]

From 27acd97dfec7ecd9667a032e3a94650cb01244c1 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Sun, 13 Oct 2019 18:42:37 -0300
Subject: [PATCH] Keep python colorization in comint (Bug#32344)

* lisp/comint.el (comint-highlight-input, comint-send-input): add
option to let major modes that colorize their input stop comint from
overriding that specialized colorization.

* lisp/progmodes/python.el (inferior-python-mode): Use the new comint
`comint-highlight-input' customization option in order to keep
colorization done by python.el.
---
 lisp/comint.el           | 13 ++++++++++---
 lisp/progmodes/python.el |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index 4bb43670354..b8f5205d9e5 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -223,6 +223,12 @@ This variable is buffer-local."
 		 (other :tag "on" t))
   :group 'comint)
 
+(defcustom comint-highlight-input t
+  "If non-nil, highlight input with `comint-highlight-input' face.
+Otherwise keep the original highlighting untouched."
+  :type 'boolean
+  :group 'comint)
+
 (defface comint-highlight-input '((t (:weight bold)))
   "Face to use to highlight user input."
   :group 'comint)
@@ -1881,9 +1887,10 @@ Similarly for Soar, Scheme, etc."
               (end (if no-newline (point) (1- (point)))))
           (with-silent-modifications
             (when (> end beg)
-              (add-text-properties beg end
-                                   '(front-sticky t
-                                     font-lock-face comint-highlight-input))
+              (when comint-highlight-input
+               (add-text-properties beg end
+                                    '(front-sticky t
+                                      font-lock-face comint-highlight-input)))
               (unless comint-use-prompt-regexp
                 ;; Give old user input a field property of `input', to
                 ;; distinguish it from both process output and unsent
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index b168b62c291..8f823532802 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -2796,6 +2796,7 @@ variable.
          python-pdbtrack-comint-output-filter-function
          python-comint-postoutput-scroll-to-bottom
          comint-watch-for-password-prompt))
+  (set (make-local-variable 'comint-highlight-input) nil)
   (set (make-local-variable 'compilation-error-regexp-alist)
        python-shell-compilation-regexp-alist)
   (add-hook 'completion-at-point-functions

[-- Attachment #3: Type: text/plain, Size: 105 bytes --]



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

bug#32344:

[Juri Linkov]

> Forgot the screenshots.

So nice looking screenshot, thanks.

But I wonder if you could just replace add-text-properties with
add-face-text-property?  It will keep your previous text properties
and will just merge them with the comint-highlight-input face.

So when the face comint-highlight-input is customized to have
no face attributes, add-face-text-property will do nothing.

But it would be more nice to change the default value of the face
comint-highlight-input to some light-grey background (it seems
a new face attribute ':extend' is not needed here).
Then add-face-text-property will add this background
to your fontified comint input.

PS: would other modes benefit from fontifying the input
with their syntax.  In particular, I don't understand
why the input is not fontified using emacs-lisp-mode
in the minibuffer of ‘M-:’ (‘eval-expression’)

bug#32344: [PATCH] 26.3/27 Option to avoid comint override colorization provided by major modes

[Gaby Launay via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Elpy maintainer here.

The advice works fine for me as well (for quite a long time now).
But Elpy tries to offer an out-of-the-box IDE, and persistent python font coloring in the shell would be very nice.

I tested Carlos' two patches just now and it works like a charm for me.
Let me know if there is anything I can do to help.

--

bug#32344: Add option to keep colorization in comint, use it in python.el

[Carlos Pita]

> But I wonder if you could just replace add-text-properties with
> add-face-text-property?  It will keep your previous text properties
> and will just merge them with the comint-highlight-input face.

I'm not sure about this:

> If any text in the region already has a non-nil ‘face’ property, those
> face(s) are retained.

Is the default face represented as a nil face property? Will it be
overridden? In that case the mix could be ugly.

In general I'd prefer to keep both paths of execution as separate as
possible so as to avoid regressions.

bug#32344: Add option to keep colorization in comint, use it in python.el

[Carlos Pita]

Added :version and NEWS entry.

https://github.com/memeplex/emacs/commit/0b2e39ec139cca6031e277e68c5203c056792276

bug#32344: Add option to keep colorization in comint, use it in python.el

[Juri Linkov]

>> But I wonder if you could just replace add-text-properties with
>> add-face-text-property?  It will keep your previous text properties
>> and will just merge them with the comint-highlight-input face.
>
> I'm not sure about this:
>
>> If any text in the region already has a non-nil ‘face’ property, those
>> face(s) are retained.
>
> Is the default face represented as a nil face property? Will it be
> overridden? In that case the mix could be ugly.

Merging with nil is a no-op anyway.

> In general I'd prefer to keep both paths of execution as separate as
> possible so as to avoid regressions.

The result would be very nice when font-lock faces that highlight keywords
are merged with the background color specified by the comint-highlight-input
face.