Python3 compatibility

[jakykong]

Some newer Vim versions (specifically, Vim included in Debian Sid as of writing) do not include has('python') but do include has('python3'). This pull request adds Python3 compatibility without (I think) breaking Python compatibility.

I can't test with plain Python because the version of Vim on my system is affected. However I tested as much as I can with Python3, and all functionality appears to be working that I tried.

In summary, the changes are to change all :python commands to be :executed with the relevant command in s:py_cmd, and to add a Python3 version of Swank.py (Swank3.py) which has a few small tweaks to bring it forward. I kept modifications as minimal as possible to ease maintenance between Swank.py and Swank3.py, which does mean some remnant code (e.g., for unicode) is present which is no longer relevant for Python3.

Final note: I did NOT update any dates or maintainer names. It seems likely that you (the maintainer) would want to do that, since there is no obvious place for changelog entries in individual files. Please let me know, I'll be more than happy to go do that.

[kovisoft]

I think it's not a good idea to duplicate the whole swank.py just because we have some (actually 10 lines of) differences. Normally the same .py file should work both in python 2.x and in python 3.x (well, maybe we need to add some conditionals or tweaks). And I'm not sure we need most of the differences at all. I think python 2.x is also able to handle the parentheses in print (...) when there is only one argument, and we can maybe concatenate multiple arguments to a single one at other places. I still need to check the unicode stuff, though.

If we have one single swank.py file, then we don't need to do any changes to the loading of the file. But yes, I think we need to handle the python vs. python3 and pyfile vs. py3file cases.

[jakykong]

Well, I will say I should have checked that earlier; a bit of mindlessness on my part, and I apologize if I wasted any of your time on that. (My error in deciding to submit a pull request at 2am instead of sleeping on it and rechecking.)

I just checked print() syntax in Python 2; it looks like the syntax from Python 3 is available with {from future import print_function}.

Regarding unicode, it appears that Python 2.4+ support the .decode() method that is required in Python 3, so that's also in the common subset here. (c.f., first sentence of this e-mail.)

Let me fix these up, I'll push changes shortly.

On 11/30/2016 12:36 PM, Tamas Kovacs wrote:

I think it's not a good idea to duplicate the whole |swank.py| just because we have some (actually 10 lines of) differences. Normally the same |.py| file should work both in python 2.x and in python 3.x (well, maybe we need to add some conditionals or tweaks). And I'm not sure we need most of the differences at all. I think python 2.x is also able to handle the parentheses in |print (...)| when there is only one argument, and we can maybe concatenate multiple arguments to a single one at other places. I still need to check the unicode stuff, though.

If we have one single |swank.py| file, then we don't need to do any changes to the loading of the file. But yes, I think we need to handle the |python| vs. |python3| and |pyfile| vs. |py3file| cases.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kovisoft/slimv/pull/46#issuecomment-263987659, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ1RLiA8UmKttlGL9VKp3U24M9WzgLxJks5rDd43gaJpZM4K_9cA.

[jakykong]

Swank3.py has been removed, the relevant changes merged into Swank.py along with the future import.

I've tested that Swank.py loads with Python2.7, but because my system doesn't has('python'), I still can't test the runtime there. However, the runtime for Python 3 tests successfully on my end still.

[kovisoft]

No, you don't need to apologize at all, I really appreciate that you're trying to help making slimv better. The modified version of your pull request looks much better and cleaner to me, thank you for your efforts. I still have some notes that I'd like to discuss:

(1) I feel the error message too strong at checking for has( python* ). I agree that there's little use of slimv without python support, but the current version does not finish the script just because python is not found. It is currently checked when connecting to the swank server. So I suggest that we have python/pyfile mapped by default, if no python3 found:

if has( 'python3' )
    let s:py_cmd = 'python3 ' "note space
    let s:pyfile_cmd = 'py3file '
else
    let s:py_cmd = 'python '  "note space
    let s:pyfile_cmd = 'pyfile '
endif

(2) The use of encode('utf-8') and decode('utf-8') is not clear for me. If the text being sent contains unicode characters then text.encode('utf-8') gives a UnicodeDecodeError, at least in python 2.x. What is the problem that you are trying to solve here? What is the difference in python 2.x and 3 regarding unicode?

(3) Another question: what does file= stand for in the line: print(':open-dedicated-output-stream result:', file=output_port)? The output_port is just a number that we want to print here.

Again, thanks for all your efforts!

[kovisoft]

