shlex refactoring

[ferringb]

BPO	7594
Nosy	@mhammond, @ezio-melotti, @merwok
Files	shlex-py2.6.4-20091229.patch: shlex-py2.6.4 refactoring

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2009-12-29.08:07:40.084>
labels = ['library', 'performance']
title = 'shlex refactoring'
updated_at = <Date 2011-06-14.16:03:49.376>
user = 'https://bugs.python.org/ferringb'

bugs.python.org fields:

activity = <Date 2011-06-14.16:03:49.376>
actor = 'eric.araujo'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2009-12-29.08:07:40.084>
creator = 'ferringb'
dependencies = []
files = ['15692']
hgrepos = []
issue_num = 7594
keywords = ['patch', 'needs review']
message_count = 3.0
messages = ['96987', '96988', '105118']
nosy_count = 4.0
nosy_names = ['mhammond', 'ferringb', 'ezio.melotti', 'eric.araujo']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue7594'
versions = ['Python 3.3']

[ferringb]

Currently, shlex.shlex's internal read_token is a bit of a nasty mess to
follow and pretty poorly performant.

The refactoring I'll be attaching essentially inverts the state machine
approach read_token uses- instead, converting it over to a more
procedural set of loops. The benefits of this is that it's far, far
easier to track what the code is actually doing and the performance goes
up fairly drastically via tightening the various for loops (for example,
if doing quoting, you only care about escapes and the terminating quote-
thus a loop there).

End result, the refactored code actually has whitespace in it and still
is shorter- specifically the tokenenizer was converted into a generator
to preserve state internally, thus simplifying the code flow immensely.

Beyond that, this should still be api compatible... and is 2x faster,
passing tests.

Note this patch is cut against 2.6.4; assuming folks are happy w/ the
general approach, I'll rebase it to py3k trunk.

From there, I'd be willing to tackle the other shlex issues where
applicable, but personally... not w/out the refactoring base, since that
state machine is a pita to trace out.

[ezio-melotti]

Thanks for the patch.

Three minor things:

1. the source contains non-ASCII chars and an encoding declaration at
   the beginning of the file. PEP-8 says that "using \x, \u or \U escapes is
   the preferred way to include non-ASCII data in string literals", so I
   would use \x escapes and remove the encoding declaration;
2. the class "stream_source" should be named "StreamSource" unless there
   are compatibility or consistency issues;
3. patches should be done against trunk first (your patch applies fine
   on trunk too and the tests pass), they will then be ported to py3k.

[mhammond]

I tried to use this in place of shlex for parsing IMAP responses for the 'imapclient' package. A couple of things struck me.

• The class no longer has a next() method but probably should be added for b/w compat.

• The class no longer has a 'token' attribute, which people may use to record the current token. Sadly it isn't clear if this is a documented part of the API or not.

• Typo:
  wordchards = property(_get_wordchars, _set_wordchars)

  'wordchards' is wrong. This implies there are no tests which set wordchars.

• I *think* the lexer is now returning an empty string token as an input source is rolled over whereas before it did not. In effect, I *think* the old lexer would allow a single token to span sources, where this patched version does not. Sadly I didn't confirm this is truly accurate - but tests for this behaviour would probably help.

[iritkatriel]

The proposed refactor had some issue with it (according to the comments) and seems to have been abandoned for over a decade. I will close the issue unless someone sees a point in leaving it open.