Repository: JEG2/highline (ruby)

Issue:
Incosistent behavior when ask with a validator and readline=true
Hi, we're using highline in a console for a cloud management platform: https://github.com/manageiq/manageiq-appliance_console. Recently we add all `Highline.ask` with `readline=true` option to add arrow key navigation. It works fine for no validator `ask`, but for cases with a validator, like `Highline.agree` or `Highline.choose`, it's behavior is inconsistent with the same method call of  `readline=false` option. Though we use a previous version of highline, but I just test with current master branch and inconsistency still exist.  For a minimum example, try some invalid input to these two `agree`:

```ruby
#!/usr/bin/env ruby
require 'highline'

def agree_readline(prompt)
  HighLine.new.agree(prompt) do |q|
    q.readline=true
  end
end

def agree(prompt)
  HighLine.new.agree(prompt)
end

agree_readline("agree readline=true? ")
agree("agree readline=false? ")
```
Similar problems for `choose` or manually call `ask` given validator. I did some hacks in our project to make them lo behave the same way as `readline=false`. But I think it's better to fix here.


Discussion:
Hi @ailisp,

Thanks for reporting the issue.
What exactly is happening on your setup? 
What **behaviour** is inconsistent?

Hi @abinoam Thanks for quick response. In a clone of current master branch. save the content I provide in previous comment as test.rb:
```
[boyao@new-host highline]$ bundler exec ./test.rb
agree readline=true?aaa
Please enter "yes" or "no".
agree readline=true?
agree readline=true?
Please enter "yes" or "no".
agree readline=true?
agree readline=true?yes
agree readline=false?
aaa
Please enter "yes" or "no".
agree readline=false?

Please enter "yes" or "no".
agree readline=false?
yes
```
I input `aaa`, nothing and `yes` for both case (`readline=true` or `false`), and you can see with `readline=true` there's no extra newline after question, but a redundant question. 
For `choose`, if we have `readline=true` then giving an invalid input, the question and menu will both display again, but for `readline=false` only question will display again.
I think I understood.

I've had a quick look at the code now and couldn't figure out a quick fix.
I'll try to take a look at it next week and try to fix it or at least provide a way to work around it.

👍 
I see that the use of `readline = true` at `agree` is pretty much unimplemented.
It doesn't even autocomplete for **yes** or **no**.

Yes. just noticed the auto complete part is not implemented. And here's of some hacks I did for make `choose` and `agree` looks the same as `readline=false`, it works but with some ugly hacks so I don't make a PR here direcly. I think you're more familiar with GNU Readline, but hope this can save you some time.
- readline replacement of `agree`
https://github.com/ManageIQ/manageiq-appliance_console/pull/6/files#diff-11f3fdac8dc445b3e93dd2459f62b9a9
- readline replacement for `choose`, based on a modified  `ask`, some observations of my experiments are in comments
https://github.com/ManageIQ/manageiq-appliance_console/pull/6/files#diff-70eae34541a813654c1e209ef9e093f4



Patch:
diff --git a/Changelog.md b/Changelog.md
--- a/Changelog.md
+++ b/Changelog.md
@@ -2,6 +2,9 @@
 
 Below is a complete listing of changes for each revision of HighLine.
 
+### 2.0.0-develop.14 / 2017-11-21
+* PR #222 / I #221 - Fix inconsistent behaviour when using agree with readline (@abinoam, @ailisp)
+
 ### 2.0.0-develop.13 / 2017-11-05
 * PR #219 - Make possible to use a callable as response (@abinoam)
 
diff --git a/lib/highline.rb b/lib/highline.rb
--- a/lib/highline.rb
+++ b/lib/highline.rb
@@ -196,6 +196,7 @@ def agree(yes_or_no_question, character = nil)
       q.responses[:not_valid]    = 'Please enter "yes" or "no".'
       q.responses[:ask_on_error] = :question
       q.character                = character
+      q.completion               = %w[yes no]
 
       yield q if block_given?
     end
diff --git a/lib/highline/question.rb b/lib/highline/question.rb
--- a/lib/highline/question.rb
+++ b/lib/highline/question.rb
@@ -575,7 +575,7 @@ def ask_on_error_msg
     # @param highline [HighLine] context
     # @return [void]
     def show_question(highline)
-      highline.say(self) unless readline && (echo == true && !limit)
+      highline.say(self)
     end
 
     # Returns an echo string that is adequate for this Question settings.
diff --git a/lib/highline/terminal.rb b/lib/highline/terminal.rb
--- a/lib/highline/terminal.rb
+++ b/lib/highline/terminal.rb
@@ -98,9 +98,7 @@ def get_line(question, highline)
     def get_line_with_readline(question, highline)
       require "readline" # load only if needed
 
-      question_string = highline.render_statement(question)
-
-      raw_answer = readline_read(question_string, question)
+      raw_answer = readline_read(question)
 
       if !raw_answer && highline.track_eof?
         raise EOFError, "The input stream is exhausted."
@@ -113,7 +111,7 @@ def get_line_with_readline(question, highline)
     # @param prompt [String] Readline prompt
     # @param question [HighLine::Question] question from where to get
     #   autocomplete candidate strings
-    def readline_read(prompt, question)
+    def readline_read(question)
       # prep auto-completion
       unless question.selection.empty?
         Readline.completion_proc = lambda do |str|
@@ -126,7 +124,7 @@ def readline_read(prompt, question)
       $VERBOSE    = nil
 
       raw_answer  = run_preserving_stty do
-        Readline.readline(prompt, true)
+        Readline.readline("", true)
       end
 
       $VERBOSE = old_verbose
diff --git a/lib/highline/version.rb b/lib/highline/version.rb
--- a/lib/highline/version.rb
+++ b/lib/highline/version.rb
@@ -2,5 +2,5 @@
 
 class HighLine
   # The version of the installed library.
-  VERSION = "2.0.0-develop.13".freeze
+  VERSION = "2.0.0-develop.14".freeze
 end


Test patch:
diff --git a/test/acceptance/at_readline_agree.rb b/test/acceptance/at_readline_agree.rb
new file mode 100644
--- /dev/null
+++ b/test/acceptance/at_readline_agree.rb
@@ -0,0 +1,18 @@
+# coding: utf-8
+
+require_relative "acceptance_test"
+
+HighLine::AcceptanceTest.check do |t|
+  t.desc =
+    "This step checks if the readline works well with agree.\n" \
+    "You should press <tab> and readline should give the default " \
+    "(yes/no) options to autocomplete."
+
+  t.action = proc do
+    answer = agree("Do you agree?") { |q| q.readline = true }
+    puts "You've entered -> #{answer} <-"
+  end
+
+  t.question =
+    "Did HighLine#agree worked well using question.readline = true (y/n)? "
+end