Python items in the todo list

[Bram Moolenaar]

I have included all the Python patches that I had listed in the todo
file, except some old ones.

Below is the list of remaining Python items. Please have a look and
tell me what is still an issue and what can be dropped.

There is also the patch from Yasuhiro Matsumoto that breaks the tests,
see an earlier message about that.

Also, please look at the Python interface documentation, and find
anything that doesn't look right. We can still make a few changes
before setting "in stone" when Vim 7.4 rolls out.

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

There is a ":py3do" command now, but not a ":pydo" command.

Patch to print the result of a :python command. (Maxim Philippov
<phili...@gmail.com>, 2012 Aug 16) Update Aug 17.
Patch no longer applies.

Python: Adding line to buffer other than the current one doesn't work
correctly. (Rozbujnik, 2010 Dec 19)

Patch to dynamically load Python on Solaris. (Danek Duvall, 2009 Feb 16)
Needs more work.

":python os.chdir('/tmp')" makes short buffer names invalid. (Xavier de Gaye)
Check directory and call shorten_fnames()?

Mac: OS/X 10.4 with Python 2.5 installed: configure finds an extra argument
that breaks the build. (Brian Victor, 2008 Sep 1)

Patch to access screen under Python. (Marko Mahni, 2010 Jul 18)

Python: ":py raw_input('prompt')" doesn't work. (Manu Hack)

Win32: The Python interface only works with one version of Python, selected at
compile time. Can this be made to work with version 2.1 and 2.2 dynamically?

Python: be able to define a Python function that can be called directly from
Vim script. Requires converting the arguments and return value.

Python interface: add vim.message() function. (Michal Vitecek, 2002 Nov 5)


--
If your life is a hard drive,
Christ can be your backup.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Danek Duvall]

On Wed, May 15, 2013 at 06:58:53PM +0200, Bram Moolenaar wrote:

> Patch to dynamically load Python on Solaris. (Danek Duvall, 2009 Feb 16)
> Needs more work.

I don't know that it's worth following up on this. The current python/dyn
interface works just fine on Solaris, and while it's probably possible to
squeeze some performance out with some linker tricks, I doubt it's worth
the code complexity.

I still think it'd be great to be able to load whatever version of python
you had on your system, rather than just the one vim was built against, but
that was never part of my patch, anyway.

Oh, and I'm still seeing problems with test86 due to patch 924, which I
reported on May 7.

Thanks,
Danek

[ZyX]

>I have included all the Python patches that I had listed in the todo
>file, except some old ones.
>
>Below is the list of remaining Python items. Please have a look and
>tell me what is still an issue and what can be dropped.
>
>There is also the patch from Yasuhiro Matsumoto that breaks the tests,
>see an earlier message about that.
>
>Also, please look at the Python interface documentation, and find
>anything that doesn't look right. We can still make a few changes
>before setting "in stone" when Vim 7.4 rolls out.
>
>-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
>
>There is a ":py3do" command now, but not a ":pydo" command.
>
>Patch to print the result of a :python command. (Maxim Philippov
><phili...@gmail.com>, 2012 Aug 16) Update Aug 17.
>Patch no longer applies.
>
>Python: Adding line to buffer other than the current one doesn't work
>correctly. (Rozbujnik, 2010 Dec 19)

Found this one. It persists, script to test:

vim -u NONE -c $'let nr=bufnr("scratch", 1)\ncall setbufvar(nr, "&bt", "nofile")\ncall setbufvar(nr, "&bh", "hide")\ncall setbufvar(nr, "&buflisted", 1)\npy import vim\ndef myprint():\n vim.buffers[2].append("hello")' -s <(<<< $'iabc\e:py myprint()\n:w /tmp/result\n')

. If everything is fine, /tmp/result must not contain hello. Note: buffer number
changed to 2 due to change in vim.buffers.

>Patch to dynamically load Python on Solaris. (Danek Duvall, 2009 Feb 16)
>Needs more work.
>
>":python os.chdir('/tmp')" makes short buffer names invalid. (Xavier de Gaye)
>Check directory and call shorten_fnames()?

Probably not only the python problem.

>Mac: OS/X 10.4 with Python 2.5 installed: configure finds an extra argument
>that breaks the build. (Brian Victor, 2008 Sep 1)
>
>Patch to access screen under Python. (Marko Mahni, 2010 Jul 18)
>
>Python: ":py raw_input('prompt')" doesn't work. (Manu Hack)

Persists. It not only “doesn't work”, it hangs vim.

>Win32: The Python interface only works with one version of Python, selected at
>compile time. Can this be made to work with version 2.1 and 2.2 dynamically?
>
>Python: be able to define a Python function that can be called directly from
>Vim script. Requires converting the arguments and return value.

Converting the arguments and return value can be easily done. This proposal
requires in first place not converting something, but changing the funcref
implementation (proposal mentioned in my RFC, no detailed description yet; old
detailed description written somewhere is probably outdated. AFAIR it was
expressed in the thread where it was proposed to add FFI better then libcall*).

>Python interface: add vim.message() function. (Michal Vitecek, 2002 Nov 5)

Unable to find. If equivalent to perl VIM::Msg then it is done by `print "foo"`
or `sys.stdout.write("foo\n")`.

[Bram Moolenaar]

ZyX wrote:

[...]

> >":python os.chdir('/tmp')" makes short buffer names invalid. (Xavier de Gaye)
> >Check directory and call shorten_fnames()?
>
> Probably not only the python problem.

I wonder if there is a hook inside Python, so that we can do the
equivalent of ":cd" in Vim, handling the side effects.

> >Python: be able to define a Python function that can be called directly from
> >Vim script. Requires converting the arguments and return value.
>
> Converting the arguments and return value can be easily done. This proposal
> requires in first place not converting something, but changing the funcref
> implementation (proposal mentioned in my RFC, no detailed description
> yet; old detailed description written somewhere is probably outdated.
> AFAIR it was expressed in the thread where it was proposed to add FFI
> better then libcall*).

Ideally we could do vim.f.{function-name}() for every function defined
in eval.c. That's more than 250 functions. Creating a funcref for each
of them might slow down startup. I assume we can do it only when used.

> >Python interface: add vim.message() function. (Michal Vitecek, 2002 Nov 5)
>
> Unable to find. If equivalent to perl VIM::Msg then it is done by
> `print "foo"` or `sys.stdout.write("foo\n")`.

It's very old, let's drop it.

--
There are 10 kinds of people: Those who understand binary and those who don't.

[Bram Moolenaar]

Danek Duvall wrote:

> On Wed, May 15, 2013 at 06:58:53PM +0200, Bram Moolenaar wrote:
>
> > Patch to dynamically load Python on Solaris. (Danek Duvall, 2009 Feb 16)
> > Needs more work.
>
> I don't know that it's worth following up on this. The current python/dyn
> interface works just fine on Solaris, and while it's probably possible to
> squeeze some performance out with some linker tricks, I doubt it's worth
> the code complexity.

I believe the note was about more work for the patch, not more work for
a working interface. If there are more improvements those are welcome,
of course.

> I still think it'd be great to be able to load whatever version of python
> you had on your system, rather than just the one vim was built against, but
> that was never part of my patch, anyway.
>
> Oh, and I'm still seeing problems with test86 due to patch 924, which I
> reported on May 7.

There is not much I can do about that..

--
There are three kinds of people: Those who can count & those who can't.

[ZyX]

> Ideally we could do vim.f.{function-name}() for every function defined
> in eval.c. That's more than 250 functions. Creating a funcref for each
> of them might slow down startup. I assume we can do it only when used.

New funcref structure is something like `struct func_T {void *func_data; struct func_funcs *functions; <GC vars>};` (former is arbitrary reference with function definition; latter holds references to C functions that actually call various functions, they all always receive func_data). It all can easily be defined statically assuming `struct type1_S {type2_T *data;};` and `struct generictype1_S {void *data;};` are equivalent structures pointers to which can be casted to each other without problems. AFAIK this statement is true. You only need to create dictionary and record these functions there with my suggestion then. Without it can be left in the statically created array; there is no must in keeping built-ins strictly in dictionary like I suggested, only reducing asymptomatic complexity of getting funcref from function name.

[Danek Duvall]

Bram Moolenaar wrote:

> Danek Duvall wrote:
>
> > On Wed, May 15, 2013 at 06:58:53PM +0200, Bram Moolenaar wrote:
> >
> > > Patch to dynamically load Python on Solaris. (Danek Duvall, 2009 Feb 16)
> > > Needs more work.
> >
> > I don't know that it's worth following up on this. The current python/dyn
> > interface works just fine on Solaris, and while it's probably possible to
> > squeeze some performance out with some linker tricks, I doubt it's worth
> > the code complexity.
>
> I believe the note was about more work for the patch, not more work for
> a working interface. If there are more improvements those are welcome,
> of course.

Sure. I was just suggesting that it's not worth keeping that item on your
list, as I think that patch is a dead-end given the work that's been done
since on dynamic loading of other language platforms.

> > Oh, and I'm still seeing problems with test86 due to patch 924, which I
> > reported on May 7.
>
> There is not much I can do about that..

Okay. I guess it's up to ZyX to provide that fix; I would, but I don't
know what the right fix is. I suspect that changing "iminsert" to
"tabstop" would be the right start, but I don't know how to change the
expected output properly. I'd like to see this fixed before 7.4, so that I
can deliver 7.4 with all tests passing, even if it's the test that's
broken.

Thanks,
Danek

[ZyX]

# HG changeset patch
# User ZyX <kp-...@ya.ru>
# Date 1368668966 -14400
# Branch python-extended-2
# Node ID 9ea73172864c892283abb0735b427cba5485820c
# Parent bdbdab08cc4de3f8e44567bff7ff3a032287dec3
Fix tests: messages (:lang C suggested by Dominique Pelle) and iminsert (replaced with shiftwidth)

diff -r bdbdab08cc4d -r 9ea73172864c src/testdir/test86.in
--- a/src/testdir/test86.in Wed May 15 19:07:47 2013 +0200
+++ b/src/testdir/test86.in Thu May 16 05:49:26 2013 +0400
@@ -7,6 +7,7 @@