Sorry, I'm probably wrong in my item (2): the text I was talking about may be a non-unicode text. Anyway, I was playing with encode/decode a little, and I got this in python 2.x:

>>> 'ééé'.encode("utf-8")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128)

However I got this in python 3.x:

>>> 'ééé'.encode("utf-8")
b'\xc3\xa9\xc3\xa9\xc3\xa9'

So I think all encode/decode calls should be in an if sys.version_info[0]>2: or similar to ensure that the old logic is used for python 2.x. What do you think?

[jakykong]

Regarding item (1), defaulting to "python" is probably fine, although one potential issue that this brings to mind which I hadn't thought of before is that has('python') and has('python3') can be true even if the relevant python isn't available (for example, on my corporate laptop, has('python') is available, but only python 3.5 is actually installed, so any attempt to call :python will fail) -- it might be a good idea to have a global, g:slimv_python_version or something, that forces the issue, so the logic would be:

if ( has( 'python3' ) && !exists('g:slimv_python_version') )
    \ || ( exists( 'g:slimv_python_version' ) && g:slimv_python_version == 3 )
    let s:py_cmd = 'python3 ' "note space
    let s:pyfile_cmd = 'py3file '
else
    let s:py_cmd = 'python '  "note space
    let s:pyfile_cmd = 'pyfile '
endif

That would need a small note in the documentation about it, but it would keep the logic that python 2 is the default unless python 3 is available or forced, and python 2 could be forced if needed.

Point (2): I'm glad this code was mostly compatible (I've run into code that had to be substantially changed to work in both 2 and 3), it shouldn't be a big deal to have that conditional there in this case, especially since it's in only one or two places.

For (3): The print() function accepts the file= named parameter for which output stream to print to. Examples could be stdout (the default), a file object, a socket, etc. I seem to have read the variable name too literally in this case; it should just be a normal parameter to print the number. (leftover from that 2am commit, I think!)

I'll push these changes shortly. Thanks again!

[jakykong]

This version has the logic I just described for point (1) and updated documentation. Have a look and see if you like it. :) If not, easy enough to revert. (2) and (3) are also completed. Still working great under Python 3!

[kovisoft]

My understanding is (was) that has(python) returns 1 only when python support is enabled in vim AND the python 2.x dll is loaded (and similarly for has(python3) and the python 3.x dll). So we just don't need to do any further check for the python support. I even remember playing with this (on Windows): removing one of the python2x.dll/python3x.dll from the path and then the corresponding has(python[3]) command just returned 0. So then isn't this test just not reliable on some systems? Is it something specific to Windows?

I see this in the vim 7.3 code (if_python3.c), this part is called upon has('python3'), and it seems it actually tries to load the python library (however there is a message that on Unix two versions of the same lib cannot be loaded simultaneously):

/*
 * If python is enabled (there is installed python on Windows system) return
 * TRUE, else FALSE.
 */
    int
python3_enabled(int verbose)
{
    return py3_runtime_link_init(DYNAMIC_PYTHON3_DLL, verbose) == OK;
}

/*
 * Load library and get all pointers.
 * Parameter 'libname' provides name of DLL.
 * Return OK or FAIL.
 */
    static int
