[Patches] [ python-Patches-1669379 ] bytes.fromhex()

[SourceForge.net]

Patches item #1669379, was opened at 2007-02-26 19:18
Message generated for change (Comment added) made by gbrandl
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669379&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Core (C code)
Group: None
>Status: Closed
>Resolution: Accepted
Priority: 5
Private: No
Submitted By: Georg Brandl (gbrandl)
Assigned to: Neal Norwitz (nnorwitz)
Summary: bytes.fromhex()

Initial Comment:
Implement bytes.fromhex().

----------------------------------------------------------------------

>Comment By: Georg Brandl (gbrandl)
Date: 2007-02-27 08:42

Message:
Logged In: YES 
user_id=849994
Originator: YES

It is in the PEP, so I assume it is in the API.

I intended for all spaces to be allowed, but the PEP says otherwise,
so I updated the implementation accordingly.

I fixed the rest of your nits, fixed another bug and checked it in as rev.
53989.


----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2007-02-27 06:41

Message:
Logged In: YES 
user_id=33168
Originator: NO

I don't know if this is supposed to be in the API or not.  I'm too tired
to care, but here are my comments on the code.

It would be better to use a variable name other than new, since new is a
keyword in C++.  isspace() checks for any whitespace, so not only can there
be spaces, there can be tabs, carriage returns, new lines, etc.  I don't
know if the code is correct or the comment is. I see you have a test for
other whitespace, so I would just update the comment to say _any_
whitespace.

Does byteslen need the + 1?

Is there a macro to get ob_bytes rather than accessing directly?
    buf = ((PyBytesObject *)new)->ob_bytes;

It would be good to add the index position (i) in the error msg.  It might
be hard to see/non-obvious from the 2 chars (or potentially ambiguous).

How about adding a test that passes a buffer('') and buffer('00')?

These are all nits.  Assuming Guido is fine with the API, I'd say clean it
up and check it in.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669379&group_id=5470