Zvezdan Petkovic <zvez...@zope.com> added the comment:
The patch `readline-libedit.patch` has the following problems:
- a typo causing an undefined variable error;
- a missing "#endif";
- it doesn't refer to the new __APPLE__ specific doc string anywhere.
It fails to compile `readline.c`. ::
Modules/readline.c:1073: error:
‘using_libedit_emultation’ undeclared (first use in this
function)
Modules/readline.c:1059:1: error: unterminated #ifdef
The fixed patch is attached as `readline-libedit-2.patch` above.
Please review it.
There are also a few minor stylistic nits I took a liberty to fix.
In readline.c
=============
Line 496
Formatted a block comment to a usual C style (used by other comments
it the patch, FWIW).
Line 505
The standard style in C (and C-like) languages is to *not* write a
space between a variable and a unary operator (e.g., "C++").
Changed it to::
idx--;
Lines 1085/86
Removed two unnecessary empty lines added by the first patch.
There's already one empty line in the original code to space an if
statement from the next line.
Line 1070
Line was longer than 79 characters (violates PEP-7). Reformatted.
In setup.py
===========
Line 561
A comment block was indented in the first patch and had lines longer
than 79 characters (violates PEP-8). I refactored the code to avoid
repeating the same else statement twice. This obviated the need to
indent the said comment block.
*Please review whether you agree with the refactoring*
Just out of curiosity: does anyone know why the first if statement that
tests for 'darwin' uses ``platform`` and the second one uses
``sys.platform``? Couldn't we use ``platform`` in both of them?
----------
Added file: http://bugs.python.org/file14899/readline-libedit-2.patch
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue6877>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
