ulif/diceware

Capilization default setting bug

Closed this issue · 5 comments

I have discovered a bug. If I have the following in my .diceware.ini file:

[diceware]
delimiter = SS
caps = off

Then the output of the command:

diceware -n 2 -r realdice
#2
#2
#2 ... etc
#

is

ChickentestChicken

Thus, the caps setting doesn't work.

I have located the error in the code. The init.py file calls the option capitalize while the config.py calls this option caps. All of the unit tests check whether the caps variable is successfully set from the ini file, but they don't test whether the capitalization is applied.

I think the best way to fix it would be to replace all the instances of capitalize with caps in the init.py file, unless you prefer otherwise. Happy to take care of this bug and submit a pull request myself.

ulif commented

Thank you, @dwcoder ! Looks like you (again) discovered a bug.

After your hints I investigated further and noticed that in __init__.py the options object supports --caps and --no-caps arguments, but both set a different variable: caps sets options.capitalize while no-caps modifies options.caps. This is weird - and also a bit embarrasing.

I guess this is easy to fix. Only question is: 'caps' or 'capitalize'. I would tend to prefer 'caps' as it would not require users to change their .diceware.ini, is shorter, and generally better understood.

A pull request would be very welcome. Please note, that I recently pushed some other changes that were not related to this bug.

Sure, I also thought that caps is the best one. I will fix it later this week and submit the request, and I will make sure to build it on your newest version.

I will also be on the lookout for other issues with the .ini file, while I am on the topic. But one thing at a time. I am also working on a new feature branch which I will submit inthe future.

ulif commented

Thanks, that sounds great! Please take your time!

ulif commented

@dwcoder fixed this with #22 . Thank you very much for spotting and fixing this bug!

Thanks for accepting, I'll remember your comments about the unit tests for future reference!