| msg92457 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2009-09-09 19:03 |
| The readline library supplied in OS X 10.6 looks good enough to use in Python. It would be nice to enable building with this library, to avoid having to install GNU readline. There's a curious off-by-one difference between Apple's readline (which, as I understand it, is just libedit wrapped to look like libreadline) and GNU readline: with 'n' history items, the valid indices for Apple's readline are 0 through n-1; for GNU they're 1 through n. I was able to get Python trunk + system readline working on OS X 10.6 using the attached patch (which obviously isn't suitable for applying, since it breaks the build with a non-system readline). A side effect of the patch is that readline.get_history_item and friends store the first history entry with index 0 rather than 1: Python 2.7a0 (trunk:74735M, Sep 9 2009, 19:40:25) [GCC 4.2.1 (Apple Inc. build 5646)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import readline [39474 refs] >>> readline.get_history_item(0) 'import readline' [39476 refs] >>> readline.get_history_item(2) 'readline.get_history_item(2)' [39476 refs] Interestingly, the Apple-supplied Python also behaves this way: Mark-Dickinsons-MacBook-Pro:trunk dickinsm$ python Python 2.6.1 (r261:67515, Jul 7 2009, 23:51:51) [GCC 4.2.1 (Apple Inc. build 5646)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import readline >>> readline.get_history_item(0) 'import readline' >>> readline.get_history_item(2) 'readline.get_history_item(2)' If people think this is worth pursuing, I'll put together a proper patch. |
|
|
| msg92458 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-09-09 19:04 |
| And here's the patch. |
|
|
| msg92460 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2009-09-09 19:18 |
| I wouldn't mind having a proper patch and doing away with the need for GNU's readline. IMHO the patch should try to stay as close to GNU readline's interface as possible, and should therefore fix the off-by-one difference you mention. BTW. I suppose the configuration (readline.parse_and_bind) needs to be in libedit format rather than readline format. If so, we should add something to the readline documentation about that and possibly add something to the readline library to make it possible to detect if readline is ultimately linked to libedit. BTW. If you want to push the Apple's readline to the limit you should try if it works properly with ipython. It used to cause hard crashes with Apple's python in 10.5. |
|
|
| msg92463 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-09-09 19:55 |
| Hmm. This is looking like a bigger task than I bargained for. I notice that the readline library currently has no tests (or maybe I'm just failing to find them). I'm not even sure how to go about writing tests for readline. > IMHO the patch should try to stay as close to GNU readline's interface > as possible, and should therefore fix the off-by-one difference you > mention. I suppose so. I'm a bit worried about subtle bugs occurring as a result of fixing off-by-one differences in some places and missing them in others; it seems safer to provide direct access to the library behaviour without trying to fix anything. Third-party stuff that wants to work with Apple's Python is also going to have to deal with zero-based history. So I guess I'd prefer not to fix the off-by-one difference; this would make it even more important to provide some way for users to tell whether they're using GNU readline or the wrapped libedit version. Thanks for the ipython suggestion; I've never used it before, but I'll see if I can play around with it a bit. |
|
|
| msg92475 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-09-10 08:57 |
| I also agree that this is desirable to have, and that the readline module should provide the GNU semantics even with a different implementation. |
|
|
| msg92485 - (view) |
Author: Zvezdan Petkovic (zvezdan) * |
Date: 2009-09-10 13:37 |
| This patch could potentially break non-Mac OS X systems. Fortunately, I have a patch that works with systems that use GNU readline and systems that use editline emulation. See issue 6877. Unfortunately, I was lingering for over a year with opening a tracker issue for it. Last night I did the last testing session with the trunk checkout and I did not notice that this issue has been opened in the meantime. Sorry for opening the double issue. I think that the patch from issue 6877 should be used. |
|
|
| msg92486 - (view) |
Author: Zvezdan Petkovic (zvezdan) * |
Date: 2009-09-10 13:43 |
| Also, the patch from issue 6877 changes setup.py in a way that enables build of the readline module on Leopard as well. Such build is used for about two years already (Python 2.4) by several people in my company and nobody noticed any issues on Mac OS X Leopard. AFAICT, it works the same now on Snow Leopard. |
|
|
| msg92492 - (view) |
Author: James (purpleidea) |
Date: 2009-09-10 16:41 |
| it seems to me, that any and all readline interfaces should/could standardize to the indexing scheme as used by the language; maybe i'm wrong, but since python is zero based, so could the readline interfaces. it's definitely more logical for a python programmer to expect zero-based, and the code will match with the python code. i would propose that everything be zero-based; this is duplicate/similar of/to http://bugs.python.org/issue6786 |
|
|
| msg92653 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2009-09-15 20:15 |
| purpleidea : Whether or not indexes should be 0-based in general is beyond the scope of this issue. |
|
|
| msg92655 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2009-09-15 21:13 |
| I've added an updated patch to issue 6877 that implements the same 1-based indexing as GNU's readline and also adds a note to the documentation to warn users about the possibility of linking the readline module to libedit. That patch would, possibly with clearer documentation, IMHO fix this issue. |
|
|
| msg92896 - (view) |
Author: Ronald Oussoren (ronaldoussoren) * |
Date: 2009-09-20 14:55 |
| This is a duplicate of issue 6877, I'm therefore closing this one. I've just committed a slightly updated patch from that issue to the trunk and 3.2. |
|
|
| msg93093 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-09-24 20:18 |
| Thanks for working on this, Ronald. |
|
|