Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

append completion_append_character only when continous completion is … #764

Merged
merged 4 commits into from
Oct 24, 2024
Merged

append completion_append_character only when continous completion is … #764

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d535d29
append completion_append_character only when continous completion is …
monkeyWzr Oct 14, 2024
224f535
refactoring
monkeyWzr Oct 19, 2024
05f1c74
remove debug puts
monkeyWzr Oct 19, 2024
6495aaf
Merge branch 'master' into append_completion_append_character_only_wh…
monkeyWzr Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions lib/reline/line_editor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,9 @@ def editing_mode
end
result
}
[target, preposing, completed, postposing]

continuable = list.count > 1
[target, preposing, completed, postposing, continuable]
end

private def perform_completion(list, just_show_list)
Expand Down Expand Up @@ -866,7 +868,7 @@ def editing_mode
@completion_state = CompletionState::PERFECT_MATCH
end
return if result.nil?
target, preposing, completed, postposing = result
target, preposing, completed, postposing, continuable = result
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuable in calculated twice.
One in complete_internal_proc(list.count > 1) and other in this method

if list.one?
  @completion_state = CompletionState::PERFECT_MATCH

How about using the condition @completion_state != CompletionState::PERFECT_MATCH instead of continuable, or set a local variable inside if list.one?

if list.one?
  @completion_state = CompletionState::PERFECT_MATCH
  perfect_matched_and_need_to_perform_append_char = true # FIXME: shorter name
end

Copy link
Contributor Author

@monkeyWzr monkeyWzr Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion! But consider this case when list is something like:

['foo', 'foo_bar']

When we input foo_[tab], it will be completed to foo_bar. A completion char should be appended but currently this case will be marked as MENU_WITH_PERFECT_MATCH. That's why I separately check list.count instead of inside if list.one? case.

After reconsideration, I think maybe it's OK to treat the above case as a PERFECT_MATCH?

(For demonstration I made the changes anyway. Looking forward to you comment! )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treating it as PERFECT_MATCH makes sense to me 👍

return if completed.nil?
if target <= completed and (@completion_state == CompletionState::COMPLETION)
if list.include?(completed)
Expand All @@ -881,9 +883,10 @@ def editing_mode
@completion_state = CompletionState::MENU
perform_completion(list, true) if @config.show_all_if_ambiguous
end
if not just_show_list and target < completed
@buffer_of_lines[@line_index] = (preposing + completed + completion_append_character.to_s + postposing).split("\n")[@line_index] || String.new(encoding: @encoding)
line_to_pointer = (preposing + completed + completion_append_character.to_s).split("\n")[@line_index] || String.new(encoding: @encoding)
unless just_show_list
_completion_append_character = continuable ? '' : completion_append_character.to_s
Copy link
Member

@tompng tompng Oct 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Underscore prefixed local variable represents unused variable. Ruby itself won't show unused variable warning if the unused variable starts with _. If it is used, it is better not start with _.
How about just append_character or completed += completion_append_character.to_s unless continuable

@buffer_of_lines[@line_index] = (preposing + completed + _completion_append_character + postposing).split("\n")[@line_index] || String.new(encoding: @encoding)
line_to_pointer = (preposing + completed + _completion_append_character).split("\n")[@line_index] || String.new(encoding: @encoding)
@byte_pointer = line_to_pointer.bytesize
end
end
Expand Down
Loading