Skip to content

Commit 74eb758

Browse files
shugoclaude
andcommitted
Address Copilot review comments
- curses_color_content, curses_pair_content: check return value of extended_color_content/color_content/extended_pair_content/pair_content and return nil on ERR instead of returning uninitialized values - curses_reset_color_pairs: add curses_stdscr() call for consistency with other color APIs - window_color_set: use wattr_get to preserve existing window attributes when calling wattr_set; use NCURSES_PAIRS_T for the pair argument to match the ncurses header type in both standard and extended modes; fall back to wattr_set without attr preservation if wattr_get is unavailable, and to wcolor_set if wattr_set is also unavailable - extconf.rb: add have_func check for wattr_get - sample/colors.rb: skip pair 0 (cannot be redefined); use i%colors instead of i%256 to handle terminals with fewer than 256 colors Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 0153604 commit 74eb758

3 files changed

Lines changed: 22 additions & 7 deletions

File tree

ext/curses/curses.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,13 +1410,15 @@ curses_color_content(VALUE obj, VALUE color)
14101410
#ifdef HAVE_EXTENDED_COLOR_CONTENT
14111411
{
14121412
int r, g, b;
1413-
extended_color_content(NUM2INT(color), &r, &g, &b);
1413+
if (extended_color_content(NUM2INT(color), &r, &g, &b) == ERR)
1414+
return Qnil;
14141415
return rb_ary_new3(3, INT2FIX(r), INT2FIX(g), INT2FIX(b));
14151416
}
14161417
#else
14171418
{
14181419
short r, g, b;
1419-
color_content(NUM2INT(color), &r, &g, &b);
1420+
if (color_content(NUM2INT(color), &r, &g, &b) == ERR)
1421+
return Qnil;
14201422
return rb_ary_new3(3, INT2FIX(r), INT2FIX(g), INT2FIX(b));
14211423
}
14221424
#endif
@@ -1452,13 +1454,15 @@ curses_pair_content(VALUE obj, VALUE pair)
14521454
#ifdef HAVE_EXTENDED_PAIR_CONTENT
14531455
{
14541456
int f, b;
1455-
extended_pair_content(NUM2INT(pair), &f, &b);
1457+
if (extended_pair_content(NUM2INT(pair), &f, &b) == ERR)
1458+
return Qnil;
14561459
return rb_ary_new3(2, INT2FIX(f), INT2FIX(b));
14571460
}
14581461
#else
14591462
{
14601463
short f, b;
1461-
pair_content(NUM2INT(pair), &f, &b);
1464+
if (pair_content(NUM2INT(pair), &f, &b) == ERR)
1465+
return Qnil;
14621466
return rb_ary_new3(2, INT2FIX(f), INT2FIX(b));
14631467
}
14641468
#endif
@@ -1520,6 +1524,7 @@ curses_support_extended_colors(VALUE obj)
15201524
static VALUE
15211525
curses_reset_color_pairs(VALUE obj)
15221526
{
1527+
curses_stdscr();
15231528
reset_color_pairs();
15241529
return Qnil;
15251530
}
@@ -2792,7 +2797,16 @@ window_color_set(VALUE obj, VALUE col)
27922797
struct windata *winp;
27932798

27942799
GetWINDOW(obj, winp);
2795-
#ifdef HAVE_WATTR_SET
2800+
#if defined(HAVE_WATTR_SET) && defined(HAVE_WATTR_GET)
2801+
/* Use wattr_set to support pair numbers > 255; preserve existing attrs. */
2802+
{
2803+
attr_t attrs;
2804+
NCURSES_PAIRS_T current_pair;
2805+
if (wattr_get(winp->window, &attrs, &current_pair, NULL) == ERR)
2806+
return Qfalse;
2807+
return (wattr_set(winp->window, attrs, NUM2INT(col), NULL) == OK) ? Qtrue : Qfalse;
2808+
}
2809+
#elif defined(HAVE_WATTR_SET)
27962810
return (wattr_set(winp->window, 0, NUM2INT(col), NULL) == OK) ? Qtrue : Qfalse;
27972811
#else
27982812
return (wcolor_set(winp->window, NUM2INT(col), NULL) == OK) ? Qtrue : Qfalse;

ext/curses/extconf.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def exec_command(cmd)
121121
newpad unget_wch get_wch wget_wch PDC_get_key_modifiers
122122
chgat wchgat newterm
123123
init_extended_color init_extended_pair extended_color_content
124-
extended_pair_content reset_color_pairs wattr_set)
124+
extended_pair_content reset_color_pairs wattr_set wattr_get)
125125
have_func(f) || (have_macro(f, curses) && $defs.push(format("-DHAVE_%s", f.upcase)))
126126
end
127127
convertible_int('chtype', [["#undef MOUSE_MOVED\n"]]+curses) or abort

sample/colors.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
addstr extended ? " (extended).\n" : ".\n"
2121

2222
(extended ? [512, color_pairs].min : colors).times { |i|
23-
Curses.init_pair(i, i%256, i>=256 ? 8 : 0)
23+
next if i == 0
24+
Curses.init_pair(i, i % colors, (i / colors) % colors)
2425
if extended
2526
# color_pair() encodes into chtype and can't handle pairs > 255;
2627
# use color_set on stdscr instead, which calls wattr_set internally.

0 commit comments

Comments
 (0)