unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* Setting term-default-fg-color/term-default-bg-color has no effect
@ 2007-06-30  0:17 Peter Povinec
  2007-07-02 17:28 ` Dan Nicolaescu
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Povinec @ 2007-06-30  0:17 UTC (permalink / raw)
  To: bug-gnu-emacs

Hi!

I use ansi-term a lot and like to have different color scheme for my term
buffers.  After upgrading to Emacs22 from 21.4 I notice that my setting of
term-default-fg-color/term-default-bg-color no longer has any effect.  I found
no documentation stating that it was not supported in Emacs22, so I suppose it
is a bug. 

I looked at the old 21.4 code and based on that created the following minimalist
patch, which is satisfactory in addressing the issue.  

HTH,
--Peter



2007-06-25  Peter Povinec  <ppovinec <at> yahoo.com>

	* term.el: Honor term-default-fg-color and term-default-bg-color
	settings when modifying term-current-face.


*** /home/ppovinec/term.el.orig	2007-06-24 01:04:07.298996000 -0700
--- /home/ppovinec/term.el	2007-06-25 16:40:33.434255000 -0700
***************
*** 695,710 ****
  
  ;;; faces -mm
  
! (defcustom term-default-fg-color 'unspecified
    "Default color for foreground in `term'."
    :group 'term
    :type 'string)
  
