Sort completion candidates

[kryzhovnik]

Method completion candidates are returned in method table order, which is not even definition order and looks random in the completion dialog. With an ActiveRecord model, u.email_address is listed below all its generated variants:

u.email_address_before_last_save
u.email_address_change_to_be_saved
u.email_address=
u.email_address_was
...
u.email_address

Constants, ivars and symbols are already sorted here, and the same problem was fixed for the readline menu in ruby/reline#96 and for part of RegexpCompletor in ruby/irb#379 (ruby/irb#349). This applies the same policy to all candidate kinds: sort once at the end of Result#completion_candidates and drop the per-branch sorts.

One exception: candidates starting with _ go last, otherwise completing u. would begin with __id__, __send__, _run_commit_callbacks and similar internals. The check uses the full method name, not the part after the typed prefix, so u.email still lists email_address before emails.

Side effect of sorting everything alphabetically: @ completion no longer groups ivars before class variables, and Foo:: now mixes constants and methods alphabetically. Can move the sort into the individual branches instead if that grouping is worth keeping.

[tompng]

I think some candidates are worth sorted in groups, so sorting at the end is not always the best.
Sorting by leading-underscore-last looks good. However this is only needed in method call with dot(in [:call, name, type, self_call]).

Examples of grouped sort:

Constants first, reserved words last(BEGIN or END, rarely used)

scope.constants.sort | RESERVED_WORDS

Instance variables first, class variables last when a single @ is typed

# Current
name == '@' ? ivars + scope.class_variables : ivars
# Sorted
name == '@' ? ivars.sort + scope.class_variables.sort : ivars

Local variables first, methods next, reserved words last

# Current
scope.self_type.all_methods.map(&:to_s) | scope.local_variables | RESERVED_WORDS
# Sorted
scope.local_variables.sort | scope.self_type.all_methods.map(&:to_s).sort | RESERVED_WORDS

[kryzhovnik]

Updated in 93ca6a1 based on your feedback.
Sorting now preserves candidate groups, and underscore-last is limited to dotted method calls

[tompng]

Thank you.
Instead of sort with condition if @analyze_result.first == :call, can you move the sort to in [:call, name, type, self_call] part?

in [:call, name, type, self_call]
  methods = (self_call ? type.all_methods : type.methods).map(&:to_s) - HIDDEN_METHODS
  methods.sort_by { ... } # sort here

If the sort is moved, candidates.filter_map part will be simple, and doesn't need change in this PR.

[kryzhovnik]

Done. One note: the sort now runs before the rescue EncodingError part, where start_with?('_') can raise, so I used 1[0] == '' instead

[tompng]

Looks great! Thank you 👍

[kryzhovnik]

Thank you for your patience