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

Introduce completion candidates limit, minor fixes #772

Closed
wants to merge 4 commits into from
Closed

Introduce completion candidates limit, minor fixes #772

wants to merge 4 commits into from

Conversation

geraldus
Copy link
Contributor

Don't merge yet, I didn't tested this stuff well and want to play with this before merge. Please review.

@gracjan
Copy link
Contributor

gracjan commented Jul 23, 2015

Please use defconst instead of defcustom. After enough of testing we can arrive at a constant that will make almost everybody happy. If somebody detects a use case where that number fails then we would like to receive a pull request so that everybody benefits.

@geraldus
Copy link
Contributor Author

@gracjan ok, I think a hundred completions is quite comfortable value.

@geraldus
Copy link
Contributor Author

@gracjan I have no idea about this

In toplevel form:
haskell-sort-imports.el:128:1:Error: the function `copy-list' is not known to be defined.
make: *** [haskell-sort-imports.elc] Error 1

@gracjan
Copy link
Contributor

gracjan commented Jul 23, 2015

Use cl-copy-list and do (require 'cl-lib) in the beginning of the file.

@geraldus
Copy link
Contributor Author

Ok, but I didn't touch haskell-sort-imports.el myself. Will create a separate PR for this issue.

@gracjan
Copy link
Contributor

gracjan commented Jul 23, 2015

I've fixed copy-list problem, just rebase over current master and you should be fine.

@geraldus
Copy link
Contributor Author

Cool, thanks (:

@geraldus
Copy link
Contributor Author

I did some quick tests, with hundred candidates limit dabbrev completions are still quite slow for short prefixes (2-4 chars), but performance increases for long prefixes. The most comfortable limit value for short prefixes for me is about 10-20 candidates.

I got some insight: what about to set candidates limit on the fly depending of prefix length, i.e. set smaller limit for short prefixes and increase it for long prefixes?

@gracjan
Copy link
Contributor

gracjan commented Jul 24, 2015

I've never heard about dabbrev performance issues. Are you sure we are using it right?

@geraldus
Copy link
Contributor Author

@gracjan, maybe I've took wrong words, I'm talking about issue described in #741 (here is a demo): while I typing nothing appears until I delay, note the "Scanning for dabbrevs" message in echo area.

@gracjan
Copy link
Contributor

gracjan commented Jul 24, 2015

@geraldus: Code looks good. It would be good to find somebody besides me and you to play with this before merge.

@geraldus
Copy link
Contributor Author

Surely, I want to make some more changes, e.g. #776.

@gracjan
Copy link
Contributor

gracjan commented Jul 26, 2015

@geraldus: I was thinking about this a bit more. Questions:

  1. Is that the limit is a problem only with dabbrev?
  2. Is there a way to DO NOTHING about dabbrev in haskell-mode and leave this task to company-dabbrev-mode?

@geraldus
Copy link
Contributor Author

@gracjan answers :D

  1. No, it is not. ikirill also reported some issues about REPL completions with big projects, but in common case limit issue is more related to dabbrev/
  2. I think that it is possible and I'm going to check it. When I implemented new synchronous completion mechanism I was not aware of dabbrev customizations in haskell-mode and that company have dabbrev backend out of box also. I will push more updates soon.

Take into account completion candidates limit when searching
completions using REPL or DABBREV facility.

Also clean up completion list from REPL: currently
`haskell-process-get-repl-completions` returns unused part of line as
first item of list (e.g. it will be "map " for line "map co", and empty
string for "ma" line).
@geraldus
Copy link
Contributor Author

@gracjan I've disabled dabbrev temporarily to test existing behaviour. I've already noticed that now completion candidates in comments are case insensitive. I need to find and look at mentioned dabbrev customizations in haskell-mode.

company-mode uses company-<dabbrev-code> backend when searching completions for identifiers and company-dabbrev when typing inside comments (and it is case insensitive). What do you think about this? Should user make global dabbrev customizations for this case if he (she) want case sensitive completions?

@gracjan
Copy link
Contributor

gracjan commented Aug 8, 2015

@geraldus: I've found these custom variables:

  • dabbrev-case-distinction
  • dabbrev-case-fold-search
  • dabbrev-case-replace

We can set the buffer-local so that case sensitivity is kept. Would that work for you?

@geraldus
Copy link
Contributor Author

geraldus commented Aug 8, 2015

@gracjan I will be able to test this in about one or two days.

@gracjan
Copy link
Contributor

gracjan commented Sep 4, 2015

@geraldus: hi, what do we do with this branch?

@geraldus
Copy link
Contributor Author

geraldus commented Sep 4, 2015

There were made so many changes, I think we can close this for now, I'm still busy, apologize :(

@gracjan
Copy link
Contributor

gracjan commented Sep 4, 2015

No problem. Take care of your stuff, return when you are ready. haskell-mode will be around.

@gracjan gracjan closed this Sep 4, 2015
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