Skip to content

Add Window#attr_set and Window#attr_get #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shugo merged 2 commits into from
Mar 6, 2026
+60 −0
Merged

Add Window#attr_set and Window#attr_get #131

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d4188e
Add Window#attr_set and Window#attr_get
shugo Mar 6, 2026
fdf92d1
Fix type conversion macros in Window#attr_set and Window#attr_get
shugo Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions ext/curses/curses.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2951,6 +2951,64 @@ window_attrset(VALUE obj, VALUE attrs)
#endif
}

/*
* Document-method: Curses::Window.attr_set
* call-seq: attr_set(attrs, pair)
*
* Sets the current attributes and color pair of the given window.
* Unlike Curses::Window.attrset, this method accepts an extended color
* pair number (> 255) when the ncurses extended colors API is available.
*
* Returns +true+ on success, +false+ on failure.
*
* see also system manual curs_attr(3)
*/
#ifdef HAVE_WATTR_SET
static VALUE
window_attr_set(VALUE obj, VALUE attrs, VALUE pair)
{
struct windata *winp;

GetWINDOW(obj, winp);
return (wattr_set(winp->window, NUM2ULONG(attrs), NUM2INT(pair), NULL) == OK) ? Qtrue : Qfalse;
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The NUM2ULONG macro converts the Ruby Integer to unsigned long, but wattr_set's second parameter expects attr_t, which in ncurses is unsigned int (32-bit). On 64-bit platforms, unsigned long is 64-bit, so this causes an implicit narrowing conversion warning and potentially incorrect behavior if the value is wider than 32 bits. The correct conversion macro here should be NUM2UINT to match attr_t (which is an unsigned int-sized type). Compare to how the existing window_color_set at line 2811 simply passes attrs as-is after reading it via wattr_get.

Suggested change
return (wattr_set(winp->window, NUM2ULONG(attrs), NUM2INT(pair), NULL) == OK) ? Qtrue : Qfalse;
return (wattr_set(winp->window, NUM2UINT(attrs), NUM2INT(pair), NULL) == OK) ? Qtrue : Qfalse;

Copilot uses AI. Check for mistakes.
}
#else
#define window_attr_set rb_f_notimplement
#endif

/*
* Document-method: Curses::Window.attr_get
* call-seq: attr_get => [attrs, pair]
*
* Returns a 2-element Array of the current attributes and color pair
* of the given window. The color pair number may exceed 255 when the
* ncurses extended colors API is available.
*
* Returns +nil+ on failure.
*
* see also system manual curs_attr(3)
*/
#ifdef HAVE_WATTR_GET
static VALUE
window_attr_get(VALUE obj)
{
struct windata *winp;
attr_t attrs;
#ifdef NCURSES_PAIRS_T
NCURSES_PAIRS_T pair;
#else
short pair;
#endif

GetWINDOW(obj, winp);
if (wattr_get(winp->window, &attrs, &pair, NULL) == ERR)
return Qnil;
return rb_ary_new3(2, ULONG2NUM(attrs), INT2FIX(pair));
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ULONG2NUM(attrs) converts attrs (of type attr_t, which is unsigned int) to a Ruby Integer via unsigned long. This implicit widening from unsigned int to unsigned long is harmless on most platforms, but it is semantically imprecise. UINT2NUM(attrs) is the correct macro for an unsigned int-sized type. Additionally, INT2FIX(pair) is used for the pair value, but INT2FIX is only valid for values in the Fixnum range. Since NCURSES_PAIRS_T is an int supporting extended color pairs (which can be a large positive number), INT2NUM(pair) is the safer choice here. Notably, INT2FIX is used rather than INT2NUM throughout other parts of the codebase (e.g., line 1459), but the intent of attr_get is specifically to support large extended pair numbers, so losing that via an unsafe INT2FIX would be a bug if the pair number is large.

Suggested change
return rb_ary_new3(2, ULONG2NUM(attrs), INT2FIX(pair));
return rb_ary_new3(2, UINT2NUM(attrs), INT2NUM(pair));

Copilot uses AI. Check for mistakes.
}
#else
#define window_attr_get rb_f_notimplement
#endif

/*
* Document-method: Curses::Window.bkgdset
* call-seq: bkgdset(ch)
Expand Down Expand Up @@ -5319,6 +5377,8 @@ Init_curses(void)
rb_define_method(cWindow, "attroff", window_attroff, 1);
rb_define_method(cWindow, "attron", window_attron, 1);
rb_define_method(cWindow, "attrset", window_attrset, 1);
rb_define_method(cWindow, "attr_set", window_attr_set, 2);
rb_define_method(cWindow, "attr_get", window_attr_get, 0);
rb_define_method(cWindow, "bkgdset", window_bkgdset, 1);
rb_define_method(cWindow, "bkgd", window_bkgd, 1);
rb_define_method(cWindow, "getbkgd", window_getbkgd, 0);
Expand Down