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.

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

Conversation

monkeyWzr
Copy link
Contributor

Fix #754. Insert completion_append_character only when there is no more possible completion

@@ -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

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 👍

@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

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

@tompng
Copy link
Member

tompng commented Oct 22, 2024

The change looks good. Could you resolve conflicts?

Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

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

Thank you 👍

@tompng tompng merged commit 01872fd into ruby:master Oct 24, 2024
40 checks passed
@ima1zumi ima1zumi added the bug Something isn't working label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Unexpected completion_append_character when tab-completing
3 participants