py3_runtime_link_init(char *libname, int verbose)
{
    int i;
    void *ucs_from_string, *ucs_from_string_and_size;

# if !defined(PY_NO_RTLD_GLOBAL) && defined(UNIX) && defined(FEAT_PYTHON)
    /* Can't have Python and Python3 loaded at the same time.
     * It cause a crash, because RTLD_GLOBAL is needed for
     * standard C extension libraries of one or both python versions. */
    if (python_loaded())
    {
    EMSG(_("E837: This Vim cannot execute :py3 after using :python"));
    return FAIL;
    }
# endif

    if (hinstPy3 != 0)
    return OK;
    hinstPy3 = load_dll(libname);
...

[jakykong]

I ran into this issue on Windows -- has('python') returned 1, but it wasn't until I tried ":python print 'Hello, World!' " that Vim offered an error about being unable to load python27.dll. At least in that particular combination, it's not reliable, I assumed that was to be expected since has('feature') normally returns 1 for anything compiled in.

The documentation (under if_pyth.txt/help has-python) states:

You can test what Python version is available with: >
    if has('python')
      echo 'there is Python 2.x'
    elseif has('python3')
      echo 'there is Python 3.x'
    endif

Note however, that when Python 2 and 3 are both available and loaded
dynamically, these has() calls will try to load them.  If only one can be
loaded at a time, just checking if Python 2 or 3 are available will prevent
the other one from being available.

That's a bit different from the behavior I observed when setting up my corporate box. I'm not sure under what circumstances only one can be loaded, so I don't know how reliable this would be under normal circumstances.

[kovisoft]

Ah, so then the key points are:

(1) "when Python 2 and 3 are both available and loaded dynamically, these has() calls will try to load them", so vim tries to load them upon the has(python[3]) statement only when python is dynamically loaded. Indeed, I found this one in the vim source that calls python3_enabled() only if DYNAMIC_PYTHON3 (+python3/dyn, I guess) defined. And the same is true for python 2.x:

Python3_Init(void)
{
    if (!py3initialised)
    {
#ifdef DYNAMIC_PYTHON3
    if (!python3_enabled(TRUE))
    {
        EMSG(_("E263: Sorry, this command is disabled, the Python library could not be loaded."));
        goto fail;
    }
#endif

(2) If only one can be loaded at a time, just checking if Python 2 or 3 are available will prevent the other one from being available - This means for me that checking has(python3) may break the subsequent has(python) calls and vice versa. So if someone has a system with both python 2.x and 3.x installed, and this user defines let g:slimv_python_version=2, then the first has(python3) will return 1 and load python 3.x dll, and then all python commands will fail, because we have mapped everything to python 2.x.

So either we just totally omit variable g:slimv_python_version, or we need to have also the has(python[3]) calls driven by g:slimv_python_version when the variable is defined.

[kovisoft]

How about if you swap the order of has( 'python3' ) and !exists('g:slimv_python_version')? This way has( 'python3' ) is not executed if g:slimv_python_version is defined:

if ( !exists( 'g:slimv_python_version' ) && has( 'python3' ) ) ||
\  (  exists( 'g:slimv_python_version' ) && g:slimv_python_version == 3 )
    let s:py_cmd = 'python3 ' "note space
    let s:pyfile_cmd = 'py3file '
else
    let s:py_cmd = 'python '  "note space
    let s:pyfile_cmd = 'pyfile '
endif

And then we need to reverse the order of the has(...) calls in SlimvConnectSwank(), so that if someone defined g:slimv_python_version to 3 (and therefore no has(...) statement was executed in the mapping phase) then has(python) would not grab python 2.x instead of the desired 3.x:

function! SlimvConnectSwank()
     if !s:python_initialized
        if ( ! has('python3') ) && ( ! has('python') )
...

[jakykong]

Sorry for the delayed response, busy couple of days!

I pushed the changes you mentioned -- spot on in your analysis. That looks to me that it satisfies all of the requirements (can load python 2 or 3, will only attempt to check 3 if it's not prevented from doing so, and that is configurable by the user to deal with edge cases.)

[jakykong]

In SlimvConnectSwank(), I think there could be an issue if the user defines g:slimv_python_version == 2, since now the first check will be has('python3'), but we wouldn't have checked it in the initialization script.

Maybe a check against s:py_cmd would resolve that - only check the respective python version that was already calculated, regardless of which one that was.

[jakykong]

Here's a commit that demonstrates the thought on that approach.

[kovisoft]

Thank you for the modifications and I also apologize for the late response (had a busy weekend, too :). You are absolutely right regarding the g:slimv_python_version = 2 case that may be screwed by has(python3) coming first. I also agree that we need to handle this somehow in SlimvConnectSwank(). Your approach seems to be the right way, except one thing I noticed:

If s:py_cmd == 'python3 ' but has(python3) fails, then this is already an error situation, because nothing will work even if the subsequent has(python) succeeds. Consider this: the user sets g:slimv_python_version = 3 but python 3.x is actually not installed on the system. In this case the script sets s:py_cmd = 'python3 ' but has(python3) returns 0 and has(python) returns 1. Therefore the python check will succeed in SlimvConnectSwank() but no :python3 command will work.

So I suggest that we check for python in SlimvConnectSwank() this way or similar:

        if ( s:py_cmd == 'python3 ' && ! has('python3') ) ||
         \ ( s:py_cmd == 'python '  && ! has('python' ) )
            call SlimvErrorWait( 'Vim is compiled without the Python feature or Python is not installed. Unable to run SWANK client.' )
            return 0
        endif

What do you think?

[jakykong]

It's just taking my approach one step further and explicitly checking for both versions, should work great! I've pushed the change with this logic. :)