STARTTEST
+:lang C
:so small.vim
:if !has('python') | e! test.ok | wq! test.out | endif
:py import vim
@@ -354,7 +355,7 @@
:" colorcolumn: string, window-local
:" statusline: string, window-local/global
:" autoindent: boolean, buffer-local
-:" iminsert: number, buffer-local
+:" shiftwidth: number, buffer-local
:" omnifunc: string, buffer-local
:" preserveindent: boolean, buffer-local/global
:" path: string, buffer-local/global
@@ -411,7 +412,7 @@
:let lst+=[['colorcolumn', '+1', '+2', '+3', 'abc', 0, 0, 1 ]]
:let lst+=[['statusline', '1', '2', '4', 0, 0, 1, 1 ]]
:let lst+=[['autoindent', 0, 1, 1, 2, 1, 0, 2 ]]
-:let lst+=[['iminsert', 0, 2, 1, 3, 0, 0, 2 ]]
+:let lst+=[['shiftwidth', 0, 2, 1, 3, 0, 0, 2 ]]
:let lst+=[['omnifunc', 'A', 'B', 'C', 1, 0, 0, 2 ]]
:let lst+=[['preserveindent', 0, 1, 1, 2, 1, 1, 2 ]]
:let lst+=[['path', '.,,', ',,', '.', 0, 0, 1, 2 ]]
diff -r bdbdab08cc4d -r 9ea73172864c src/testdir/test86.ok
--- a/src/testdir/test86.ok Wed May 15 19:07:47 2013 +0200
+++ b/src/testdir/test86.ok Thu May 16 05:49:26 2013 +0400
@@ -232,7 +232,7 @@
G: 0
W: 1:0 2:1 3:0 4:1
B: 1:0 2:1 3:0 4:1
->>> iminsert
+>>> shiftwidth
p/gopts1! KeyError
inv: 3! KeyError
gopts1! KeyError
@@ -241,15 +241,15 @@
wopts1! KeyError
wopts2! KeyError
wopts3! KeyError
- p/bopts1: 2
- G: 1
- W: 1:0 2:2 3:2 4:1
- B: 1:0 2:2 3:2 4:1
+ p/bopts1: 8
+ G: 8
+ W: 1:0 2:2 3:8 4:1
+ B: 1:0 2:2 3:8 4:1
del wopts3! KeyError
del bopts3! ValueError
- G: 1
- W: 1:0 2:2 3:2 4:1
- B: 1:0 2:2 3:2 4:1
+ G: 8
+ W: 1:0 2:2 3:8 4:1
+ B: 1:0 2:2 3:8 4:1
>>> omnifunc
p/gopts1! KeyError
inv: 1! KeyError
@@ -333,7 +333,7 @@
Current tab pages:
<tabpage 0>(1): 1 windows, current is <window object (unknown)>
Windows:
- <window object (unknown)>(0): displays buffer <buffer test86.in>; cursor is at (954, 0)
+ <window object (unknown)>(0): displays buffer <buffer test86.in>; cursor is at (955, 0)
<tabpage 1>(2): 1 windows, current is <window object (unknown)>
Windows:
<window object (unknown)>(0): displays buffer <buffer 0>; cursor is at (1, 0)
diff -r bdbdab08cc4d -r 9ea73172864c src/testdir/test87.in
--- a/src/testdir/test87.in Wed May 15 19:07:47 2013 +0200
+++ b/src/testdir/test87.in Thu May 16 05:49:26 2013 +0400
@@ -1,6 +1,7 @@
Tests for various python features. vim: set ft=vim :

STARTTEST
+:lang C
:so small.vim
:if !has('python3') | e! test.ok | wq! test.out | endif
:py3 import vim
@@ -340,7 +341,7 @@
:" colorcolumn: string, window-local
:" statusline: string, window-local/global
:" autoindent: boolean, buffer-local
-:" iminsert: number, buffer-local
+:" shiftwidth: number, buffer-local
:" omnifunc: string, buffer-local
:" preserveindent: boolean, buffer-local/global
:" path: string, buffer-local/global
@@ -397,7 +398,7 @@
:let lst+=[['colorcolumn', '+1', '+2', '+3', 'abc', 0, 0, 1 ]]
:let lst+=[['statusline', '1', '2', '4', 0, 0, 1, 1 ]]
:let lst+=[['autoindent', 0, 1, 1, 2, 1, 0, 2 ]]
-:let lst+=[['iminsert', 0, 2, 1, 3, 0, 0, 2 ]]
+:let lst+=[['shiftwidth', 0, 2, 1, 3, 0, 0, 2 ]]
:let lst+=[['omnifunc', 'A', 'B', 'C', 1, 0, 0, 2 ]]
:let lst+=[['preserveindent', 0, 1, 1, 2, 1, 1, 2 ]]
:let lst+=[['path', '.,,', ',,', '.', 0, 0, 1, 2 ]]
diff -r bdbdab08cc4d -r 9ea73172864c src/testdir/test87.ok
--- a/src/testdir/test87.ok Wed May 15 19:07:47 2013 +0200
+++ b/src/testdir/test87.ok Thu May 16 05:49:26 2013 +0400
@@ -221,7 +221,7 @@
G: 0
W: 1:0 2:1 3:0 4:1
B: 1:0 2:1 3:0 4:1
->>> iminsert
+>>> shiftwidth
p/gopts1! KeyError
inv: 3! KeyError
gopts1! KeyError
@@ -230,15 +230,15 @@
wopts1! KeyError
wopts2! KeyError
wopts3! KeyError
- p/bopts1: 2
- G: 1
- W: 1:0 2:2 3:2 4:1
- B: 1:0 2:2 3:2 4:1
+ p/bopts1: 8
+ G: 8
+ W: 1:0 2:2 3:8 4:1
+ B: 1:0 2:2 3:8 4:1
del wopts3! KeyError
del bopts3! ValueError
- G: 1
- W: 1:0 2:2 3:2 4:1
- B: 1:0 2:2 3:2 4:1
+ G: 8
+ W: 1:0 2:2 3:8 4:1
+ B: 1:0 2:2 3:8 4:1
>>> omnifunc
p/gopts1! KeyError
inv: 1! KeyError
@@ -322,7 +322,7 @@
Current tab pages:
<tabpage 0>(1): 1 windows, current is <window object (unknown)>
Windows:
- <window object (unknown)>(0): displays buffer <buffer test87.in>; cursor is at (929, 0)
+ <window object (unknown)>(0): displays buffer <buffer test87.in>; cursor is at (930, 0)
<tabpage 1>(2): 1 windows, current is <window object (unknown)>
Windows:
<window object (unknown)>(0): displays buffer <buffer 0>; cursor is at (1, 0)

[ZyX]

> Python: Adding line to buffer other than the current one doesn't work
> correctly. (Rozbujnik, 2010 Dec 19)

The following patch fixes things, but don’t ask me why. Just taken code from f_setbufvar for saving/restoring curbuf.

Maybe code somewhere uses curwin to get current buffer in place of curbuf.

# HG changeset patch
# User ZyX <kp-...@ya.ru>

# Date 1368677693 -14400
# Branch python-extended-2
# Node ID dbd9fe86da1020ad6b9bdadf332ccf43a4e1e6d6
# Parent 5f8adb1d88b905d73dc2b4d106740759a66113b3
Use aucmd_prepbuf/aucmd_restbuf everywhere in place of setting curbuf directly

diff -r 5f8adb1d88b9 -r dbd9fe86da10 src/if_py_both.h
--- a/src/if_py_both.h Thu May 16 06:49:12 2013 +0400
+++ b/src/if_py_both.h Thu May 16 08:14:53 2013 +0400
@@ -2316,10 +2316,10 @@
*/
if (line == Py_None || line == NULL)
{
- buf_T *savebuf = curbuf;
+ aco_save_T aco;

PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_savedel((linenr_T)n, 1L) == FAIL)
PyErr_SetVim(_("cannot save undo information"));
@@ -2332,7 +2332,7 @@
deleted_lines_mark((linenr_T)n, 1L);
}

- curbuf = savebuf;
+ aucmd_restbuf(&aco);

if (PyErr_Occurred() || VimErrorCheck())
return FAIL;
@@ -2345,14 +2345,14 @@
else if (PyString_Check(line))
{
char *save = StringToLine(line);
- buf_T *savebuf = curbuf;
+ aco_save_T aco;

if (save == NULL)
return FAIL;

/* We do not need to free "save" if ml_replace() consumes it. */
PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_savesub((linenr_T)n) == FAIL)
{
@@ -2367,7 +2367,7 @@
else
changed_bytes((linenr_T)n, 0);

- curbuf = savebuf;
+ aucmd_restbuf(&aco);

/* Check that the cursor is not beyond the end of the line now. */
if (buf == curwin->w_buffer)
@@ -2409,10 +2409,10 @@
{
PyInt i;
PyInt n = (int)(hi - lo);
- buf_T *savebuf = curbuf;
+ aco_save_T aco;

PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_savedel((linenr_T)lo, (long)n) == FAIL)
PyErr_SetVim(_("cannot save undo information"));
@@ -2431,7 +2431,7 @@
deleted_lines_mark((linenr_T)lo, (long)i);
}

- curbuf = savebuf;
+ aucmd_restbuf(&aco);

if (PyErr_Occurred() || VimErrorCheck())
return FAIL;
@@ -2448,7 +2448,7 @@
PyInt old_len = hi - lo;
PyInt extra = 0; /* lines added to text, can be negative */
char **array;
- buf_T *savebuf;
+ aco_save_T aco;

if (new_len == 0) /* avoid allocating zero bytes */
array = NULL;
@@ -2476,10 +2476,8 @@
}
}

- savebuf = curbuf;
-
PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_save((linenr_T)(lo-1), (linenr_T)hi) == FAIL)
PyErr_SetVim(_("cannot save undo information"));
@@ -2559,7 +2557,7 @@
if (buf == curwin->w_buffer)
py_fix_cursor((linenr_T)lo, (linenr_T)hi, (linenr_T)extra);

- curbuf = savebuf;
+ aucmd_restbuf(&aco);

if (PyErr_Occurred() || VimErrorCheck())
return FAIL;
@@ -2593,15 +2591,13 @@
if (PyString_Check(lines))
{
char *str = StringToLine(lines);
- buf_T *savebuf;
+ aco_save_T aco;

if (str == NULL)
return FAIL;

- savebuf = curbuf;
-
PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_save((linenr_T)n, (linenr_T)(n+1)) == FAIL)
PyErr_SetVim(_("cannot save undo information"));
@@ -2611,7 +2607,7 @@
appended_lines_mark((linenr_T)n, 1L);

