col_inverse_bg=0 && col_inverse_fg=7

[Dennis Preiser]

While playing with colors I found the following issue which happens in
the USE_CURSES && HAVE_USE_DEFAULT_COLORS case:

It's not possible to set col_inverse_bg=0 (black) and col_inverse_fg=7
(white). tin uses the (terminal) default fg an bg colors instead and
thus the inverse bar gets 'invisible'. Steps to reproduce:

open 'xterm -fg yellow -bg blue', start tin
'M'enu
Draw -> instead of highlighted bar : OFF
Use inverse video for page headers : ON
Standard foreground color : Default
Standard background color : Default
Color for inverse text (foreground) : White
Color for inverse text (background) : Black

-> the inverse bar is 'invisible'.

What happens: During startup use_default_colors() (in
tcurses.c:InitScreen()) assign terminals fg and bg to color pair 0.
Later on, color.c:set_colors() checks if the given fcolor and bcolor
matches COLOR_WHITE and COLOR_BLACK. If so, color pair 0 is used.
Unfortunately, for HAVE_USE_DEFAULT_COLORS, color pair 0 may hold other
color informations. For col_inverse_bg=0 && col_inverse_fg=7 this
results in an inverse bar with standard fg/bg -> the bar is 'invisible'.

The following fixes this for me, alas I'm not an curses expert so I
might be wrong:

diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-12 18:01:32.000000000 +0100
@@ -110,8 +110,12 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;

+# ifdef HAVE_USE_DEFAULT_COLORS
+ if (fcolor != default_fcol || bcolor != default_bcol) {
+# else
/* curses assumes white/black */
if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+# endif /* HAVE_USE_DEFAULT_COLORS */
struct LIST *p;
t_bool found = FALSE;

default_fcol and default_bcol are either white/black or, if
use_default_colors() succeeds, -1/-1 (default colors from terminal).

Dennis

[Thomas Dickey]

Post by Dennis Preiser
While playing with colors I found the following issue which happens in
It's not possible to set col_inverse_bg=0 (black) and col_inverse_fg=7
(white). tin uses the (terminal) default fg an bg colors instead and
open 'xterm -fg yellow -bg blue', start tin
'M'enu
Draw -> instead of highlighted bar : OFF
Use inverse video for page headers : ON
Standard foreground color : Default
Standard background color : Default
Color for inverse text (foreground) : White
Color for inverse text (background) : Black
-> the inverse bar is 'invisible'.
What happens: During startup use_default_colors() (in
tcurses.c:InitScreen()) assign terminals fg and bg to color pair 0.
Later on, color.c:set_colors() checks if the given fcolor and bcolor
matches COLOR_WHITE and COLOR_BLACK. If so, color pair 0 is used.
Unfortunately, for HAVE_USE_DEFAULT_COLORS, color pair 0 may hold other
color informations. For col_inverse_bg=0 && col_inverse_fg=7 this
results in an inverse bar with standard fg/bg -> the bar is 'invisible'.
The following fixes this for me, alas I'm not an curses expert so I
diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-12 18:01:32.000000000 +0100
@@ -110,8 +110,12 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;
+# ifdef HAVE_USE_DEFAULT_COLORS
+ if (fcolor != default_fcol || bcolor != default_bcol) {
+# else
/* curses assumes white/black */
if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+# endif /* HAVE_USE_DEFAULT_COLORS */
struct LIST *p;
t_bool found = FALSE;
default_fcol and default_bcol are either white/black or, if
use_default_colors() succeeds, -1/-1 (default colors from terminal).

something like that (I'd be inclined to fold the two if-lines together
by defining DEFAULT_FCOL and DEFAULT_BCOL somewhere, and using those
for cases like this).

[Dennis Preiser]

Post by Thomas Dickey

Post by Dennis Preiser
diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-12 18:01:32.000000000 +0100
@@ -110,8 +110,12 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;
+# ifdef HAVE_USE_DEFAULT_COLORS
+ if (fcolor != default_fcol || bcolor != default_bcol) {
+# else
/* curses assumes white/black */
if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+# endif /* HAVE_USE_DEFAULT_COLORS */
struct LIST *p;
t_bool found = FALSE;
default_fcol and default_bcol are either white/black or, if
use_default_colors() succeeds, -1/-1 (default colors from terminal).

something like that (I'd be inclined to fold the two if-lines together
by defining DEFAULT_FCOL and DEFAULT_BCOL somewhere, and using those
for cases like this).

Thanks. Initially, default_fcol is 7 and default_bcol is 0. This changes
to -1/-1 only when use_default_colors() succeeds. So the diff shrinks
to:

diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-13 14:08:08.000000000 +0100
@@ -110,8 +110,7 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;

- /* curses assumes white/black */
- if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+ if (fcolor != default_fcol || bcolor != default_bcol) {
struct LIST *p;
t_bool found = FALSE;


Can we assume that curses always define COLOR_WHITE 7 and COLOR_BLACK 0?

If that's not guaranteed, we additionally need the following change:

diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-13 14:14:24.000000000 +0100
@@ -53,10 +53,9 @@

# define MIN_COLOR -1 /* -1 is default, otherwise 0-7 or 0-15 */

-int default_fcol = 7;
-int default_bcol = 0;
-
# ifdef USE_CURSES
+ int default_fcol = COLOR_WHITE;
+ int default_bcol = COLOR_BLACK;
static int current_fcol = 7;
static struct LIST {
struct LIST *link;
@@ -64,6 +63,9 @@ int default_bcol = 0;
int fg;
int bg;
} *list;
+# else
+ int default_fcol = 7;
+ int default_bcol = 0;
# endif /* USE_CURSES */
static int current_bcol = 0;


Dennis

[Thomas Dickey]

Post by Dennis Preiser

Post by Thomas Dickey

Post by Dennis Preiser
diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-12 18:01:32.000000000 +0100
@@ -110,8 +110,12 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;
+# ifdef HAVE_USE_DEFAULT_COLORS
+ if (fcolor != default_fcol || bcolor != default_bcol) {
+# else
/* curses assumes white/black */
if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+# endif /* HAVE_USE_DEFAULT_COLORS */
struct LIST *p;
t_bool found = FALSE;
default_fcol and default_bcol are either white/black or, if
use_default_colors() succeeds, -1/-1 (default colors from terminal).

something like that (I'd be inclined to fold the two if-lines together
by defining DEFAULT_FCOL and DEFAULT_BCOL somewhere, and using those
for cases like this).

Thanks. Initially, default_fcol is 7 and default_bcol is 0. This changes
to -1/-1 only when use_default_colors() succeeds. So the diff shrinks
diff -urp tin-1.9.6/src/color.c tin-1.9.6_r2/src/color.c
--- tin-1.9.6/src/color.c 2010-05-07 16:00:53.000000000 +0200
+++ tin-1.9.6_r2/src/color.c 2010-11-13 14:08:08.000000000 +0100
@@ -110,8 +110,7 @@ set_colors(
if (bcolor > 0)
bcolor %= COLORS;
- /* curses assumes white/black */
- if (fcolor != COLOR_WHITE || bcolor != COLOR_BLACK) {
+ if (fcolor != default_fcol || bcolor != default_bcol) {
struct LIST *p;
t_bool found = FALSE;
Can we assume that curses always define COLOR_WHITE 7 and COLOR_BLACK 0?

yes - works everywhere that I'm aware of (we're not supporting non-color
curses).