! (defcustom term-default-bg-color 'unspecified
    "Default color for background in `term'."
    :group 'term
    :type 'string)
  
  ;;; Use the same colors that xterm uses, see `xterm-standard-colors'.
  (defvar ansi-term-color-vector
    [unspecified "black" "red3" "green3" "yellow3" "blue2"
--- 695,714 ----
  
  ;;; faces -mm
  
! (defcustom term-default-fg-color (face-foreground term-current-face)
    "Default color for foreground in `term'."
    :group 'term
    :type 'string)
  
! (defcustom term-default-bg-color (face-background term-current-face)
    "Default color for background in `term'."
    :group 'term
    :type 'string)
  
+ ;;; Start with requested fg/bg.
+ (setq term-current-face (list :background term-default-bg-color 
+                               :foreground term-default-fg-color))
+ 
  ;;; Use the same colors that xterm uses, see `xterm-standard-colors'.
  (defvar ansi-term-color-vector
    [unspecified "black" "red3" "green3" "yellow3" "blue2"
***************
*** 3055,3061 ****
    (setq term-scroll-start 0)
    (setq term-scroll-end term-height)
    (setq term-insert-mode nil)
!   (setq term-current-face nil)
    (setq term-ansi-current-underline nil)
    (setq term-ansi-current-bold nil)
    (setq term-ansi-current-reverse nil)
--- 3059,3066 ----
    (setq term-scroll-start 0)
    (setq term-scroll-end term-height)
    (setq term-insert-mode nil)
!   (setq term-current-face (list :background term-default-bg-color 
!                                 :foreground term-default-fg-color))
    (setq term-ansi-current-underline nil)
    (setq term-ansi-current-bold nil)
    (setq term-ansi-current-reverse nil)
***************
*** 3117,3123 ****
  
  ;;; 0 (Reset) or unknown (reset anyway)
     (t
!     (setq term-current-face nil)
      (setq term-ansi-current-underline nil)
      (setq term-ansi-current-bold nil)
      (setq term-ansi-current-reverse nil)
--- 3122,3129 ----
  
  ;;; 0 (Reset) or unknown (reset anyway)
     (t
!     (setq term-current-face (list :background term-default-bg-color 
! 				  :foreground term-default-fg-color))
      (setq term-ansi-current-underline nil)
      (setq term-ansi-current-bold nil)
      (setq term-ansi-current-reverse nil)
***************
*** 3154,3164 ****
  	    (setq term-current-face
  		  (list :background
  			(if (= term-ansi-current-color 0)
! 			    (face-foreground 'default)
  			  (elt ansi-term-color-vector term-ansi-current-color))
  			:foreground
  			(if (= term-ansi-current-bg-color 0)
! 			    (face-background 'default)
  			  (elt ansi-term-color-vector term-ansi-current-bg-color))))
  	    (when term-ansi-current-bold
  	      (setq term-current-face
--- 3160,3170 ----
  	    (setq term-current-face
  		  (list :background
  			(if (= term-ansi-current-color 0)
! 			    term-default-fg-color
  			  (elt ansi-term-color-vector term-ansi-current-color))
  			:foreground
  			(if (= term-ansi-current-bg-color 0)
! 			    term-default-bg-color
  			  (elt ansi-term-color-vector term-ansi-current-bg-color))))
  	    (when term-ansi-current-bold
  	      (setq term-current-face
***************
*** 3181,3189 ****
  		  )
  	  (setq term-current-face
  		(list :foreground
! 		      (elt ansi-term-color-vector term-ansi-current-color)
  		      :background
  		      (elt ansi-term-color-vector term-ansi-current-bg-color)))
  	  (when term-ansi-current-bold
  	    (setq term-current-face
  		  (append '(:weight bold) term-current-face)))
--- 3187,3200 ----
  		  )
  	  (setq term-current-face
  		(list :foreground
!                       (if (= term-ansi-current-color 0)
!                           term-default-fg-color
!                         (elt ansi-term-color-vector term-ansi-current-color))
  		      :background
+                       (if (= term-ansi-current-bg-color 0)
+                           term-default-bg-color
  		      (elt ansi-term-color-vector term-ansi-current-bg-color)))
+                 )
  	  (when term-ansi-current-bold
  	    (setq term-current-face
  		  (append '(:weight bold) term-current-face)))

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-06-30  0:17 Setting term-default-fg-color/term-default-bg-color has no effect Peter Povinec
@ 2007-07-02 17:28 ` Dan Nicolaescu
  2007-07-23 19:22   ` Peter Povinec
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Nicolaescu @ 2007-07-02 17:28 UTC (permalink / raw)
  To: Peter Povinec; +Cc: bug-gnu-emacs

Peter Povinec <pp_publiclists@yahoo.com> writes:

  > Hi!
  > 
  > I use ansi-term a lot and like to have different color scheme for my term
  > buffers.  After upgrading to Emacs22 from 21.4 I notice that my setting of
  > term-default-fg-color/term-default-bg-color no longer has any effect.  I found
  > no documentation stating that it was not supported in Emacs22, so I suppose it
  > is a bug. 
  > 
  > I looked at the old 21.4 code and based on that created the following minimalist
  > patch, which is satisfactory in addressing the issue.  


Can you please describe exactly what are you doing and how? How did
you test your patch? 


  > 2007-06-25  Peter Povinec  <ppovinec <at> yahoo.com>
  > 
  > 	* term.el: Honor term-default-fg-color and term-default-bg-color
  > 	settings when modifying term-current-face.

Please look at the other ChangeLog entries in emacs, it should
describe the changes to each function/variable


  >   
  > ! (defcustom term-default-fg-color (face-foreground term-current-face)
  >     "Default color for foreground in `term'."
  >     :group 'term
  >     :type 'string)
  >   
  > ! (defcustom term-default-bg-color (face-background term-current-face)
  >     "Default color for background in `term'."
  >     :group 'term
  >     :type 'string)

This can't be right, term-current-face is nil when term.el is
loaded... 


  > + ;;; Start with requested fg/bg.
  > + (setq term-current-face (list :background term-default-bg-color 
  > +                               :foreground term-default-fg-color))

We don't want forms that have side effects at the top level...


Thanks

        --dan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-07-02 17:28 ` Dan Nicolaescu
@ 2007-07-23 19:22   ` Peter Povinec
  2007-08-01  4:10     ` Dan Nicolaescu
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Povinec @ 2007-07-23 19:22 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: bug-gnu-emacs

> Can you please describe exactly what are you doing and how? How did
> you test your patch? 

The basic idea is the same as it was in Emacs 21.  Take the user preferences
in terms of fg/bg colors as stored in term-default-fg-color and
term-default-bg-color and apply them consistently every time term-current-face
is set.  This is done for the initial value of term-current-face, when the
terminal is reset in term-reset-terminal, and when the escape sequences
setting the default fg/bg colors are interpreted in term-handle-color-array. 
Note that this was partially done already in term-handle-color-array, e.g. in
the case of  setting reverse+invisible. 

For the testing, I've tested with my color customizations, including colored
terminal prompt in csh.  I verified the reverse video logic by running 'top'
and 'emacs -nw' inside an ansi-term, both with my customizations, and plain
emacs.  Customizations to term-default-fg-color or term-default-bg-color while
one or more term sessions are running take effect as expected, e.g. by running
'reset'.  

 
>   >   
>   > ! (defcustom term-default-fg-color (face-foreground term-current-face)
>   >     "Default color for foreground in `term'."
>   >     :group 'term
>   >     :type 'string)
>   >   
>   > ! (defcustom term-default-bg-color (face-background term-current-face)
>   >     "Default color for background in `term'."
>   >     :group 'term
>   >     :type 'string)
> 
> This can't be right, term-current-face is nil when term.el is
> loaded... 

See line 462 (in revision 1.89):
(defvar term-current-face 'default)

I'd agree the code would be easier to read if this line was moved, but as I
mentioned I didn't want to do any unnecessary changes to keep this a tiny
patch.


>   > + ;;; Start with requested fg/bg.
>   > + (setq term-current-face (list :background term-default-bg-color 
>   > +                               :foreground term-default-fg-color))
> 
> We don't want forms that have side effects at the top level...

Moved to (term-mode) now.

The updated patch is below.
HTH,
--Peter


2007-07-23  Peter Povinec  <ppovinec <at> yahoo.com>

	* term.el: Honor term-default-fg-color and term-default-bg-color 
	settings when modifying term-current-face.
	(term-default-fg-color, term-default-bg-color): initialized from 
	default term-current-face
	(term-mode, term-reset-terminal): set term-current-face with 
	term-default-fg-color and term-default-bg-color
	(term-handle-colors-array): term-current-face has term-default-fg-color
	and term-default-bg-color after reset escape sequence
	(term-handle-colors-array): set term-current-color with
	term-default-fg/bg-color instead of ansi-term-color-vector when the 
	index (term-ansi-current-color or term-ansi-current-bg-color) is zero


*** /home/ppovinec/term.el.orig	2007-06-24 01:04:07.298996000 -0700
--- /home/ppovinec/term.el	2007-07-23 12:18:01.418489000 -0700
***************
*** 695,706 ****
  
  ;;; faces -mm
  
! (defcustom term-default-fg-color 'unspecified
    "Default color for foreground in `term'."
    :group 'term
    :type 'string)
  
! (defcustom term-default-bg-color 'unspecified
    "Default color for background in `term'."
    :group 'term
    :type 'string)
--- 695,706 ----
  
  ;;; faces -mm
  
! (defcustom term-default-fg-color (face-foreground term-current-face)
    "Default color for foreground in `term'."
    :group 'term
    :type 'string)
  
! (defcustom term-default-bg-color (face-background term-current-face)
    "Default color for background in `term'."
    :group 'term
    :type 'string)
***************
*** 1098,1103 ****
--- 1098,1105 ----
    (make-local-variable 'term-pending-delete-marker)
    (setq term-pending-delete-marker (make-marker))
    (make-local-variable 'term-current-face)
+   (setq term-current-face (list :background term-default-bg-color 
+                                 :foreground term-default-fg-color))
    (make-local-variable 'term-pending-frame)
    (setq term-pending-frame nil)
    ;; Cua-mode's keybindings interfere with the term keybindings, disable it.
***************
*** 3055,3061 ****
    (setq term-scroll-start 0)
    (setq term-scroll-end term-height)
    (setq term-insert-mode nil)
!   (setq term-current-face nil)
    (setq term-ansi-current-underline nil)
    (setq term-ansi-current-bold nil)
    (setq term-ansi-current-reverse nil)
--- 3057,3064 ----
    (setq term-scroll-start 0)
    (setq term-scroll-end term-height)
    (setq term-insert-mode nil)
!   (setq term-current-face (list :background term-default-bg-color 
!                                 :foreground term-default-fg-color))
    (setq term-ansi-current-underline nil)
    (setq term-ansi-current-bold nil)
    (setq term-ansi-current-reverse nil)
***************
*** 3117,3123 ****
  
  ;;; 0 (Reset) or unknown (reset anyway)
     (t
!     (setq term-current-face nil)
      (setq term-ansi-current-underline nil)
      (setq term-ansi-current-bold nil)
      (setq term-ansi-current-reverse nil)
--- 3120,3127 ----
  
  ;;; 0 (Reset) or unknown (reset anyway)
     (t
!     (setq term-current-face (list :background term-default-bg-color 
!                                   :foreground term-default-fg-color))
      (setq term-ansi-current-underline nil)
      (setq term-ansi-current-bold nil)
      (setq term-ansi-current-reverse nil)
***************
*** 3154,3164 ****
  	    (setq term-current-face
  		  (list :background
  			(if (= term-ansi-current-color 0)
! 			    (face-foreground 'default)
  			  (elt ansi-term-color-vector term-ansi-current-color))
  			:foreground
  			(if (= term-ansi-current-bg-color 0)
! 			    (face-background 'default)
  			  (elt ansi-term-color-vector term-ansi-current-bg-color))))
  	    (when term-ansi-current-bold
  	      (setq term-current-face
--- 3158,3168 ----
  	    (setq term-current-face
  		  (list :background
  			(if (= term-ansi-current-color 0)
! 			    term-default-fg-color
  			  (elt ansi-term-color-vector term-ansi-current-color))
  			:foreground
  			(if (= term-ansi-current-bg-color 0)
! 			    term-default-bg-color
  			  (elt ansi-term-color-vector term-ansi-current-bg-color))))
  	    (when term-ansi-current-bold
  	      (setq term-current-face
***************
*** 3181,3189 ****
  		  )
  	  (setq term-current-face
  		(list :foreground
! 		      (elt ansi-term-color-vector term-ansi-current-color)
  		      :background
  		      (elt ansi-term-color-vector term-ansi-current-bg-color)))
  	  (when term-ansi-current-bold
  	    (setq term-current-face
  		  (append '(:weight bold) term-current-face)))
--- 3185,3198 ----
  		  )
  	  (setq term-current-face
  		(list :foreground
!                       (if (= term-ansi-current-color 0)
!                           term-default-fg-color
!                         (elt ansi-term-color-vector
term-ansi-current-color))
  		      :background
+                       (if (= term-ansi-current-bg-color 0)
+                           term-default-bg-color
  		      (elt ansi-term-color-vector term-ansi-current-bg-color)))
+                 )
  	  (when term-ansi-current-bold
  	    (setq term-current-face
  		  (append '(:weight bold) term-current-face)))







       
____________________________________________________________________________________
Choose the right car based on your needs.  Check out Yahoo! Autos new Car Finder tool.
http://autos.yahoo.com/carfinder/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-07-23 19:22   ` Peter Povinec
@ 2007-08-01  4:10     ` Dan Nicolaescu
  2007-08-02  0:50       ` Peter Povinec
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Nicolaescu @ 2007-08-01  4:10 UTC (permalink / raw)
  To: Peter Povinec; +Cc: bug-gnu-emacs

Peter Povinec <pp_publiclists@yahoo.com> writes:

  > > Can you please describe exactly what are you doing and how? How did
  > > you test your patch? 
  > 
  > The basic idea is the same as it was in Emacs 21.  Take the user preferences
  > in terms of fg/bg colors as stored in term-default-fg-color and
  > term-default-bg-color and apply them consistently every time term-current-face
  > is set.  This is done for the initial value of term-current-face, when the
  > terminal is reset in term-reset-terminal, and when the escape sequences
  > setting the default fg/bg colors are interpreted in term-handle-color-array. 
  > Note that this was partially done already in term-handle-color-array, e.g. in
  > the case of  setting reverse+invisible. 
  > 
  > For the testing, I've tested with my color customizations, including colored
  > terminal prompt in csh.  I verified the reverse video logic by running 'top'
  > and 'emacs -nw' inside an ansi-term, both with my customizations, and plain
  > emacs.  Customizations to term-default-fg-color or term-default-bg-color while
  > one or more term sessions are running take effect as expected, e.g. by running
  > 'reset'.  

I tested your changes by setting term-default-fg-color to blue and
term-default-bg-color to red on an emacs run with -Q and comparing
with running various curses applications in M-x term and "xterm -fg
blue -bg red". The results are not the same.  

The problem is that the screen is updated in a lot of places in
term.el not only term-handle-colors-array. So this could not have
worked correctly in emacs 21 either.

Given that AFAIK nothing else in emacs changes the default foreground
and background for a buffer, it's kind of hard to find a convincing
argument that such a feature is needed for term.el. It can probably be
done with some effort, but is it worth the added complexity? Not
sure... 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-08-01  4:10     ` Dan Nicolaescu
@ 2007-08-02  0:50       ` Peter Povinec
  2007-08-02  2:28         ` Dan Nicolaescu
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Povinec @ 2007-08-02  0:50 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: bug-gnu-emacs

> I tested your changes by setting term-default-fg-color to blue and
> term-default-bg-color to red on an emacs run with -Q and comparing
> with running various curses applications in M-x term and "xterm -fg
> blue -bg red". The results are not the same.  
Please be specific: what applications and how were the results different?  I'd
just like to know.

> 
> The problem is that the screen is updated in a lot of places in
> term.el not only term-handle-colors-array. So this could not have
> worked correctly in emacs 21 either.
That is quite possible.  The patch I submitted was intended to merely remove
the regression in functionality introduced in Emacs 22.  I believe it does
that and is useful on its own, whether additional color handling fixes are
applied or not. 

> 
> Given that AFAIK nothing else in emacs changes the default foreground
> and background for a buffer, it's kind of hard to find a convincing
> argument that such a feature is needed for term.el. It can probably be
> done with some effort, but is it worth the added complexity? Not
> sure... 
The additional complexity introduced by the patch is negligible.  We can look
at what it would take to fix the other color related issues you have seen, but
if those existed in version 21, I'd say they are separate issues and should be
treated as such.

--Peter




       
____________________________________________________________________________________
Take the Internet to Go: Yahoo!Go puts the Internet in your pocket: mail, news, photos & more. 
http://mobile.yahoo.com/go?refer=1GNXIC

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-08-02  0:50       ` Peter Povinec
@ 2007-08-02  2:28         ` Dan Nicolaescu
  2007-08-03  6:35           ` Peter Povinec
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Nicolaescu @ 2007-08-02  2:28 UTC (permalink / raw)
  To: Peter Povinec; +Cc: bug-gnu-emacs

Peter Povinec <pp_publiclists@yahoo.com> writes:

  > > I tested your changes by setting term-default-fg-color to blue and
  > > term-default-bg-color to red on an emacs run with -Q and comparing
  > > with running various curses applications in M-x term and "xterm -fg
  > > blue -bg red". The results are not the same.  
  > Please be specific: what applications and how were the results different?  I'd
  > just like to know.

I tried "emacs -nw", all empty lines did not use term-default-bg-color.

  > > The problem is that the screen is updated in a lot of places in
  > > term.el not only term-handle-colors-array. So this could not have
  > > worked correctly in emacs 21 either.
  > That is quite possible.  The patch I submitted was intended to merely remove
  > the regression in functionality introduced in Emacs 22.  I believe it does
  > that and is useful on its own, whether additional color handling fixes are
  > applied or not. 

It's hard to call a regression replacing one type of broken
behavior with another type of just slightly different broken
behavior.

  > > Given that AFAIK nothing else in emacs changes the default foreground
  > > and background for a buffer, it's kind of hard to find a convincing
  > > argument that such a feature is needed for term.el. It can probably be
  > > done with some effort, but is it worth the added complexity? Not
  > > sure... 
  > The additional complexity introduced by the patch is negligible.  We can look
  > at what it would take to fix the other color related issues you have seen, but
  > if those existed in version 21, I'd say they are separate issues and should be
  > treated as such.

I only see 2 possible fixes for this: either a patch that makes
changing the default fg and bg for M-x term work correctly, or simply
removing this configurability for the user. Anything else seems to be
just busy work. As I said, I am inclined to believe that the right
thing to do is the latter.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Setting term-default-fg-color/term-default-bg-color has no effect
  2007-08-02  2:28         ` Dan Nicolaescu
@ 2007-08-03  6:35           ` Peter Povinec
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Povinec @ 2007-08-03  6:35 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: bug-gnu-emacs

>   > > The problem is that the screen is updated in a lot of places in
>   > > term.el not only term-handle-colors-array. So this could not have
>   > > worked correctly in emacs 21 either.
>   > That is quite possible.  The patch I submitted was intended to merely
> remove
>   > the regression in functionality introduced in Emacs 22.  I believe it
> does
>   > that and is useful on its own, whether additional color handling fixes
> are
>   > applied or not. 
> 
> It's hard to call a regression replacing one type of broken
> behavior with another type of just slightly different broken
> behavior.
It is a complete loss of service to a user who only wanted to customize the
default fg color.  The regression is really that the settings are not honored,
not whether or not setting them results in expected behavior in all the cases.
 As I said, the patch attempts to restore the behavior to Emacs 21 level.  It
does not attempt to fix all related bugs that may exist in term.el.  For all I
know, the fact that the background of empty or short lines is not displayed
with the customized bg color may have been a design limitation, and considered
acceptable when the feature was first introduced.  

> I only see 2 possible fixes for this: either a patch that makes
> changing the default fg and bg for M-x term work correctly, or simply
> removing this configurability for the user. Anything else seems to be
> just busy work. As I said, I am inclined to believe that the right
> thing to do is the latter.
Sure, the full support would be preferred and if you could do that it would be
great.  But removing this configuration functionality completely just because
it continues to have the same unusual behavior in certain cases as it had in
prior releases would seem like a busy work to me...

--Peter



       
____________________________________________________________________________________
Looking for a deal? Find great prices on flights and hotels with Yahoo! FareChase.
http://farechase.yahoo.com/

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-08-03  6:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-30  0:17 Setting term-default-fg-color/term-default-bg-color has no effect Peter Povinec
2007-07-02 17:28 ` Dan Nicolaescu
2007-07-23 19:22   ` Peter Povinec
2007-08-01  4:10     ` Dan Nicolaescu
2007-08-02  0:50       ` Peter Povinec
2007-08-02  2:28         ` Dan Nicolaescu
2007-08-03  6:35           ` Peter Povinec

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).