Fix for Issue #2 in cro-ki/xdice

[Remillard]

• Gave Score a __str__ method so expressions would construct properly (formerly the string constructed was using __repr__ which gives a full class representation but is not appropriate for eval)
• Updated roll.py to use argparse (mainly for my own sanity.)

[coveralls]

Coverage remained the same at 100.0% when pulling 9e3da6e on Remillard:feature/fix_str_v_repr into a4da4c4 on cro-ki:master.

[olinox14]

Alright for the argparse use,. As said in the issue tread, the str addition is for compat with python3.8+

[olinox14]

Also, I restored the verbose option, since the default output of CLI is:

8    ([2,2]+4)

The numeric-only is:

8

and the verbose output is:

8       (2d6(scores:[2, 2])+4)

[olinox14]

At last, since you allowed one on more expressions in the CLI, I changed the main method to allow the user to eval a few expressions in one row (ex: python roll.py 1d6 2d4), but I wonder: did you did it to allow user to put whitespaces in its expression? (since you joined the args afterwards)

[Remillard]

Oh no sweat on the argparse thing. I guess I hadn't noticed the difference between the two more verbose options so sorry about that. I half expected you to kick it back due to the roll.py alteration :D

Only other thing I can think left to do is that I noticed your version on GitHub was 1.2 and pip was installing 1.1.4. That's why there's that initial comment about the error lines not lining up. As soon as I uninstalled the 1.1.4 version and cloned the repository everything lined up again. So you might consider updating the pip distribution once you are wholly satisfied that it's ready for the wider world.

[olinox14]

You're right, I did'nt published it so far on pypi, it's done. Thanks!

[Remillard]

At last, since you allowed one on more expressions in the CLI, I changed the main method to allow the user to eval a few expressions in one row (ex: python roll.py 1d6 2d4), but I wonder: did you did it to allow user to put whitespaces in its expression? (since you joined the args afterwards)

Well when you get a series of inputs from argparse like that, it's a list of strings. i joined with no whitespace just as a matter of habit. I thought you stripped whitespace before sending to eval so it didn't trip any mental warning bells. If it'll do multiple groups of dice rolls, then you'd want to join with whitespace I suppose.

[olinox14]

Well, the 2 options seems good to me:

• only accept one expression per command, this way the ''.join(expression) will get rid of potential whitespaces in the expression
• accept one or more expression inline, which can be cool, but won't allow any whitespace inside one expression

I choose the second one for now, but I'm still hesitating...

[Remillard]

Well I'll leave that up to you. From my perspective, the CLI application is primarily for testing and experimenting. I know personally I'm putting xdice into a Discord bot (we needed something to handle some casual dice rolling for Blood Bowl) and the command line tool is just to check certain things out.

So whether the CLI takes multiple expressions or not doesn't bother much all that much. Out of habit, I tend to put spaces around mathematical operators, so my own personal leaning would be to just do a single expression and handle whitespace without quotes, but entirely up to you. Having the library functional was what prompted the entire investigation. I did notice you switched that to str(int(val)) which is much superior to my string-with-format. Don't know why I didn't think of that to begin with.