From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: tty-color-mode Date: Fri, 04 Apr 2008 15:58:32 -0400 Message-ID: NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1207339130 14374 80.91.229.12 (4 Apr 2008 19:58:50 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 4 Apr 2008 19:58:50 +0000 (UTC) To: 80@emacsbugs.donarmstrong.com, emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Apr 04 21:59:21 2008 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.50) id 1Jhs4F-0007zR-SJ for ged-emacs-devel@m.gmane.org; Fri, 04 Apr 2008 21:59:20 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jhs3d-000161-6j for ged-emacs-devel@m.gmane.org; Fri, 04 Apr 2008 15:58:41 -0400 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Jhs3Z-00013l-1W for emacs-devel@gnu.org; Fri, 04 Apr 2008 15:58:37 -0400 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Jhs3W-00010I-Rg for emacs-devel@gnu.org; Fri, 04 Apr 2008 15:58:35 -0400 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Jhs3W-000104-MP for emacs-devel@gnu.org; Fri, 04 Apr 2008 15:58:34 -0400 Original-Received: from ironport2-out.pppoe.ca ([206.248.154.182] helo=ironport2-out.teksavvy.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Jhs3W-0000gB-6R for emacs-devel@gnu.org; Fri, 04 Apr 2008 15:58:34 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AmgCANsl9kfO+J/WdGdsb2JhbACBWo9zASqaDQ X-IronPort-AV: E=Sophos;i="4.25,606,1199682000"; d="scan'208";a="17841210" Original-Received: from smtp.pppoe.ca (HELO smtp.teksavvy.com) ([65.39.196.238]) by ironport2-out.teksavvy.com with ESMTP; 04 Apr 2008 15:58:33 -0400 Original-Received: from pastel.home ([206.248.159.214]) by smtp.teksavvy.com (Internet Mail Server v1.0) with ESMTP id KXK06133; Fri, 04 Apr 2008 15:58:33 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id BAC6D872B; Fri, 4 Apr 2008 15:58:32 -0400 (EDT) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) X-detected-kernel: by monty-python.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:94370 Archived-At: > I expected this to disable colors on the current frame: > (set-frame-parameter nil 'tty-color-mode -1) > But it disables colors on all tty frames. Indeed the code that handles this is wrong. It tries to handle it inside `select-frame' but does it at a time where it has no effect (it always ends up deciding that nothing has changed because it compares the new selected window with what it thinks is the old one, but is already the new one as well). Furthermore it's lucky that it doesn't work: if you fix the text to make it work, you then discover that it creates havoc, because you end up calling set_tty_color_mode (which runs Elisp code) directly from select-frame, which leads to breakage (and slow down because we end up recomputing faces even when we do a `with-selected-frame' so that the displayed frame actually never changes). I believe the patch below gets it right, but since I'm not familiar with this part of the code, I'd like other people to take a look at it: is it a good idea to set "FRAME_TTY (f)->previous_frame = NULL;" to force redisplay of the whole frame or should I use some other mechanism? Stefan Index: src/term.c =================================================================== RCS file: /sources/emacs/emacs/src/term.c,v retrieving revision 1.214 diff -u -r1.214 term.c --- src/term.c 27 Feb 2008 22:49:29 -0000 1.214 +++ src/term.c 4 Apr 2008 19:50:55 -0000 @@ -2161,56 +2161,40 @@ } void -set_tty_color_mode (f, val) +set_tty_color_mode (tty, f) + struct tty_display_info *tty; struct frame *f; - Lisp_Object val; { - Lisp_Object color_mode_spec, current_mode_spec; - Lisp_Object color_mode, current_mode; - int mode, old_mode; + Lisp_Object tem, val, color_mode_spec; + Lisp_Object color_mode; + int mode; extern Lisp_Object Qtty_color_mode; - Lisp_Object tty_color_mode_alist; + Lisp_Object tty_color_mode_alist + = Fintern_soft (build_string ("tty-color-mode-alist"), Qnil); - tty_color_mode_alist = Fintern_soft (build_string ("tty-color-mode-alist"), - Qnil); + eassert (FRAME_TERMCAP_P (f)); + eassert (FRAME_TTY (t) == tty); + tem = assq_no_quit (Qtty_color_mode, XFRAME (val)->param_alist); + val = CONSP (tem) ? XCDR (tem) : Qnil; if (INTEGERP (val)) color_mode = val; else { - if (NILP (tty_color_mode_alist)) - color_mode_spec = Qnil; - else - color_mode_spec = Fassq (val, XSYMBOL (tty_color_mode_alist)->value); - - if (CONSP (color_mode_spec)) - color_mode = XCDR (color_mode_spec); - else - color_mode = Qnil; + tem = (NILP (tty_color_mode_alist) ? Qnil + : Fassq (val, XSYMBOL (tty_color_mode_alist)->value)); + color_mode = CONSP (tem) ? XCDR (tem) : Qnil; } - current_mode_spec = assq_no_quit (Qtty_color_mode, f->param_alist); - - if (CONSP (current_mode_spec)) - current_mode = XCDR (current_mode_spec); - else - current_mode = Qnil; - if (INTEGERP (color_mode)) - mode = XINT (color_mode); - else - mode = 0; /* meaning default */ - if (INTEGERP (current_mode)) - old_mode = XINT (current_mode); - else - old_mode = 0; + mode = INTEGERP (color_mode) ? XINT (color_mode) : 0; - if (mode != old_mode) + if (mode != tty->previous_color_mode) { - tty_setup_colors (FRAME_TTY (f), mode); - /* This recomputes all the faces given the new color - definitions. */ - call0 (intern ("tty-set-up-initial-frame-faces")); - redraw_frame (f); + Lisp_Object funsym = intern ("tty-set-up-initial-frame-faces"); + tty->previous_color_mode = mode; + tty_setup_colors (tty , mode); + /* This recomputes all the faces given the new color definitions. */ + safe_call (1, &funsym); } } Index: src/frame.c =================================================================== RCS file: /sources/emacs/emacs/src/frame.c,v retrieving revision 1.369 diff -u -r1.369 frame.c --- src/frame.c 29 Mar 2008 01:46:08 -0000 1.369 +++ src/frame.c 4 Apr 2008 19:50:56 -0000 @@ -884,23 +884,6 @@ Fselect_window (XFRAME (frame)->selected_window, Qnil); -#ifndef WINDOWSNT - /* Make sure to switch the tty color mode to that of the newly - selected frame. */ - sf = SELECTED_FRAME (); - if (FRAME_TERMCAP_P (sf)) - { - Lisp_Object color_mode_spec, color_mode; - - color_mode_spec = assq_no_quit (Qtty_color_mode, sf->param_alist); - if (CONSP (color_mode_spec)) - color_mode = XCDR (color_mode_spec); - else - color_mode = make_number (0); - set_tty_color_mode (sf, color_mode); - } -#endif /* !WINDOWSNT */ - /* We want to make sure that the next event generates a frame-switch event to the appropriate frame. This seems kludgy to me, but before you take it out, make sure that evaluating something like @@ -2302,11 +2285,13 @@ } #ifndef WINDOWSNT - /* The tty color mode needs to be set before the frame's parameter - alist is updated with the new value, because set_tty_color_mode - wants to look at the old mode. */ - if (FRAME_TERMCAP_P (f) && EQ (prop, Qtty_color_mode)) - set_tty_color_mode (f, val); + /* The tty color needed to be set before the frame's parameter + alist was updated with the new value. This is not true any more, + but we still do this test early on. */ + if (FRAME_TERMCAP_P (f) && EQ (prop, Qtty_color_mode) + && f == FRAME_TTY (f)->previous_frame) + /* Force redisplay of this tty. */ + FRAME_TTY (f)->previous_frame = NULL; #endif /* Update the frame parameter alist. */ Index: src/dispextern.h =================================================================== RCS file: /sources/emacs/emacs/src/dispextern.h,v retrieving revision 1.241 diff -u -r1.241 dispextern.h --- src/dispextern.h 1 Mar 2008 22:30:51 -0000 1.241 +++ src/dispextern.h 4 Apr 2008 19:50:56 -0000 @@ -3057,7 +3057,7 @@ extern void produce_glyphs P_ ((struct it *)); extern void produce_special_glyphs P_ ((struct it *, enum display_element_type)); extern int tty_capable_p P_ ((struct tty_display_info *, unsigned, unsigned long, unsigned long)); -extern void set_tty_color_mode P_ ((struct frame *, Lisp_Object)); +extern void set_tty_color_mode (struct tty_display_info *, struct frame *); extern struct terminal *get_tty_terminal P_ ((Lisp_Object, int)); extern struct terminal *get_named_tty P_ ((char *)); EXFUN (Ftty_type, 1);