tty-display-color-cells problem

[Dan Nicolaescu]

After patch-550 vc-annotate does not work on X11, it gives an error
about the frame not being a TERMCAP terminal (or something of the
sort).

The problem is caused by tty-display-color-cells.
Here is a fix:

--- term.c.orig 2006-04-19 13:21:36.000000000 -0700
+++ term.c 2006-04-19 13:21:54.000000000 -0700
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (NILP(terminal)?selected_frame:terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else


It seems that a few other functions in term.c that call get_tty_terminal need the same treatment...

[Han Boetes]

Post by Dan Nicolaescu
--- term.c.orig 2006-04-19 13:21:36.000000000 -0700
+++ term.c 2006-04-19 13:21:54.000000000 -0700
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (NILP(terminal)?selected_frame:terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else

Ehm... appart from the whitespace I don't see a change. What am I
missing?


# Han

[Dan Nicolaescu]

Post by Han Boetes

Post by Dan Nicolaescu
--- term.c.orig 2006-04-19 13:21:36.000000000 -0700
+++ term.c 2006-04-19 13:21:54.000000000 -0700
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (NILP(terminal)?selected_frame:terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else

Ehm... appart from the whitespace I don't see a change. What am I
missing?

Sorry about that, I probably copied the file after changing it, here's the diff generated with C-x v =
(oh, bazaar is SLOOWWWW at generating diffs...)

--- orig/src/term.c
+++ mod/src/term.c
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else

[Károly Lőrentey]

Post by Dan Nicolaescu
--- orig/src/term.c
+++ mod/src/term.c
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else

This is still a no-op: get_tty_terminal already handles a nil argument
via get_terminal.

The problem is that `tty-display-color-p' does not throw an error on X
in CVS, while it does on multi-tty. I think the multi-tty behaviour
is more desirable, so I'm keeping it for the moment.

In patch-551, I fixed the immediate problem by changing
`vc-annotate-color-map' not to call `tty-display-color-p' when the
frame is not a tty frame. Note that `vc-annotate-color-map' needs
work for multi-tty support anyway, since it depends on the terminal
type during load time, which is a big no-no.

[Dan Nicolaescu]

Post by Károly Lőrentey

Post by Dan Nicolaescu
--- orig/src/term.c
+++ mod/src/term.c
@@ -1842,7 +1842,8 @@
(terminal)
Lisp_Object terminal;
{
- struct terminal *t = get_tty_terminal (terminal);
+ struct terminal *t = get_tty_terminal (NILP(terminal)
+ ?selected_frame:terminal);
if (!t)
return Qnil;
else

This is still a no-op: get_tty_terminal already handles a nil argument
via get_terminal.

It handles it by throwing an error...

Post by Károly Lőrentey
The problem is that `tty-display-color-p' does not throw an error on X
in CVS, while it does on multi-tty. I think the multi-tty behaviour
is more desirable, so I'm keeping it for the moment.

If you think that the behavior is desirable, the please change the
function name and doc string. Of the top of my head I don't know any
-p function in emacs that throws an error when called with some
arguments. Predicates usually return true or false, it's kind of
unexpected to throw an error...

Post by Károly Lőrentey
In patch-551, I fixed the immediate problem by changing
`vc-annotate-color-map' not to call `tty-display-color-p' when the
frame is not a tty frame. Note that `vc-annotate-color-map' needs
work for multi-tty support anyway, since it depends on the terminal
type during load time, which is a big no-no.

IMO that code should just build deffaces, that way it can take
advantage of the existing code to make faces look differently
depending on the display type.

[Károly Lőrentey]

Post by Dan Nicolaescu

Post by Károly Lőrentey
This is still a no-op: get_tty_terminal already handles a nil argument
via get_terminal.

It handles it by throwing an error...

No, it throws an error when the given terminal is not a tty.

Post by Dan Nicolaescu

Post by Károly Lőrentey
The problem is that `tty-display-color-p' does not throw an error on X
in CVS, while it does on multi-tty. I think the multi-tty behaviour
is more desirable, so I'm keeping it for the moment.

If you think that the behavior is desirable, the please change the
function name and doc string.

Ah, good catch. I wasn't aware that the nil-returning behaviour is
documented. I stopped reading at the first sentence, and assumed the
rest was just standard boilerplate.

Oh well. Patch-552 coming up. :-)

Post by Dan Nicolaescu
Of the top of my head I don't know any -p function in emacs that
throws an error when called with some arguments. Predicates usually
return true or false, it's kind of unexpected to throw an error...

Type predicates don't throw type errors, but other predicates
do sometimes require a specific type.

(file-exists-p 42)
==> Lisp error: (wrong-type-argument stringp 42)

Post by Dan Nicolaescu

Post by Károly Lőrentey
In patch-551, I fixed the immediate problem by changing
`vc-annotate-color-map' not to call `tty-display-color-p' when the
frame is not a tty frame. Note that `vc-annotate-color-map' needs
work for multi-tty support anyway, since it depends on the terminal
type during load time, which is a big no-no.

IMO that code should just build deffaces, that way it can take
advantage of the existing code to make faces look differently
depending on the display type.

Would you like to change it to do that?

[Dan Nicolaescu]

Post by Károly Lőrentey

Post by Dan Nicolaescu

Post by Károly Lőrentey
In patch-551, I fixed the immediate problem by changing
`vc-annotate-color-map' not to call `tty-display-color-p' when the
frame is not a tty frame. Note that `vc-annotate-color-map' needs
work for multi-tty support anyway, since it depends on the terminal
type during load time, which is a big no-no.

IMO that code should just build deffaces, that way it can take
advantage of the existing code to make faces look differently
depending on the display type.

Would you like to change it to do that?

I'll put it on my TODO, but it won't be anytime soon, my motivation
for this is pretty low as I only use X or xterm-256color terminals...