Skip to content

Do not use rb_f_notimplement for attr_set/attr_get #133

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 1 commit into from
Mar 6, 2026
+4 −4
Merged

Do not use rb_f_notimplement for attr_set/attr_get #133

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions ext/curses/curses.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2970,8 +2970,6 @@ window_attr_set(VALUE obj, VALUE attrs, VALUE pair)
GetWINDOW(obj, winp);
return (wattr_set(winp->window, NUM2UINT(attrs), NUM2INT(pair), NULL) == OK) ? Qtrue : Qfalse;
}
#else
#define window_attr_set rb_f_notimplement
#endif

/*
Expand Down Expand Up @@ -3003,8 +3001,6 @@ window_attr_get(VALUE obj)
return Qnil;
return rb_ary_new3(2, UINT2NUM(attrs), INT2NUM(pair));
}
#else
#define window_attr_get rb_f_notimplement
#endif

/*
Expand Down Expand Up @@ -5375,8 +5371,12 @@ 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);
#ifdef HAVE_WATTR_SET
rb_define_method(cWindow, "attr_set", window_attr_set, 2);
#endif
#ifdef HAVE_WATTR_GET
Comment on lines +5374 to +5377
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.

Guarding the rb_define_method calls changes behavior from “method exists but raises NotImplementedError” to “method is undefined and raises NoMethodError”. If this is intentional, please document the compatibility impact (e.g., in the method docs) so callers know to check respond_to?/method_defined? rather than rescuing NotImplementedError.

Suggested change
#ifdef HAVE_WATTR_SET
rb_define_method(cWindow, "attr_set", window_attr_set, 2);
#endif
#ifdef HAVE_WATTR_GET
#ifdef HAVE_WATTR_SET
/*
* Document-method: Curses::Window#attr_set
*
* This method is only defined when the underlying curses library
* provides wattr_set(3x) (i.e., when HAVE_WATTR_SET is true at
* compile time). On platforms without wattr_set, this Ruby method
* is not defined at all and calling it will raise NoMethodError.
*
* For portable code, check +window.respond_to?(:attr_set)+ (or
* +Curses::Window.method_defined?(:attr_set)+) before calling
* this method instead of rescuing NotImplementedError.
*/
rb_define_method(cWindow, "attr_set", window_attr_set, 2);
#endif
#ifdef HAVE_WATTR_GET
/*
* Document-method: Curses::Window#attr_get
*
* This method is only defined when the underlying curses library
* provides wattr_get(3x) (i.e., when HAVE_WATTR_GET is true at
* compile time). On platforms without wattr_get, this Ruby method
* is not defined at all and calling it will raise NoMethodError.
*
* For portable code, check +window.respond_to?(:attr_get)+ (or
* +Curses::Window.method_defined?(:attr_get)+) before calling
* this method instead of rescuing NotImplementedError.
*/

Copilot uses AI. Check for mistakes.
rb_define_method(cWindow, "attr_get", window_attr_get, 0);
#endif
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