[tor-dev] Stem code review 2012-12-04

[Damian Johnson]

> I must not be reasoning about Event._parse_standard_attr() correctly.  I
> think it is already looking for _QUOTED positional args, but is working at
> it backwards from _KEYWORD_ARGS parsing.

Right. The thing that I'm talking about is *new* quoted arguments (ie,
things not presently in the spec and hence not in _POSITIONAL_ARGS,
_KEYWORD_ARGS, or _QUOTED). Presently positional and keyword work a
little differently from each other for those new additions...

* With positional arguments we only parse them as being quoted if
they're in _QUOTED. Hence new positional additions that are quoted
will result multiple values in positional_args until we update the
parser.

* With keyword arguments _QUOTED is not consulted. Instead we
determine if they're quoted or not based on which regex they match.

The former is a strict parsing approach, where we're assured to parse
the things we presently support correctly but new additions could put
mis-parsed content in our positional_args.

The later is a more permissive parsing approach where it's possible
(though highly unlikely) that some content could confuse us but we
more gracefully handle new additions.

Probably the best of both worlds would be to use the strict approach
for things we presently parse, and a permissive approach for new
additions. Ie...

if len(unparsed_positional_args) > len(_POSITIONAL_ARGS):
  # Get the positional argument content that isn't recognized by
  # our parser. This probably represents a new addition to the
  # spec, and might or might not be quoted values.

  unrecognized_content = "
".join(unparsed_positional_args[len(_POSITIONAL_ARGS):])

  # TODO: Try to parse out quoted values, falling back to
  # parse them as being unquoted.

... then move on to the present parsing for _POSITIONAL_ARGS

Hope that helps. -Damian