comint fix for shell colors

comint fix for shell colors

[Erik Selberg]

[-- Attachment #1.1: Type: text/plain, Size: 490 bytes --]

I've been trying to use shell (vs term / mult-term) with bash / zsh and my
color prompts. In comint, comint-highlight-prompt clobbers the colors. I
made the following patch against 24.5 but I see comint has been updated in
25. What's the right process for adding this for 25? This is solved for me
locally, but seems something that could be added for all.

Screenshot included (I use a variant of the agnoster theme and zsh, along
with solarized-dark and power line mode lines)

Thanks,
-e

[-- Attachment #1.2: Type: text/html, Size: 578 bytes --]

[-- Attachment #2: comint-24.5.patch --]
[-- Type: application/octet-stream, Size: 3186 bytes --]

*** /tmp/comint.el	2016-11-14 23:04:50.000000000 -0800
--- /Users/selberg/elisp/comint.el	2016-11-14 23:06:36.000000000 -0800
***************
*** 104,109 ****
--- 104,110 ----
  (require 'ring)
  (require 'ansi-color)
  (require 'regexp-opt)                   ;For regexp-opt-charset.
+ (require 'simple)                       ;For password-word-equivalents
  \f
  ;; Buffer Local Variables:
  ;;============================================================================
***************
*** 154,159 ****
--- 155,168 ----
  ;;;   :prefix "comint-"
  ;;;   :group 'comint)
  
+ (defcustom comint-do-highlight-prompt t
+   "Highlight prompt. Set to nil to let underlying process set colors (useful for modern shells)"
+   :type 'boolean
+   :group 'comint
+   :version "22.1"
+   )
+ 
+ 
  (defvar comint-prompt-regexp "^"
    "Regexp to recognize prompts in the inferior process.
  Defaults to \"^\", the null string at BOL.
*************** the start, the cdr to the end of the las
*** 1922,1936 ****
    "Snapshot the current `comint-last-prompt'.
  Freezes the `font-lock-face' text property in place."
    (when comint-last-prompt
      (with-silent-modifications
        (add-text-properties
         (car comint-last-prompt)
         (cdr comint-last-prompt)
!        '(font-lock-face comint-highlight-prompt)))
      ;; Reset comint-last-prompt so later on comint-output-filter does
      ;; not remove the font-lock-face text property of the previous
      ;; (this) prompt.
!     (setq comint-last-prompt nil)))
  
  (defun comint-carriage-motion (start end)
    "Interpret carriage control characters in the region from START to END.
--- 1931,1949 ----
    "Snapshot the current `comint-last-prompt'.
  Freezes the `font-lock-face' text property in place."
    (when comint-last-prompt
+     (if comint-do-highlight-prompt
      (with-silent-modifications
        (add-text-properties
         (car comint-last-prompt)
         (cdr comint-last-prompt)
!        '(font-lock-face comint-highlight-prompt))
!       )
!     )
      ;; Reset comint-last-prompt so later on comint-output-filter does
      ;; not remove the font-lock-face text property of the previous
      ;; (this) prompt.
!     (setq comint-last-prompt nil)
!     ))
  
  (defun comint-carriage-motion (start end)
    "Interpret carriage control characters in the region from START to END.
*************** Make backspaces delete the previous char
*** 2082,2090 ****
  					'(font-lock-face)))
  	      (setq comint-last-prompt
  		    (cons (copy-marker prompt-start) (point-marker)))
! 	      (add-text-properties prompt-start (point)
! 				   '(rear-nonsticky t
! 				     font-lock-face comint-highlight-prompt)))
  	    (goto-char saved-point)))))))
  
  (defun comint-preinput-scroll-to-bottom ()
--- 2095,2106 ----
  					'(font-lock-face)))
  	      (setq comint-last-prompt
  		    (cons (copy-marker prompt-start) (point-marker)))
! 	      (if comint-do-highlight-prompt
! 		  (add-text-properties prompt-start (point)
! 				       '(rear-nonsticky t
! 							font-lock-face comint-highlight-prompt))
! 		)
! 	      )
  	    (goto-char saved-point)))))))
  
  (defun comint-preinput-scroll-to-bottom ()

[-- Attachment #3: Screen Shot 2016-11-14 at 11.12.29 PM.png --]
[-- Type: image/png, Size: 54018 bytes --]

Re: comint fix for shell colors

[Eli Zaretskii]

> From: Erik Selberg <erik@selberg.org>
> Date: Mon, 14 Nov 2016 23:14:25 -0800
> 
> I've been trying to use shell (vs term / mult-term) with bash / zsh and my color prompts. In comint,
> comint-highlight-prompt clobbers the colors. I made the following patch against 24.5 but I see comint has
> been updated in 25. What's the right process for adding this for 25? This is solved for me locally, but seems
> something that could be added for all.

Please rebase the patch on the current master branch and resubmit.

See also a few minor comments below.

> *** /tmp/comint.el	2016-11-14 23:04:50.000000000 -0800
> --- /Users/selberg/elisp/comint.el	2016-11-14 23:06:36.000000000 -0800
> ***************
> *** 104,109 ****
> --- 104,110 ----
>   (require 'ring)
>   (require 'ansi-color)
>   (require 'regexp-opt)                   ;For regexp-opt-charset.
> + (require 'simple)                       ;For password-word-equivalents

simple.el is preloaded, so there shouldn't be a need to require it.

> + (defcustom comint-do-highlight-prompt t
> +   "Highlight prompt. Set to nil to let underlying process set colors (useful for modern shells)"

The first line of the doc string should be a single complete
sentence.  The rest should go to the next lines.

> +   :type 'boolean
> +   :group 'comint
> +   :version "22.1"

Please update the version tag to 26.1.

> +   )

Please don't put closing parentheses on a separate line.

>     "Snapshot the current `comint-last-prompt'.
>   Freezes the `font-lock-face' text property in place."
>     (when comint-last-prompt
> +     (if comint-do-highlight-prompt
>       (with-silent-modifications
>         (add-text-properties
>          (car comint-last-prompt)
>          (cdr comint-last-prompt)
> !        '(font-lock-face comint-highlight-prompt))
> !       )
> !     )

Once again, please put all the closing parentheses on the last line of
their sexp.

>       ;; Reset comint-last-prompt so later on comint-output-filter does
>       ;; not remove the font-lock-face text property of the previous
>       ;; (this) prompt.
> !     (setq comint-last-prompt nil)
> !     ))

Same here.

>   	      (setq comint-last-prompt
>   		    (cons (copy-marker prompt-start) (point-marker)))
> ! 	      (if comint-do-highlight-prompt
> ! 		  (add-text-properties prompt-start (point)
> ! 				       '(rear-nonsticky t
> ! 							font-lock-face comint-highlight-prompt))
> ! 		)
> ! 	      )

And here.

Finally, please include with the patch a ChangeLog-style commit log
message for the changes, and a suitable entry for NEWS.

Thanks for working on this.

Re: comint fix for shell colors

[Wolfgang Jenkner]

Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Erik Selberg <erik@selberg.org>
> > Date: Mon, 14 Nov 2016 23:14:25 -0800
> > 
> > I've been trying to use shell (vs term / mult-term) with bash / zsh and my color prompts. In comint,
> > comint-highlight-prompt clobbers the colors. I made the following patch against 24.5 but I see comint has
> > been updated in 25. What's the right process for adding this for 25? This is solved for me locally, but seems
> > something that could be added for all.
>
> Please rebase the patch on the current master branch and resubmit.

In emacs-25 and master comint-highlight-prompt is prepended to
ansi-color faces used for highlighting prompts.  The user has just
to customize the comint-highlight-prompt face so that it doesn't
inherit from minibuffer-prompt and it won't clobber anything.  See

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20084

commit 792d44b3c31d2a682607ab8b79ae7d26b7402f41

So I think there's nothing to do here execept perhaps for documenting
something.

Wolfgang

Re: comint fix for shell colors

[Eli Zaretskii]

> Date: Fri, 18 Nov 2016 15:25:41 +0100
> From: Wolfgang Jenkner <wjenkner@inode.at>
> Cc: emacs-devel@gnu.org
> 
> In emacs-25 and master comint-highlight-prompt is prepended to
> ansi-color faces used for highlighting prompts.  The user has just
> to customize the comint-highlight-prompt face so that it doesn't
> inherit from minibuffer-prompt and it won't clobber anything.  See
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20084
> 
> commit 792d44b3c31d2a682607ab8b79ae7d26b7402f41

Thanks.  (I wonder why you didn't respond to the original message --
it was posted 4 days ago, and I waited for someone knowledgeable to
chime in before replying.)

> So I think there's nothing to do here execept perhaps for documenting
> something.

Indeed, I think this change should be described in NEWS.  Could you
provide a patch for that, please?

Re: comint fix for shell colors

[Wolfgang Jenkner]

Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Fri, 18 Nov 2016 15:25:41 +0100
> > From: Wolfgang Jenkner <wjenkner@inode.at>
> > Cc: emacs-devel@gnu.org
> > 
> > In emacs-25 and master comint-highlight-prompt is prepended to
> > ansi-color faces used for highlighting prompts.  The user has just
> > to customize the comint-highlight-prompt face so that it doesn't
> > inherit from minibuffer-prompt and it won't clobber anything.  See
> > 
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=20084
> > 
> > commit 792d44b3c31d2a682607ab8b79ae7d26b7402f41
>
[...]
> Indeed, I think this change should be described in NEWS.  Could you
> provide a patch for that, please?

Please note that there's nothing new here, it was just a bug fix.
The very first message in the bug report mentioned above states

        In Emacs 24.3, comint-highlight-prompt allows ANSI colors to show through
        if it does not specify any colors itself. In Emacs 24.4, it does not, even
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        if comint-highlight-prompt is the "empty face".

Wolfgang

Re: comint fix for shell colors

[Eli Zaretskii]

> Date: Fri, 18 Nov 2016 16:06:28 +0100
> From: Wolfgang Jenkner <wjenkner@inode.at>
> Cc: erik@selberg.org, emacs-devel@gnu.org
> 
> > Indeed, I think this change should be described in NEWS.  Could you
> > provide a patch for that, please?
> 
> Please note that there's nothing new here, it was just a bug fix.
> The very first message in the bug report mentioned above states
> 
>         In Emacs 24.3, comint-highlight-prompt allows ANSI colors to show through
>         if it does not specify any colors itself. In Emacs 24.4, it does not, even
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>         if comint-highlight-prompt is the "empty face".

Then how do you explain the OP's confusion on this matter?

Re: comint fix for shell colors

[Erik Selberg]

Thanks for the feedback, I'll rebase.

I do believe this is needed; when the shell prompt is multicolor (for examine using the agnoster theme as shown in the screenshot) applying a face removes the shell coloring.

Cheers,
-e

Sent from my iPhone

On Nov 18, 2016, at 7:34 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Fri, 18 Nov 2016 16:06:28 +0100
>> From: Wolfgang Jenkner <wjenkner@inode.at>
>> Cc: erik@selberg.org, emacs-devel@gnu.org
>> 
>>> Indeed, I think this change should be described in NEWS.  Could you
>>> provide a patch for that, please?
>> 
>> Please note that there's nothing new here, it was just a bug fix.
>> The very first message in the bug report mentioned above states
>> 
>>        In Emacs 24.3, comint-highlight-prompt allows ANSI colors to show through
>>        if it does not specify any colors itself. In Emacs 24.4, it does not, even
>>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>        if comint-highlight-prompt is the "empty face".
> 
> Then how do you explain the OP's confusion on this matter?
> 

Re: comint fix for shell colors

[Wolfgang Jenkner]

Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Fri, 18 Nov 2016 16:06:28 +0100
> > From: Wolfgang Jenkner <wjenkner@inode.at>
> > Cc: erik@selberg.org, emacs-devel@gnu.org
> > 
> > > Indeed, I think this change should be described in NEWS.  Could you
> > > provide a patch for that, please?
> > 
> > Please note that there's nothing new here, it was just a bug fix.
> > The very first message in the bug report mentioned above states
> > 
> >         In Emacs 24.3, comint-highlight-prompt allows ANSI colors to show through
> >         if it does not specify any colors itself. In Emacs 24.4, it does not, even
> >         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >         if comint-highlight-prompt is the "empty face".
>
> Then how do you explain the OP's confusion on this matter?

He said that he "made the following patch against 24.5", so I guess
that's what he runs and that release still had the regression
reported in bug#20084 (see my closing message there).

Wolfgang

Re: comint fix for shell colors

[Erik Selberg]

I see fixed in 25.1. Thx ill give that a look-see.

Cheers,
-e

Sent from my iPhone

> On Nov 18, 2016, at 7:49 AM, Wolfgang Jenkner <wjenkner@inode.at> wrote:
> 
> Eli Zaretskii <eliz@gnu.org> wrote:
> 
>>> Date: Fri, 18 Nov 2016 16:06:28 +0100
>>> From: Wolfgang Jenkner <wjenkner@inode.at>
>>> Cc: erik@selberg.org, emacs-devel@gnu.org
>>> 
>>>> Indeed, I think this change should be described in NEWS.  Could you
>>>> provide a patch for that, please?
>>> 
>>> Please note that there's nothing new here, it was just a bug fix.
>>> The very first message in the bug report mentioned above states
>>> 
>>>        In Emacs 24.3, comint-highlight-prompt allows ANSI colors to show through
>>>        if it does not specify any colors itself. In Emacs 24.4, it does not, even
>>>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>        if comint-highlight-prompt is the "empty face".
>> 
>> Then how do you explain the OP's confusion on this matter?
> 
> He said that he "made the following patch against 24.5", so I guess
> that's what he runs and that release still had the regression
> reported in bug#20084 (see my closing message there).
> 
> Wolfgang
> 
> 

Re: comint fix for shell colors

[Erik Selberg]

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

Confirmed fixed in 25.1. I did have to remove the inheritance from
mode-line to get the font proper, but I knew to look for that. Perhaps
disabling inheritance by default? Or something for NEWS. :)

Thanks!
Erik


On Fri, Nov 18, 2016 at 7:53 AM, Erik Selberg <erik@selberg.org> wrote:

> I see fixed in 25.1. Thx ill give that a look-see.
>
> Cheers,
> -e
>
> Sent from my iPhone
>
> > On Nov 18, 2016, at 7:49 AM, Wolfgang Jenkner <wjenkner@inode.at> wrote:
> >
> > Eli Zaretskii <eliz@gnu.org> wrote:
> >
> >>> Date: Fri, 18 Nov 2016 16:06:28 +0100
> >>> From: Wolfgang Jenkner <wjenkner@inode.at>
> >>> Cc: erik@selberg.org, emacs-devel@gnu.org
> >>>
> >>>> Indeed, I think this change should be described in NEWS.  Could you
> >>>> provide a patch for that, please?
> >>>
> >>> Please note that there's nothing new here, it was just a bug fix.
> >>> The very first message in the bug report mentioned above states
> >>>
> >>>        In Emacs 24.3, comint-highlight-prompt allows ANSI colors to
> show through
> >>>        if it does not specify any colors itself. In Emacs 24.4, it
> does not, even
> >>>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>>        if comint-highlight-prompt is the "empty face".
> >>
> >> Then how do you explain the OP's confusion on this matter?
> >
> > He said that he "made the following patch against 24.5", so I guess
> > that's what he runs and that release still had the regression
> > reported in bug#20084 (see my closing message there).
> >
> > Wolfgang
> >
> >
>

[-- Attachment #2: Type: text/html, Size: 2478 bytes --]

Re: comint fix for shell colors

[Eli Zaretskii]

> From: Erik Selberg <erik@selberg.org>
> Date: Fri, 18 Nov 2016 11:24:08 -0800
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> Confirmed fixed in 25.1. I did have to remove the inheritance from mode-line to get the font proper, but I knew
> to look for that. Perhaps disabling inheritance by default? Or something for NEWS. :)

Thanks.  What would you suggest to say in NEWS?  Or maybe in some
relevant doc string?

Re: comint fix for shell colors

[Erik Selberg]

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

I'm not 100% sure why comint-prompt-highlight inherits from modeline. I
know close to 100% of the people I work with have some kind of colored
prompt, so rather than make everyone who does that and uses shell, I'd set
the default to not inherit from modeline.

absent that, something like:
FIXED: comint will now properly pass through colors from inferior processes
(e.g. shell color prompt). Please disable inheritance from
comint-prompt-highlight via (....)

-e

On Fri, Nov 18, 2016 at 11:50 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Erik Selberg <erik@selberg.org>
> > Date: Fri, 18 Nov 2016 11:24:08 -0800
> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> >
> > Confirmed fixed in 25.1. I did have to remove the inheritance from
> mode-line to get the font proper, but I knew
> > to look for that. Perhaps disabling inheritance by default? Or something
> for NEWS. :)
>
> Thanks.  What would you suggest to say in NEWS?  Or maybe in some
> relevant doc string?
>
>

[-- Attachment #2: Type: text/html, Size: 1535 bytes --]