Refactor completion #647
Refactor completion #647
Conversation
| else | ||
| complete(result) | ||
| if !@config.disable_completion | ||
| process_insert(force: true) |
There was a problem hiding this comment.
For my learning: Why is force: true needed?
There was a problem hiding this comment.
If you type "1." and then paste "0.\t\t" to the terminal, "0." is inserted to @continuous_insertion_buffer (because in_pasting? is true).
Without this process_insert(force: true), 0. part will be ignored.
irb(main):001> 1.floor
1.abs
1.floor[selected]
1.ceil
We need to force insert "0." into the buffer before calculating completion list.
irb(main):001> 1.0.ceil
1.0.floor
1.0.ceil[selected]
1.0.round
There was a problem hiding this comment.
Thank you for your information!
But I could not paste "0./t/t" in my environment. Can you paste it?
I got this:
irb(main):001> 1.__id__
1.__send__[selected]
1.equal?
There was a problem hiding this comment.
It was broken in the last commit...
Fixed and I added test for it
lib/reline/line_editor.rb
Outdated
| list = call_completion_proc | ||
| return false unless list.is_a?(Array) | ||
|
|
||
| candidates = list.select{ |item| item.start_with?(target) } |
There was a problem hiding this comment.
For my learning: I'm not sure why select is needed. Is it possible to create a list that does not start with target?
There was a problem hiding this comment.
Is it possible to create a list that does not start with target?
Yes, it's possible by this code
require 'reline'
Reline.autocompletion = true
Reline.completion_proc = ->*{['abc','def','ghi','jkl']}
p Reline.readmultiline('>'){true}I'll remove this select because older reline does not reject candidates that does not start with target.
> x
abc
def
ghi
jkl
There was a problem hiding this comment.
The old code was doing this select in L886
private def move_completed_list(list, direction)
...
@completion_journey_data = CompletionJourneyData.new(
preposing, postposing,
[target] + list.select{ |item| item.start_with?(target) }, 0)The old behavior (renders candidates that does not start from target, but disappears after pressing tab) seems buggy.
tompng
force-pushed
the
autocompletion_jouney_refactor
branch
from
0666479
to
ebeb56e
Compare
| @@ -537,6 +536,8 @@ def rerender | |||
| end | |||
|
|
|||
| class DialogProcScope | |||
| CompletionJourneyData = Struct.new(:preposing, :postposing, :list, :pointer) | |||
There was a problem hiding this comment.
How about separating list[0] from list?
CompletionJourneyData only uses by completion_journey_data. I don't think it constitutes a breaking change. By separating this, there will be no need to drop it at the caller side.
Moving a constant may also be considered a breaking change, but it doesn't seem to be used by anyone.
There was a problem hiding this comment.
I think it's reasonable. It will make DEFAULT_DIALOG_PROC_AUTOCOMPLETE simpler.
However, I think removing list[0] will make completion_journey_data not a journey_data thing.
completion-journey includes target as pointer=0. Renaming it to something like completion_list_for_completion_dialog will break IRB so this is not possible now.
There was a problem hiding this comment.
Thank you for the explanation. I understand.
tompng
force-pushed
the
autocompletion_jouney_refactor
branch
from
ebeb56e
to
4c645a4
Compare
tompng
force-pushed
the
autocompletion_jouney_refactor
branch
from
4c645a4
to
0ea822a
Compare
(ruby/reline#647) * Refactor completion: split autocompletion and tabcompletion logic and state * Move completion candidate listup logic from dialog proc to LineEditor ruby/reline@c3c09ac9c2
(ruby/reline#647) * Refactor completion: split autocompletion and tabcompletion logic and state * Move completion candidate listup logic from dialog proc to LineEditor ruby/reline@c3c09ac9c2
Fixes #611 and #621
Split logic and state of autocompletion and tab completion
Before
Reline has tab complete and autocomplete.
Tab complete uses
Autocomplete uses
It's a separate feature but uses same instance variable
@completion_statethat making hard to fix some bugs.After
Tab complete uses
Autocomplete uses
State is separated. Logic is also separated. Now we can remove redundant proc calls and fix #611 #621
completion_journey_data
completion_journey_datais an API for dialog proc, defined in DialogProcScope. We need to keep compatibility.It was an instance variable but changed to a method that calculates from
@completion_journey_state.DEFAULT_DIALOG_PROC_AUTOCOMPLETE
Move completion candidate list up logic to LineEditor
All completion logic is in LineEditor. DEFAULT_DIALOG_PROC_AUTOCOMPLETE only needs to create dialog content from completion_journey_data