-
Notifications
You must be signed in to change notification settings - Fork 74
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 Python 3 #2269
Add support for Python 3 #2269
Conversation
What still needs to be done before support for Python 3 is added to KVIrc? |
iirc this was completely functional, but I left it here because I wanted to implement a 2/3 toggle in cmake. Not necessary anymore it seems, right? |
I guess not |
Can someone fix the cmake rules so it will only detect python if it's 3.x? The way it is now, I believe it will detect both 2 and 3 but it won't work with 2. |
craps out on windows 32 bit |
src/modules/python/libkvipython.cpp
Outdated
static bool python_kvs_fnc_version(KviKvsModuleFunctionCall * c) | ||
{ | ||
#ifdef COMPILE_PYTHON_SUPPORT | ||
c->returnValue()->setInteger(PY_MAJOR_VERSION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe value containing also at least the minor version of Python would be more useful.
Python headers, beside PY_MAJOR_VERSION
, PY_MINOR_VERSION
, PY_MICRO_VERSION
, PY_RELEASE_LEVEL
, PY_RELEASE_SERIAL
, already provide PY_VERSION_HEX
:
/* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2.
Use this for numeric comparisons, e.g. #if PY_VERSION_HEX >= ... */
#define PY_VERSION_HEX ((PY_MAJOR_VERSION << 24) | \
(PY_MINOR_VERSION << 16) | \
(PY_MICRO_VERSION << 8) | \
(PY_RELEASE_LEVEL << 4) | \
(PY_RELEASE_SERIAL << 0))
src/modules/python/libkvipython.cpp
Outdated
static bool python_module_init(KviModule * m) | ||
{ | ||
// register the command anyway | ||
KVSM_REGISTER_SIMPLE_COMMAND(m, "begin", python_kvs_cmd_begin); | ||
KVSM_REGISTER_SIMPLE_COMMAND(m, "destroy", python_kvs_cmd_destroy); | ||
|
||
KVSM_REGISTER_FUNCTION(m, "isAvailable", python_kvs_fnc_isAvailable); | ||
KVSM_REGISTER_FUNCTION(m, "version", python_kvs_fnc_version); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a potentially useful information to expose to consumers of this API.
Perhaps it should be called python_version
instead of version
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I withdraw my suggestions.
Something like the following already works:
python.begin
import sys
kvirc.setLocal("python_major_version", str(sys.version_info[0]))
kvirc.setLocal("python_minor_version", str(sys.version_info[1]))
kvirc.setLocal("python_micro_version", str(sys.version_info[2]))
python.end
echo %python_major_version
echo %python_minor_version
echo %python_micro_version
@wodim yo sup? |
Still the same error, seemingly. |
oh the cmake rule, right? |
I haven't got a clue, honestly. |
c:\cygwin\bin\bash : 1 [main] bash (3052) c:\cygwin\bin\bash.exe: *** fatal error - cygheap base mismatch detected - 0x1180408/0xFB0408.
0f41a4c
to
c72a5ac
Compare
@wodim I haven't tested runtime, but the windows build at least doesn't confuse x86 and x64 libraries anymore. |
Seems to work fine. If it's ready (seeing that you added another commit) then merge. Thank you for your help. |
The last commit was to fix the debug build - it tried to link to both python38.lib and python38_d.lib |
Not for merging yet