Fix tab completion appending quote (#782)

Author: tompng

lib/reline/line_editor.rb

@@ -578,8 +578,9 @@ def context
       @context
     end
 
-    def retrieve_completion_block(set_completion_quote_character = false)
-      @line_editor.retrieve_completion_block(set_completion_quote_character)
+    def retrieve_completion_block(_unused = false)
+      preposing, target, postposing, _quote = @line_editor.retrieve_completion_block
+      [preposing, target, postposing]
     end
 
     def call_completion_proc_with_checking_args(pre, target, post)
@@ -826,8 +827,7 @@ def editing_mode
     end.uniq
   end
 
-  private def perform_completion(list)
-    preposing, target, postposing = retrieve_completion_block
+  private def perform_completion(preposing, target, postposing, quote, list)
     candidates = filter_normalize_candidates(target, list)
 
     case @completion_state
@@ -851,7 +851,7 @@ def editing_mode
     append_character = ''
     if candidates.include?(completed)
       if candidates.one?
-        append_character = completion_append_character.to_s
+        append_character = quote || completion_append_character.to_s
         @completion_state = CompletionState::PERFECT_MATCH
       elsif @config.show_all_if_ambiguous
         menu(candidates)
@@ -895,8 +895,8 @@ def dialog_proc_scope_completion_journey_data
   end
 
   private def retrieve_completion_journey_state
-    preposing, target, postposing = retrieve_completion_block
-    list = call_completion_proc
+    preposing, target, postposing, quote = retrieve_completion_block
+    list = call_completion_proc(preposing, target, postposing, quote)
     return unless list.is_a?(Array)
 
     candidates = list.select{ |item| item.start_with?(target) }
@@ -1146,9 +1146,8 @@ def scroll_into_view
     end
   end
 
-  def call_completion_proc
-    result = retrieve_completion_block(true)
-    pre, target, post = result
+  def call_completion_proc(pre, target, post, quote)
+    Reline.core.instance_variable_set(:@completion_quote_character, quote)
     result = call_completion_proc_with_checking_args(pre, target, post)
     Reline.core.instance_variable_set(:@completion_quote_character, nil)
     result
@@ -1224,11 +1223,12 @@ def set_current_lines(lines, byte_pointer = nil, line_index = 0)
     process_auto_indent
   end
 
-  def retrieve_completion_block(set_completion_quote_character = false)
+  def retrieve_completion_block
     quote_characters = Reline.completer_quote_characters
     before = current_line.byteslice(0, @byte_pointer).grapheme_clusters
     quote = nil
-    unless quote_characters.empty?
+    # Calcualte closing quote when cursor is at the end of the line
+    if current_line.bytesize == @byte_pointer && !quote_characters.empty?
       escaped = false
       before.each do |c|
         if escaped
@@ -1243,26 +1243,20 @@ def retrieve_completion_block(set_completion_quote_character = false)
         end
       end
     end
+
     word_break_characters = quote_characters + Reline.completer_word_break_characters
     break_index = before.rindex { |c| word_break_characters.include?(c) || quote_characters.include?(c) } || -1
     preposing = before.take(break_index + 1).join
     target = before.drop(break_index + 1).join
     postposing = current_line.byteslice(@byte_pointer, current_line.bytesize - @byte_pointer)
-    if target
-      if set_completion_quote_character and quote
-        Reline.core.instance_variable_set(:@completion_quote_character, quote)
-        insert_text(quote) # FIXME: should not be here
-        target += quote
-      end
-    end
     lines = whole_lines
     if @line_index > 0
       preposing = lines[0..(@line_index - 1)].join("\n") + "\n" + preposing
     end
     if (lines.size - 1) > @line_index
       postposing = postposing + "\n" + lines[(@line_index + 1)..-1].join("\n")
     end
-    [preposing.encode(encoding), target.encode(encoding), postposing.encode(encoding)]
+    [preposing.encode(encoding), target.encode(encoding), postposing.encode(encoding), quote&.encode(encoding)]
   end
 
   def confirm_multiline_termination
@@ -1393,10 +1387,11 @@ def finish
       @completion_occurs = move_completed_list(:down)
     else
       @completion_journey_state = nil
-      result = call_completion_proc
+      pre, target, post, quote = retrieve_completion_block
+      result = call_completion_proc(pre, target, post, quote)
       if result.is_a?(Array)
         @completion_occurs = true
-        perform_completion(result)
+        perform_completion(pre, target, post, quote, result)
       end
     end
   end
@@ -1860,9 +1855,9 @@ def finish
     if current_line.empty? or @byte_pointer < current_line.bytesize
       em_delete(key)
     elsif !@config.autocompletion # show completed list
-      result = call_completion_proc
+      pre, target, post, quote = retrieve_completion_block
+      result = call_completion_proc(pre, target, post, quote)
       if result.is_a?(Array)
-        _preposing, target = retrieve_completion_block
         candidates = filter_normalize_candidates(target, result)
         menu(candidates)
       end

test/reline/helper.rb

@@ -135,6 +135,13 @@ def input_raw_keys(input, convert = true)
     end
   end
 
+  def set_line_around_cursor(before, after)
+    input_keys("\C-a\C-k")
+    input_keys(after)
+    input_keys("\C-a")
+    input_keys(before)
+  end
+
   def assert_line_around_cursor(before, after)
     before = convert_str(before)
     after = convert_str(after)

test/reline/test_key_actor_emacs.rb

@@ -933,6 +933,30 @@ def test_completion_append_character
     assert_line_around_cursor('foo_fooX foo_barX', '')
   end
 
+  def test_completion_with_quote_append
+    @line_editor.completion_proc = proc { |word|
+      %w[foo bar baz].select { |s| s.start_with? word }
+    }
+    set_line_around_cursor('x = "b', '')
+    input_keys("\C-i", false)
+    assert_line_around_cursor('x = "ba', '')
+    set_line_around_cursor('x = "f', ' ')
+    input_keys("\C-i", false)
+    assert_line_around_cursor('x = "foo', ' ')
+    set_line_around_cursor("x = 'f", '')
+    input_keys("\C-i", false)
+    assert_line_around_cursor("x = 'foo'", '')
+    set_line_around_cursor('"a "f', '')
+    input_keys("\C-i", false)
+    assert_line_around_cursor('"a "foo', '')
+    set_line_around_cursor('"a\\" "f', '')
+    input_keys("\C-i", false)
+    assert_line_around_cursor('"a\\" "foo', '')
+    set_line_around_cursor('"a" "f', '')
+    input_keys("\C-i", false)
+    assert_line_around_cursor('"a" "foo"', '')
+  end
+
   def test_completion_with_completion_ignore_case
     @line_editor.completion_proc = proc { |word|
       %w{

test/reline/test_line_editor.rb

@@ -15,16 +15,12 @@ def retrieve_completion_block(lines, line_index, byte_pointer)
       @line_editor.instance_variable_set(:@buffer_of_lines, lines)
       @line_editor.instance_variable_set(:@line_index, line_index)
       @line_editor.instance_variable_set(:@byte_pointer, byte_pointer)
-      @line_editor.retrieve_completion_block(false)
+      @line_editor.retrieve_completion_block
     end
 
     def retrieve_completion_quote(line)
-      retrieve_completion_block([line], 0, line.bytesize)
-      _, target = @line_editor.retrieve_completion_block(false)
-      _, target2 = @line_editor.retrieve_completion_block(true)
-      # This is a hack to get the quoted character.
-      # retrieve_completion_block should be refactored to return the quoted character.
-      target2.chars.last if target2 != target
+      _, _, _, quote = retrieve_completion_block([line], 0, line.bytesize)
+      quote
     end
 
     def teardown
@@ -35,20 +31,20 @@ def teardown
     def test_retrieve_completion_block
       Reline.completer_word_break_characters = ' ([{'
       Reline.completer_quote_characters = ''
-      assert_equal(['', '', 'foo'], retrieve_completion_block(['foo'], 0, 0))
-      assert_equal(['', 'f', 'oo'], retrieve_completion_block(['foo'], 0, 1))
-      assert_equal(['foo ', 'ba', 'r baz'], retrieve_completion_block(['foo bar baz'], 0, 6))
-      assert_equal(['foo([', 'b', 'ar])baz'], retrieve_completion_block(['foo([bar])baz'], 0, 6))
-      assert_equal(['foo([{', '', '}])baz'], retrieve_completion_block(['foo([{}])baz'], 0, 6))
-      assert_equal(["abc\nfoo ", 'ba', "r baz\ndef"], retrieve_completion_block(['abc', 'foo bar baz', 'def'], 1, 6))
+      assert_equal(['', '', 'foo', nil], retrieve_completion_block(['foo'], 0, 0))
+      assert_equal(['', 'f', 'oo', nil], retrieve_completion_block(['foo'], 0, 1))
+      assert_equal(['foo ', 'ba', 'r baz', nil], retrieve_completion_block(['foo bar baz'], 0, 6))
+      assert_equal(['foo([', 'b', 'ar])baz', nil], retrieve_completion_block(['foo([bar])baz'], 0, 6))
+      assert_equal(['foo([{', '', '}])baz', nil], retrieve_completion_block(['foo([{}])baz'], 0, 6))
+      assert_equal(["abc\nfoo ", 'ba', "r baz\ndef", nil], retrieve_completion_block(['abc', 'foo bar baz', 'def'], 1, 6))
     end
 
     def test_retrieve_completion_block_with_quote_characters
       Reline.completer_word_break_characters = ' ([{'
       Reline.completer_quote_characters = ''
-      assert_equal(['"" ', '"wo', 'rd'], retrieve_completion_block(['"" "word'], 0, 6))
+      assert_equal(['"" ', '"wo', 'rd', nil], retrieve_completion_block(['"" "word'], 0, 6))
       Reline.completer_quote_characters = '"'
-      assert_equal(['"" "', 'wo', 'rd'], retrieve_completion_block(['"" "word'], 0, 6))
+      assert_equal(['"" "', 'wo', 'rd', nil], retrieve_completion_block(['"" "word'], 0, 6))
     end
 
     def test_retrieve_completion_quote

test/reline/test_string_processing.rb

@@ -27,42 +27,20 @@ def test_calculate_width_with_escape_sequence
   def test_completion_proc_with_preposing_and_postposing
     buf = ['def hoge', '  puts :aaa', 'end']
 
-    @line_editor.instance_variable_set(:@is_multiline, true)
-    @line_editor.instance_variable_set(:@buffer_of_lines, buf)
-    @line_editor.instance_variable_set(:@byte_pointer, 3)
-    @line_editor.instance_variable_set(:@line_index, 1)
-    @line_editor.instance_variable_set(:@completion_proc, proc { |target|
-      assert_equal('p', target)
-    })
-    @line_editor.__send__(:call_completion_proc)
-
     @line_editor.instance_variable_set(:@is_multiline, true)
     @line_editor.instance_variable_set(:@buffer_of_lines, buf)
     @line_editor.instance_variable_set(:@byte_pointer, 6)
     @line_editor.instance_variable_set(:@line_index, 1)
+    completion_proc_called = false
     @line_editor.instance_variable_set(:@completion_proc, proc { |target, pre, post|
       assert_equal('puts', target)
       assert_equal("def hoge\n  ", pre)
       assert_equal(" :aaa\nend", post)
+      completion_proc_called = true
     })
-    @line_editor.__send__(:call_completion_proc)
 
-    @line_editor.instance_variable_set(:@byte_pointer, 6)
-    @line_editor.instance_variable_set(:@line_index, 0)
-    @line_editor.instance_variable_set(:@completion_proc, proc { |target, pre, post|
-      assert_equal('ho', target)
-      assert_equal('def ', pre)
-      assert_equal("ge\n  puts :aaa\nend", post)
-    })
-    @line_editor.__send__(:call_completion_proc)
-
-    @line_editor.instance_variable_set(:@byte_pointer, 1)
-    @line_editor.instance_variable_set(:@line_index, 2)
-    @line_editor.instance_variable_set(:@completion_proc, proc { |target, pre, post|
-      assert_equal('e', target)
-      assert_equal("def hoge\n  puts :aaa\n", pre)
-      assert_equal('nd', post)
-    })
-    @line_editor.__send__(:call_completion_proc)
+    assert_equal(["def hoge\n  ", 'puts', " :aaa\nend", nil], @line_editor.retrieve_completion_block)
+    @line_editor.__send__(:call_completion_proc, "def hoge\n  ", 'puts', " :aaa\nend", nil)
+    assert(completion_proc_called)
   end
 end