Skip to content

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

Merged
vstinner merged 1 commit into
python:masterfrom
vstinner:curses_color_pair
Aug 14, 2019
Merged

bpo-37738: Fix curses addch(str, color_pair)#15071
vstinner merged 1 commit into
python:masterfrom
vstinner:curses_color_pair

Conversation

@vstinner

@vstinner vstinner commented Aug 1, 2019

Copy link
Copy Markdown
Member

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.
Comment thread Modules/_cursesmodule.c
if (type == 2) {
funcname = "add_wch";
wstr[1] = L'\0';
setcchar(&wcval, wstr, attr, 0, NULL);

Copy link
Copy Markdown
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
Copy Markdown

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.

@mgedmin mgedmin Aug 1, 2019

Copy link
Copy Markdown

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
Copy Markdown

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

vstinner commented Aug 1, 2019

Copy link
Copy Markdown
Member Author

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
Copy Markdown
Contributor

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

@bedevere-bot

Copy link
Copy Markdown

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

@miss-islington

Copy link
Copy Markdown
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
Copy Markdown

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