Skip to content
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.

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

bpo-37738: Fix curses addch(str, color_pair) #15071

Merged
merged 1 commit into from
Aug 14, 2019
Merged

bpo-37738: Fix curses addch(str, color_pair) #15071

merged 1 commit into from
Aug 14, 2019

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Aug 1, 2019

Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.

https://bugs.python.org/issue37738

Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
@@ -606,7 +618,7 @@ _curses_window_addch_impl(PyCursesWindowObject *self, int group_left_1,
if (type == 2) {
funcname = "add_wch";
wstr[1] = L'\0';
setcchar(&wcval, wstr, attr, 0, NULL);
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if we are supposed to remove the color pair from attr passed to setcchar()?

Copy link

Choose a reason for hiding this comment

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

According to http://pubs.opengroup.org/onlinepubs/7908799/xcurses/setcchar.html, setcchar() takes an attr_t as the third argument.

According to http://pubs.opengroup.org/onlinepubs/7908799/xcurses/curses.h.html, attr_t is a bit set that can have these constants ORed:

WA_ALTCHARSET Alternate character set
WA_BLINK Blinking
WA_BOLD Extra bright or bold
WA_DIM Half bright
WA_HORIZONTAL Horizontal highlight
WA_INVIS Invisible
WA_LEFT Left highlight
WA_LOW Low highlight
WA_PROTECT Protected
WA_REVERSE Reverse video
WA_RIGHT Right highlight
WA_STANDOUT Best highlighting mode of the terminal
WA_TOP Top highlight
WA_UNDERLINE Underlining
WA_VERTICAL Vertical highlight

Meanwhile, waddch() takes a chtype, which contains a character, a color pair, and a bunch of attributes from this set:

A_ALTCHARSET Alternate character set
A_BLINK Blinking
A_BOLD Extra bright or bold
A_DIM Half bright
A_INVIS Invisible
A_PROTECT Protected
A_REVERSE Reverse video
A_STANDOUT Best highlighting mode of the terminal
A_UNDERLINE Underlining

There's no guarantee that, for example, A_BOLD == WA_BOLD. I'm afraid a more elaborate translation may be necessary here to be 100% correct.

Copy link

@mgedmin mgedmin Aug 1, 2019

Choose a reason for hiding this comment

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

curses.h on my system has

/*
 * X/Open attributes.  In the ncurses implementation, they are identical to the
 * A_ attributes.
 */
#define WA_ATTRIBUTES	A_ATTRIBUTES
#define WA_NORMAL	A_NORMAL
#define WA_STANDOUT	A_STANDOUT
#define WA_UNDERLINE	A_UNDERLINE
#define WA_REVERSE	A_REVERSE
#define WA_BLINK	A_BLINK
#define WA_DIM		A_DIM
#define WA_BOLD		A_BOLD
#define WA_ALTCHARSET	A_ALTCHARSET
#define WA_INVIS	A_INVIS
#define WA_PROTECT	A_PROTECT
#define WA_HORIZONTAL	A_HORIZONTAL
#define WA_LEFT		A_LEFT
#define WA_LOW		A_LOW
#define WA_RIGHT	A_RIGHT
#define WA_TOP		A_TOP
#define WA_VERTICAL	A_VERTICAL

#if 1
#define WA_ITALIC	A_ITALIC	/* ncurses extension */
#endif

so maybe we're fine? As long as we know we're using ncurses, and not some other curses library?

Copy link

Choose a reason for hiding this comment

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

I googled around to see whether there are curses implementations that use different bit values for these constants, and yes:

change ncurses-examples to use attr_t vs chtype to follow X/Open documentation more closely since Solaris xpg4-curses uses different values for WA_xxx vs A_xxx that rely on attr_t being an unsigned short. Tru64 aka OSF1, HPUX, AIX did as ncurses does, equating the two sets.

--- from https://www.gnu.org/software/ncurses/

@vstinner
Copy link
Member Author

vstinner commented Aug 1, 2019

Python not really supports Solaris: last time I checked, the compilation of the _curses module failed with a compiler error anyway.

@vstinner vstinner merged commit 077af8c into python:master Aug 14, 2019
@vstinner vstinner deleted the curses_color_pair branch August 14, 2019 10:31
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-15272 is a backport of this pull request to the 3.8 branch.

@miss-islington
Copy link
Contributor

Sorry, @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 077af8c2c93dd71086e2c5e5ff1e634b6da8f214 3.7

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 14, 2019
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
(cherry picked from commit 077af8c)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
@bedevere-bot
Copy link

GH-15273 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request Aug 14, 2019
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
(cherry picked from commit 077af8c)

Co-authored-by: Victor Stinner <vstinner@redhat.com>
vstinner added a commit that referenced this pull request Aug 14, 2019
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.

(cherry picked from commit 077af8c)
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
Fix the implementation of curses addch(str, color_pair): pass the
color pair to setcchar(), instead of always passing 0 as the color
pair.
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.

5 participants