Author zvezdan
Recipients ronaldoussoren, zvezdan
Date 2009-09-16.15:53:44
SpamBayes Score 2.27596e-15
Marked as misclassified No
Message-id <1253116425.95.0.927269075073.issue6877@psf.upfronthosting.co.za>
In-reply-to
Content
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?
History
Date User Action Args
2009-09-16 15:53:46zvezdansetrecipients: + zvezdan, ronaldoussoren
2009-09-16 15:53:45zvezdansetmessageid: <1253116425.95.0.927269075073.issue6877@psf.upfronthosting.co.za>
2009-09-16 15:53:44zvezdanlinkissue6877 messages
2009-09-16 15:53:44zvezdancreate