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

fuzzy completions #308

Merged
merged 4 commits into from
Apr 6, 2020
Merged

fuzzy completions #308

merged 4 commits into from
Apr 6, 2020

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Apr 1, 2020

should work once JuliaRegistries/General#11930 gets merged.
After that I will refactor the test cases a bit and update precompile stmts.

- configurable to switch back to the previous strict match (REPL-based) 
completions
@aviatesk
Copy link
Member Author

aviatesk commented Apr 1, 2020

examples:

  • image
  • image

@aviatesk aviatesk force-pushed the avi/fuzzycompletions branch from faaaab2 to 9d2487a Compare April 1, 2020 13:11
@aviatesk aviatesk closed this Apr 4, 2020
@aviatesk aviatesk reopened this Apr 4, 2020
@aviatesk aviatesk force-pushed the avi/fuzzycompletions branch from 9d2487a to e2021ba Compare April 4, 2020 12:43
@codecov
Copy link

codecov bot commented Apr 4, 2020

Codecov Report

Merging #308 into master will decrease coverage by 0.04%.
The diff coverage is 92.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
- Coverage   54.92%   54.88%   -0.05%     
==========================================
  Files          42       42              
  Lines        2933     2602     -331     
==========================================
- Hits         1611     1428     -183     
+ Misses       1322     1174     -148     
Impacted Files Coverage Δ
src/debugger/repl.jl 0.00% <ø> (ø)
src/repl.jl 3.87% <ø> (-0.30%) ⬇️
src/completions.jl 74.03% <68.33%> (+4.41%) ⬆️
src/precompile.jl 97.18% <97.17%> (+29.20%) ⬆️
src/display/markdown.jl 55.81% <0.00%> (-10.86%) ⬇️
src/debugger/debugger.jl 33.33% <0.00%> (-6.67%) ⬇️
src/outline.jl 70.00% <0.00%> (-2.42%) ⬇️
src/datatip.jl 74.24% <0.00%> (-2.33%) ⬇️
src/comm.jl 29.09% <0.00%> (-1.68%) ⬇️
src/display/errors.jl 37.50% <0.00%> (-1.64%) ⬇️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b4a09a...97220da. Read the comment docs.

- reflect fuzzy completion adapter addition
- revert back to one-liner statement for minor visibility improvement
@aviatesk
Copy link
Member Author

aviatesk commented Apr 4, 2020

Alright, I updated precompile statements. It would be nice if I could have a feedback on this :)

src/completions.jl Show resolved Hide resolved
)
end

function fuzzycompletionadapter(
Copy link
Member

Choose a reason for hiding this comment

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

This seems fairly similar to the replcompletionadapter -- maybe we could have an is_fuzzy kwarg and do something like

repl_provider = is_fuzzy ? FuzzyCompletions : REPLCompletions
repl_provider.completions(...)

plus have an if block for the different filter methods below?

Copy link
Member Author

@aviatesk aviatesk Apr 6, 2020

Choose a reason for hiding this comment

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

hm, then the code after repl_provider used would be really type-instable (I confirmed that Julia 1.4 can't infer the type of repl_provider.completions, for example)

I haven't taken any actual benchmark, but I would like to endure the verbose code for performance here.

Copy link
Member

Choose a reason for hiding this comment

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

Hm fair enough then. I don't think the dynamic dispatch would make much of a difference, but let's just keep this as-is then.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think the dynamic dispatch would make much of a difference

yeah maybe. I may be too nervous about dynamic dispatches..
I will refactor them in the future when I get confidence that dynamic dispatches here don't matter.

@pfitzseb pfitzseb merged commit 8a3007d into master Apr 6, 2020
@aviatesk aviatesk deleted the avi/fuzzycompletions branch April 6, 2020 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants