Skip to content

Add Window#attr_set and Window#attr_get#131

Merged
shugo merged 2 commits intomasterfrom
feature/wattr
Mar 6, 2026
Merged

Add Window#attr_set and Window#attr_get#131
shugo merged 2 commits intomasterfrom
feature/wattr

Conversation

@shugo
Copy link
Member

@shugo shugo commented Mar 6, 2026

These wrap wattr_set() and wattr_get() respectively, exposing the full extended color pair number (> 255) that attrset/color_set cannot represent. attr_set(attrs, pair) sets both attributes and color pair; attr_get returns [attrs, pair] as a 2-element Array.

Both are guarded by HAVE_WATTR_SET / HAVE_WATTR_GET and fall back to rb_f_notimplement when unavailable.

These wrap wattr_set() and wattr_get() respectively, exposing the
full extended color pair number (> 255) that attrset/color_set cannot
represent. attr_set(attrs, pair) sets both attributes and color pair;
attr_get returns [attrs, pair] as a 2-element Array.

Both are guarded by HAVE_WATTR_SET / HAVE_WATTR_GET and fall back to
rb_f_notimplement when unavailable.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR exposes the ncurses extended attribute API to Ruby by wrapping wattr_set() and wattr_get() as Window#attr_set and Window#attr_get. Unlike the existing attrset/color_set, these methods operate on a separate color pair integer (rather than embedding the pair inside a chtype), allowing pair numbers greater than 255. Both methods are guarded with HAVE_WATTR_SET/HAVE_WATTR_GET and fall back to rb_f_notimplement when unavailable.

Changes:

  • Adds window_attr_set C function implementing Window#attr_set(attrs, pair) via wattr_set()
  • Adds window_attr_get C function implementing Window#attr_get via wattr_get(), returning [attrs, pair]
  • Registers both methods on cWindow via rb_define_method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.
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.
- attr_set: NUM2ULONG -> NUM2UINT to match attr_t (unsigned int)
- attr_get: ULONG2NUM -> UINT2NUM for attr_t; INT2FIX -> INT2NUM for
  pair number to safely handle extended pair values on all platforms

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@shugo shugo merged commit 9b3c95b into master Mar 6, 2026
8 checks passed
@shugo shugo deleted the feature/wattr branch March 6, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants