divmod-dev team mailing list archive

[Jean-Paul Calderone <exarkun@xxxxxxxxxxxxxxxxx>]

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.