piotrmurach / tty-prompt

A beautiful and powerful interactive command line prompt
https://ttytoolkit.org
MIT License
1.47k stars 136 forks source link

fix #106 - yes? and no? shouldn't raise an exception on invalid/blank input #114

Closed slowbro closed 5 years ago

slowbro commented 5 years ago

Describe the change

Fixes #106 by adding a default validator and message:

+               @validation = /^(y(es)?|no?)$/i
+               @messages[ :valid? ] = "Invalid input."

Also, overrides Question's process_input to insert the default positive or negative string on blank input.

Why are we doing this?

Fixes unhandled exceptions in yes?/no? when a user inputs unexpected text:

[4] pry(main)> p.yes?( "Yes?" )
Yes? (Y/n) b
TTY::Prompt::ConversionError: 'b' could not be converted from `string` into `boolean` 
from /usr/home/kschiesser/.rvm/gems/ruby-2.6.3@laika/gems/tty-prompt-0.19.0/lib/tty/prompt/converters.rb:23:in `rescue in on_error'

vs

[3] pry(main)> p.yes?( "Yes?" )
Yes? (Y/n) b
>> Invalid input.

Benefits

Fixes a couple unhandled exceptions.

Drawbacks

I stopped using Necromancer's :bool conversion for this - it seemed unnecessary, given the already existent conversion function plus the validation. Feel free to override me there :)

Requirements

Put an X between brackets on each line if you have done the item: [x] Tests written & passing locally? [x] Code style checked? [x] Rebased with master branch? [x] Documentation updated?

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.2%) to 96.928% when pulling 1637d239cfa6af81e23183a15f91133b5b14fcdb on slowbro:issue-106 into a26e3e79ba1a2c349bedcf6db5086c8634c50339 on piotrmurach:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.2%) to 96.928% when pulling 1637d239cfa6af81e23183a15f91133b5b14fcdb on slowbro:issue-106 into a26e3e79ba1a2c349bedcf6db5086c8634c50339 on piotrmurach:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.1%) to 97.003% when pulling a5b8f17831d6734e44c88fc5454289172acabd7e on slowbro:issue-106 into a26e3e79ba1a2c349bedcf6db5086c8634c50339 on piotrmurach:master.

slowbro commented 5 years ago

re: coveralls, that looks like lines from a previous commit - I did not change those files.

piotrmurach commented 5 years ago

Thank you for looking into this! I took a bit of break from open source but I'm back :)

Would you have time to add test case for this behaviour so I don't break it in future?

slowbro commented 5 years ago

Welcome back! I pushed some tests :)

piotrmurach commented 5 years ago

Thank you! ❤️ Tests will ensure longevity😄