vim_free(str);
- curbuf = savebuf;
+ aucmd_restbuf(&aco);
update_screen(VALID);

if (PyErr_Occurred() || VimErrorCheck())
@@ -2627,7 +2623,7 @@
PyInt i;
PyInt size = PyList_Size(lines);
char **array;
- buf_T *savebuf;
+ aco_save_T aco;

array = (char **)alloc((unsigned)(size * sizeof(char *)));
if (array == NULL)
@@ -2650,10 +2646,8 @@
}
}

- savebuf = curbuf;
-
PyErr_Clear();
- curbuf = buf;
+ aucmd_prepbuf(&aco, buf);

if (u_save((linenr_T)n, (linenr_T)(n + 1)) == FAIL)
PyErr_SetVim(_("cannot save undo information"));
@@ -2683,7 +2677,7 @@
*/
vim_free(array);

- curbuf = savebuf;
+ aucmd_restbuf(&aco);
update_screen(VALID);

if (PyErr_Occurred() || VimErrorCheck())
@@ -3099,7 +3093,7 @@
pos_T *posp;
char *pmark;
char mark;
- buf_T *curbuf_save;
+ aco_save_T aco;

if (CheckBuffer((BufferObject *)(self)))
return NULL;
@@ -3108,10 +3102,9 @@
return NULL;
mark = *pmark;

- curbuf_save = curbuf;
- curbuf = ((BufferObject *)(self))->buf;
+ aucmd_prepbuf(&aco, ((BufferObject *)(self))->buf);
posp = getmark(mark, FALSE);
- curbuf = curbuf_save;
+ aucmd_restbuf(&aco);

if (posp == NULL)
{

[ZyX]

> Maybe code somewhere uses curwin to get current buffer in place of curbuf.

Testing with the following patch applied (note the hash after diff -r, it is the same as parent of the posted patch) seems to prove my guess: it indeed does work. I don’t know where bugs may lie if you set curwin->w_buffer without all the stuff aucmd_prepbuf does, but this approach may be preferred for performance reasons. If we don’t need autocommands maybe using non-autocommand version should be preferred:


/*
* Prepare for executing commands for (hidden) buffer "buf".
* This is the non-autocommand version, it simply saves "curbuf" and sets
* "curbuf" and "curwin" to match "buf".
*/
void
aucmd_prepbuf(aco, buf)
aco_save_T *aco; /* structure to save values in */
buf_T *buf; /* new curbuf */
{
aco->save_curbuf = curbuf;
--curbuf->b_nwindows;
curbuf = buf;
curwin->w_buffer = buf;
++curbuf->b_nwindows;
}

, but I have not checked how setting non-current buffer should interact with TextChanged events. Currently I do not see TextChanged event in non-current buffer (prepended `-c 'au TextChanged * :echom expand("<abuf>")'` to command-line, got two lines with `1` on both; should be `1` then `2` I guess; tested against both this and patch in previous message), probably this should be fixed.

diff -r 5f8adb1d88b9 src/if_py_both.h

--- a/src/if_py_both.h Thu May 16 06:49:12 2013 +0400

+++ b/src/if_py_both.h Thu May 16 08:28:15 2013 +0400
@@ -2602,6 +2602,7 @@

PyErr_Clear();
curbuf = buf;
+ curwin->w_buffer = curbuf;

if (u_save((linenr_T)n, (linenr_T)(n+1)) == FAIL)
PyErr_SetVim(_("cannot save undo information"));

@@ -2612,6 +2613,7 @@

vim_free(str);
curbuf = savebuf;
+ curwin->w_buffer = savebuf;
update_screen(VALID);

if (PyErr_Occurred() || VimErrorCheck())

[Xavier de Gaye]

On Wed, May 15, 2013 at 8:51 PM, Bram Moolenaar wrote:
>> >":python os.chdir('/tmp')" makes short buffer names invalid. (Xavier de Gaye)
>> >Check directory and call shorten_fnames()?
>>
>> Probably not only the python problem.
>
> I wonder if there is a hook inside Python, so that we can do the
> equivalent of ":cd" in Vim, handling the side effects.

Indeed, a hook to chdir can be made with the following code in
a module named vim_hook:

########### vim_hook.py ###########
import vim
import os

_chdir = os.chdir

def chdir(path):
_chdir(path)
vim.command("cd " + path)

os.chdir = chdir
###########

This module is imported on vim startup with:

PyRun_SimpleString("import vim_hook");


To test that this fixes os.chdir, run the following commands after
having copied the above vim_hook.py file in the current directory:

:py3 import sys; sys.path[:1] = ['.']; import vim_hook
:py3 import os; os.chdir("/tmp")

Note that we must add the current directory to sys.path in order to be
able to import vim_hook. I believe this is another bug.


--
Xavier

Les Chemins de Lokoti: http://lokoti.alwaysdata.net

[James McCoy]

On Thu, May 16, 2013 at 01:53:11PM +0200, Xavier de Gaye wrote:
> Note that we must add the current directory to sys.path in order to be
> able to import vim_hook. I believe this is another bug.

No, it actually isn't. Please see CVE-2009-0316.

If there's a need for Vim to look in a specific directory for Python
modules, then a known directory (or set of directories) should be
defined and documented, similar to 'runtimepath'. Loading from the
current working directory is just broken.

Cheers,
--
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jame...@debian.org>

[Xavier de Gaye]

On Thu, May 16, 2013 at 2:28 PM, James McCoy wrote:
> On Thu, May 16, 2013 at 01:53:11PM +0200, Xavier de Gaye wrote:
>> Note that we must add the current directory to sys.path in order to be
>> able to import vim_hook. I believe this is another bug.
>
> No, it actually isn't. Please see CVE-2009-0316.

Thanks for the information.

[lilydjwg]

On Wed, May 15, 2013 at 06:58:53PM +0200, Bram Moolenaar wrote:

> [...]

>
> There is a ":py3do" command now, but not a ":pydo" command.
>

> [...]

Attached patch adds this.

--
Best regards,
lilydjwg

[Bram Moolenaar]

This comes up once in a while. Just changing "curwin" and/or "curbuf"
causes problems for some code. It's very hard to predict what code will
be executed. E.g., setting an option may cause window sizes to be
recomputed.

Let me take the existing switch_win() function and add switch_buf().
And disable autocommands in this state, that whould avoid a lot of
trouble.


--
My girlfriend told me I should be more affectionate.
So I got TWO girlfriends.

[Bram Moolenaar]

Lilydjwg wrote:

> On Wed, May 15, 2013 at 06:58:53PM +0200, Bram Moolenaar wrote:
> > [...]
> >
> > There is a ":py3do" command now, but not a ":pydo" command.
> >
> > [...]
>
> Attached patch adds this.

Thanks!

Would be nice to also have a few tests. Also for :py3do.

--
In Africa some of the native tribes have a custom of beating the ground
with clubs and uttering spine chilling cries. Anthropologists call
this a form of primitive self-expression. In America we call it golf.

[Bram Moolenaar]

Xavier de Gaye wrote:

> On Wed, May 15, 2013 at 8:51 PM, Bram Moolenaar wrote:
> >> >":python os.chdir('/tmp')" makes short buffer names invalid. (Xavier de Gaye)
> >> >Check directory and call shorten_fnames()?
> >>
> >> Probably not only the python problem.
> >
> > I wonder if there is a hook inside Python, so that we can do the
> > equivalent of ":cd" in Vim, handling the side effects.
>
>
> Indeed, a hook to chdir can be made with the following code in
> a module named vim_hook:
>
> ########### vim_hook.py ###########
> import vim
> import os
>
> _chdir = os.chdir
>
> def chdir(path):
> _chdir(path)
> vim.command("cd " + path)
>
> os.chdir = chdir
> ###########
>
> This module is imported on vim startup with:
>
> PyRun_SimpleString("import vim_hook");
>
>
> To test that this fixes os.chdir, run the following commands after
> having copied the above vim_hook.py file in the current directory:
>
> :py3 import sys; sys.path[:1] = ['.']; import vim_hook
> :py3 import os; os.chdir("/tmp")

OK, but we want this to work without any commands being used in a
script. It should be done when initalizing Python.

> Note that we must add the current directory to sys.path in order to be
> able to import vim_hook. I believe this is another bug.

--

The Law of VIM:
For each member b of the possible behaviour space B of program P, there exists
a finite time t before which at least one user u in the total user space U of
program P will request b becomes a member of the allowed behaviour space B'
(B' <= B).
In other words: Sooner or later everyone wants everything as an option.
-- Vince Negri

[Xavier de Gaye]

This is what I meant.

The attached patch does this.

A new directory named 'python' is added to the vim runtime files.
This directory contains at the moment a _vim_python.py file that is
imported when initalizing Python after the python directory pathname
has been added to sys.path.

_vim_python.py is meant to contain (or to import the other components
of) the python part of the vim-python interface.

[Xavier de Gaye]

Maybe we should use:

vim.command('silent cd! ' + path)

instead of:

vim.command('cd! ' + path)

[Bram Moolenaar]

Interesting. So we would have a $VIMRUNTIME/python directory and we can
put Python scripts there that we include with the distribution.

I'm sure it is only a small step that users will ask to have a python
directory in ~/.vim/python. Or more general, using 'runtimepath'.

Then a plugin does not require to have its Python code in between a
:python << EOF and EOF. That would be a lot nicer, right?

What do others think?

--
BROTHER MAYNARD: Armaments Chapter Two Verses Nine to Twenty One.
ANOTHER MONK: And St. Attila raised his hand grenade up on high saying "O
Lord bless this thy hand grenade that with it thou mayest
blow thine enemies to tiny bits, in thy mercy. "and the Lord
did grin and people did feast upon the lambs and sloths and
carp and anchovies and orang-utans and breakfast cereals and
fruit bats and...
BROTHER MAYNARD: Skip a bit brother ...
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

[ZyX]

> Interesting. So we would have a $VIMRUNTIME/python directory and we can
> put Python scripts there that we include with the distribution.
>
> I'm sure it is only a small step that users will ask to have a python
> directory in ~/.vim/python. Or more general, using 'runtimepath'.
>
> Then a plugin does not require to have its Python code in between a
> :python << EOF and EOF. That would be a lot nicer, right?
>
> What do others think?

Automatically adding all items from `&rtp` to sys.path would be good. It (and python3/) is already a standard directory in frawor for python files.

But I would vote against the patch to os.chdir implemented in python and (as there is no better variant) for the solution based on comparing current directories before and after `os.chdir`:

1. Implementation based on comparing current directories can be written once and easily applied to all other interfaces.
2. `os.chdir` is most common, but not the only way to change directories from python: there are also at least `posix.chdir` and calls to libc (e.g. indirectly from some bindings or directly using ctypes, though I can’t imagine why the latter may be required).
3. Proposed implementation will break once somebody deletes `os` from sys.modules for some reason (I used to use this variant to reset `os` module after mocking its methods for testing purposes; though as for all non-builtin modules touching `sys.modules`).

About the implementation of `&rtp` handling: you can add there to `sys.path` a special path like `'_vim_runtimepaths_'` and add hook to `sys.path_hooks` that can handle it. Requires dropping python 2.2 support (`path_hooks` were added in python 2.3), but you won’t then need to hack `options.c` to add or remove appropriate paths from `sys.path` when changing `&rtp`.

[Bram Moolenaar]

ZyX wrote:

> > Interesting. So we would have a $VIMRUNTIME/python directory and we can
> > put Python scripts there that we include with the distribution.
> >
> > I'm sure it is only a small step that users will ask to have a python
> > directory in ~/.vim/python. Or more general, using 'runtimepath'.
> >
> > Then a plugin does not require to have its Python code in between a
> > :python << EOF and EOF. That would be a lot nicer, right?
> >
> > What do others think?
>
> Automatically adding all items from `&rtp` to sys.path would be good.
> It (and python3/) is already a standard directory in frawor for python
> files.
>
> But I would vote against the patch to os.chdir implemented in python
> and (as there is no better variant) for the solution based on
> comparing current directories before and after `os.chdir`:
>
> 1. Implementation based on comparing current directories can be
> written once and easily applied to all other interfaces.
> 2. `os.chdir` is most common, but not the only way to change
> directories from python: there are also at least `posix.chdir` and
> calls to libc (e.g. indirectly from some bindings or directly using
> ctypes, though I can�t imagine why the latter may be required).
> 3. Proposed implementation will break once somebody deletes `os` from
> sys.modules for some reason (I used to use this variant to reset `os`
> module after mocking its methods for testing purposes; though as for
> all non-builtin modules touching `sys.modules`).

I think this implies we need to get and save the current directory for
every Python command that is executed. Doesn't this add quite a bit of
overhead? I originally asked for some kind of hook or callback, so that
we can do something when Python changes directory at a lower level.

I suppose we do need to wait with the Vim implementation of :cd until
the Python command finishes. Thus os.chdir() would set a flag. Then
when the Python command restores the directory we don't need to do
anything. I would expect this to be quite common:
saved_curdir = os.cwd()
os.chdir(somewhere)
... do something with curdir ..
os.chdir(saved_curdir)

> About the implementation of `&rtp` handling: you can add there to
> `sys.path` a special path like `'_vim_runtimepaths_'` and add hook to
> `sys.path_hooks` that can handle it. Requires dropping python 2.2
> support (`path_hooks` were added in python 2.3), but you won�t then
> need to hack `options.c` to add or remove appropriate paths from
> `sys.path` when changing `&rtp`.

Do we need this kind of magic? Updating sys.path is much simpler and
also does not hide what we are doing (in case someone is debugging
what's going on). The only trick we need is when the user changes
'runtimepath'. But that would happen very infrequently.

--
Arthur pulls Pin out. The MONK blesses the grenade as ...
ARTHUR: (quietly) One, two, five ...
GALAHAD: Three, sir!
ARTHUR: Three.

[Xavier de Gaye]

On Fri, May 17, 2013 at 9:07 PM, ZyX wrote:
>> Interesting. So we would have a $VIMRUNTIME/python directory and we can
>> put Python scripts there that we include with the distribution.
>>
>> I'm sure it is only a small step that users will ask to have a python
>> directory in ~/.vim/python. Or more general, using 'runtimepath'.
>>
>> Then a plugin does not require to have its Python code in between a
>> :python << EOF and EOF. That would be a lot nicer, right?
>>
>> What do others think?
>
> Automatically adding all items from `&rtp` to sys.path would be
> good. It (and python3/) is already a standard directory in frawor
> for python files.
>
> But I would vote against the patch to os.chdir implemented in python
> and (as there is no better variant) for the solution based on
> comparing current directories before and after `os.chdir`:
>
> 1. Implementation based on comparing current directories can be
> written once and easily applied to all other interfaces.
> 2. `os.chdir` is most common, but not the only way to change
> directories from python: there are also at least `posix.chdir` and

`os.chdir` and `posix.chdir` are two names that are bound to the same
built-in function, as you can check with:

>>> import os, posix
>>> os.chdir is posix.chdir
True

`posix.chdir` is low level and is never used. The python documentation
on 'posix' states:

"Do not import this module directly. Instead, import the module os".

`posix.chdir` is not even listed in the python documentation.

> calls to libc (e.g. indirectly from some bindings or directly using
> ctypes, though I can’t imagine why the latter may be required).

This is kind of a red herring as the same issue already exists with
the vimL libcall() function.

> 3. Proposed implementation will break once somebody deletes `os`
> from sys.modules for some reason (I used to use this variant to
> reset `os` module after mocking its methods for testing purposes;
> though as for all non-builtin modules touching `sys.modules`).

It is not good practice to import a module twice (unless you own it
and thus, know what you are doing) because importing a module may have
side effects: when you import a module, you execute all the statements
within the module, and executing them twice might not have been
expected by the author of the module.

If you mock some 'os' methods for testing purposes, then, when you are
done with it, you must restore those methods instead of deleting the
module from sys.modules and re-importing it a second time which is not
safe.

> About the implementation of `&rtp` handling: you can add there to
> `sys.path` a special path like `'_vim_runtimepaths_'` and add hook
> to `sys.path_hooks` that can handle it. Requires dropping python 2.2
> support (`path_hooks` were added in python 2.3), but you won’t then
> need to hack `options.c` to add or remove appropriate paths from
> `sys.path` when changing `&rtp`.

[ZyX]

>>> Interesting. So we would have a $VIMRUNTIME/python directory and we can
>>> put Python scripts there that we include with the distribution.
>>>
>>> I'm sure it is only a small step that users will ask to have a python
>>> directory in ~/.vim/python. Or more general, using 'runtimepath'.
>>>
>>> Then a plugin does not require to have its Python code in between a
>>> :python << EOF and EOF. That would be a lot nicer, right?
>>>
>>> What do others think?
>>
>> Automatically adding all items from `&rtp` to sys.path would be
>> good. It (and python3/) is already a standard directory in frawor
>> for python files.
>>
>> But I would vote against the patch to os.chdir implemented in python
>> and (as there is no better variant) for the solution based on
>> comparing current directories before and after `os.chdir`:
>>
>> 1. Implementation based on comparing current directories can be
>> written once and easily applied to all other interfaces.
>> 2. `os.chdir` is most common, but not the only way to change
>> directories from python: there are also at least `posix.chdir` and
>
>
>`os.chdir` and `posix.chdir` are two names that are bound to the same
>built-in function, as you can check with:

This does not matter. When you assign to `os.chdir` they are no longer
identical.

> >>> import os, posix
> >>> os.chdir is posix.chdir
> True
>
>`posix.chdir` is low level and is never used. The python documentation
>on 'posix' states:
>
> "Do not import this module directly. Instead, import the module os".
>
>`posix.chdir` is not even listed in the python documentation.
>
>
>> calls to libc (e.g. indirectly from some bindings or directly using
>> ctypes, though I can’t imagine why the latter may be required).
>
>
>This is kind of a red herring as the same issue already exists with
>the vimL libcall() function.

libcall() can use the same function for checking for os.chdir.

>> 3. Proposed implementation will break once somebody deletes `os`
>> from sys.modules for some reason (I used to use this variant to
>> reset `os` module after mocking its methods for testing purposes;
>> though as for all non-builtin modules touching `sys.modules`).
>
>
>It is not good practice to import a module twice (unless you own it
>and thus, know what you are doing) because importing a module may have
>side effects: when you import a module, you execute all the statements
>within the module, and executing them twice might not have been
>expected by the author of the module.

`os` seems to be safe regarding the issue.

>If you mock some 'os' methods for testing purposes, then, when you are
>done with it, you must restore those methods instead of deleting the
>module from sys.modules and re-importing it a second time which is not
>safe.

Yes, I changed the implementation immediately after I found that removing from
`sys.modules` may introduce strange TypeErrors (objects in module got assigned
None). I have never seen such behavior for C modules though, only for python
ones.

[ZyX]

>Do we need this kind of magic? Updating sys.path is much simpler and
>also does not hide what we are doing (in case someone is debugging
>what's going on). The only trick we need is when the user changes
>'runtimepath'. But that would happen very infrequently.

There are two other big advantages of this “magic”: user did not loose control
over `sys.path` and it can be written in pure python. All advantages in a list:
it …

1. makes it possible to specify which directories should be searched before
plugin ones and which after;
2. makes it possible to remove vim paths from sys.path completely and not ever
have problems with vim adding them again;
3. can be written in pure python and fallback implementation for old vims (the
ones even without vim.eval) can be created;
4. makes hacking non-if_py* C files not needed.

There is a reason for coding it in C though (and porting @Xavier de Gaye file to C as well): this makes vim core functionality independent of presence of runtime files. There is a reason for not coding it in C as well: it is faster to write in python and even being written in python this will not have problems with performance (it’ll in any case waste much time on IO).

[Xavier de Gaye]

I think that it is reasonable to answer to anyone that complains
something is broken after he deletes a module from sys.modules and
re-import it again:

"Please don't do that, if you really need to do it, then you are
on your own".

For example with the 'sys' module in the vim-python interface ('sys' is
a C extension module by the way):

:py3 import sys; del sys.modules['sys']
:py3 import sys
:py3 print(sys.stdout)
Traceback (most recent call last):
File "<string>", line 1, in <module>
AttributeError: 'module' object has no attribute 'stdout'
Press ENTER or type command to continue

Re-binding a name such as done in the patch with os.chdir is actually
quite common. It is even already done in the current vim-python
interface with sys.stdout and sys.stderr (see PythonIO_Init_io()). A
vim user writing a python program with the vim-python interface may
also re-bind sys.stdout and sys.stderr to implement i/o redirection,
but he is expected to restore the original bindings after he is done
and certainly must not delete the 'sys' module and re-import it again.