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

cmake: adjust logic so variadic tparm is used on NetBSD curses. #6626

merged 1 commit into from Feb 20, 2020


Copy link

@coypoop coypoop commented Feb 18, 2020

  • Define TPARM_VARARGS macro before including the needed headers
    where tparm would be defined.
  • If we need TPARM_VARARGS define, define it in the config_cmake.h
  • don't define TPARM_SOLARIS_KLUDGE in the case we need to define
    TPARM_VARARGS, and simplify logic.

Copy link
Contributor Author

coypoop commented Feb 18, 2020

Sorry for the noise, I was testing if some part was actually necessary (it was) and accidentally made a commit like that.

Copy link

faho commented Feb 19, 2020

I'm not entirely sure what I'm looking at here. What does this improve or fix?

Copy link

faho commented Feb 19, 2020

Ah, okay. So this makes it so we don't use our awful "solaris kludge" on NetBSD, instead relying on the actual working tparm function?

That would be an improvement, yes.

- Define it before the headers so they can pick the variadic tparm
- We need a TPARM_VARARGS define, add it to config_cmake.h.
- Move & adjust comment - put it near the code, and mentiont that
NetBSD curses doesn't need the kludge.

Now variadic tparm is used on NetBSD instead of the Solaris kludge.
@zanchey zanchey added this to the fish 3.1.1 milestone Feb 20, 2020
@zanchey zanchey added the cmake label Feb 20, 2020
@zanchey zanchey modified the milestones: fish 3.1.1, fish 3.2.0 Feb 20, 2020
@faho faho merged commit 934f708 into fish-shell:master Feb 20, 2020
Copy link

faho commented Feb 20, 2020

Merged, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet

Successfully merging this pull request may close these issues.

None yet

3 participants