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

Add support for extended color functions in ncurses 6.1 #81163

Closed
websurfer5 mannequin opened this issue May 21, 2019 · 6 comments
Closed

Add support for extended color functions in ncurses 6.1 #81163

websurfer5 mannequin opened this issue May 21, 2019 · 6 comments
Labels
3.10 extension-modules type-feature

Comments

@websurfer5
Copy link
Mannequin

@websurfer5 websurfer5 mannequin commented May 21, 2019

BPO 36982
Nosy @ned-deily, @ambv, @serhiy-storchaka, @yan12125, @websurfer5, @hpjansson
PRs
  • #13534
  • #17536
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-08-11.04:41:08.873>
    created_at = <Date 2019-05-21.02:43:51.180>
    labels = ['extension-modules', 'type-feature', '3.10']
    title = 'Add support for extended color functions in ncurses 6.1'
    updated_at = <Date 2020-08-11.04:41:08.870>
    user = 'https://github.com/websurfer5'

    bugs.python.org fields:

    activity = <Date 2020-08-11.04:41:08.870>
    actor = 'ned.deily'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-08-11.04:41:08.873>
    closer = 'ned.deily'
    components = ['Extension Modules']
    creation = <Date 2019-05-21.02:43:51.180>
    creator = 'Jeffrey.Kintscher'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36982
    keywords = ['patch']
    message_count = 6.0
    messages = ['342974', '343336', '343338', '374527', '374791', '375163']
    nosy_count = 6.0
    nosy_names = ['ned.deily', 'lukasz.langa', 'serhiy.storchaka', 'yan12125', 'Jeffrey.Kintscher', 'hpj']
    pr_nums = ['13534', '17536']
    priority = 'critical'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue36982'
    versions = ['Python 3.10']

    @websurfer5
    Copy link
    Mannequin Author

    @websurfer5 websurfer5 mannequin commented May 21, 2019

    ncurses 6.1 adds extended color functions to support terminals with 256 colors (e.g. xterm-256color). The extended functions take color pair values that are signed integers because the existing functions only take signed 16-bit values.

    My goal with this issue is to transparently use the ncurses extended color functions when compiling with ncurses 6.1 or newer. This should be straightforward and transparent to curses module users because the short int restrictions are in the ncurses library and not in the curses module API.

    This will fix the problems observed in issue bpo-36630 but is broader, which is why I created a separete issue. I will work on this and post a PR whit it is ready.

    @websurfer5 websurfer5 mannequin added 3.9 stdlib 3.8 labels May 21, 2019
    @websurfer5
    Copy link
    Mannequin Author

    @websurfer5 websurfer5 mannequin commented May 24, 2019

    A new function called curses.has_extended_color_support() will indicate whether the linked ncurses library provides extended color support. It returns true if curses.h defines NCURSES_EXT_COLORS and NCURSES_EXT_FUNCS, indicating that the extended color functions are available. This seems more useful to developers than using an indirect method like trying to set a color-pair greater than 0x7fff and checking for an exception to indicate lack of support.

    At first glance, has_extended_color() seems like a better name because it is similar to has_colors(), but the two functions have very different semantics that could confuse developers. has_extended_color_support() indicates available functionality in the underlying ncurses library that is set when the interpreter is compiled and linked, while has_curses() indicates available functionality of the current terminal at run-time.

    @websurfer5 websurfer5 mannequin removed the 3.9 label May 24, 2019
    @websurfer5
    Copy link
    Mannequin Author

    @websurfer5 websurfer5 mannequin commented May 24, 2019

    Corrected typos in msg343336 for clarity:

    At first glance, has_extended_colors() seems like a better name because it is similar to has_colors(), but the two functions have very different semantics that could confuse developers. has_extended_color_support() indicates available functionality in the underlying ncurses library that is set when the interpreter is compiled and linked, while has_colors() indicates available functionality of the current terminal at run-time. The name has_extended_colors() would imply that it indicates run-time terminal functionality and not static library functionality.

    @websurfer5 websurfer5 mannequin added type-feature extension-modules and removed stdlib labels May 24, 2019
    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Jul 28, 2020

    PR 17536 was based on the original PR 13534 and has now gone through a couple of rounds of code review. Other than a missing doc change, everything in PR 13534 is covered (and updated) in PR 17536 so I've closed the original PR. Other than adding the doc change and a final core developer review of the last requested changes, this *should* be good to go.

    We really need to get this merged since, without it, Python builds fail with the newer versions of ncurses now in most distributions. Once it is merged into master for 3.10, Łukasz can provide direction about whether and when it should be backported to 3.9 and/or 3.8.

    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Aug 4, 2020

    New changeset da4e09f by Hans Petter Jansson in branch 'master':
    bpo-36982: Add support for extended color functions in ncurses 6.1 (GH-17536)
    da4e09f

    @ned-deily
    Copy link
    Member

    @ned-deily ned-deily commented Aug 11, 2020

    We really need to get this merged since, without it, Python builds fail with the newer versions of ncurses now in most distributions.

    That is a bit of an overstatment on my part. What is true is that test_curses fails on 3.9 and 3.8 when run with ncurses 6.1+. It is also true that the relevant parts of test_curses are often skipped in CI and buildbot runs as described in bpo-12669 so a test failure is often not seen.

    Łukasz can provide direction about whether and when it should be backported to 3.9 and/or 3.8.

    We discussed this and decided that a backport to 3.8 was out-of-scope but a backport in time for 3.9.0 might be OK. Alas, I had forgotten that other changes have gone into master for 3.10 prior to this merge which complicates a backport to 3.9, enough that we shouldn't be trying to throw this in just prior to 3.9.0rc1.

    Thanks everyone for the work on this!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 extension-modules type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant