I wonder about the change to explicitly use `pyparsing.LineEnd()` in the 
expression for `Go`.  Am I correct in concluding from this change that commands 
are currently allowed to match lines as long as the command is a *prefix* of 
the line (potentially leaving unmatched text at the end)?  I can imagine this 
arising here since none of the existing commands are as like to be prefixes of 
other commands or bogus inputs as the new "n", "e", "s", and "w".  If this is 
the case it might make sense to fix this more generally by declaring (to us, 
the development team) that `Action.expr` implicitly has `pyparsing.LineEnd()` 
at the end and make `Action` automatically put it there for us.

Otherwise I think this is generally okay, but a couple more 
hopefully-straightforward comments:

  - there are help files in Imaginary/imaginary/resources/ that should be kept 
up-to-date with respect to the actual implementation of commands.
  - the unit tests should be broken up more so that each one is only testing 
one case (I know this does not reflect the prevailing style in Imaginary's test 
suite, sorry :/)
  - I want to mumble something about twisted.python.constants but that can 
clearly be delayed for a later ticket

Thanks for the contribution!

-- 
https://code.launchpad.net/~jerith/divmod.org/short-directions-1215929/+merge/181830
Your team Divmod-dev is requested to review the proposed merge of 
lp:~jerith/divmod.org/short-directions-1215929 into lp:divmod.org.

-- 
Mailing list: https://launchpad.net/~divmod-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~divmod-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to