Skip to content
This repository has been archived by the owner on Jul 30, 2020. It is now read-only.

More options for completion filtering #786

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ddovod
Copy link

@ddovod ddovod commented Aug 28, 2018

Here's the first try to add more options for completion filtering

@ddovod
Copy link
Author

ddovod commented Aug 28, 2018

For mecaseInsensitivePrefixMatcher works better than others so far. Will try another approaches

Copy link
Owner

@jacobdufault jacobdufault left a comment

Choose a reason for hiding this comment

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

Happy to merge when implementation is finalized

class CompletionMatcher {
public:
// virtual ~CompletionMatcher() = default; // don't know why but it crashed if I uncomment this like, investigating
virtual int Match(std::string_view text) = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add some timing to see if the virtual function call is negatively impacting how long it takes to filter completion results?

Copy link
Author

Choose a reason for hiding this comment

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

I'll check it a bit later, no problem. I'm sure that virtual function call will not spend much more time than plain function call, especially comparing to all that fuzzy-matching stuff = )

src/config.h Outdated
@@ -156,6 +156,9 @@ struct Config {
// For example, to hide all files in a /CACHE/ folder, use ".*/CACHE/.*"
std::vector<std::string> includeBlacklist;
std::vector<std::string> includeWhitelist;

//
Copy link
Owner

Choose a reason for hiding this comment

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

Make sure to document all possible options here

Copy link
Author

Choose a reason for hiding this comment

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

Done


class CompletionMatcher {
public:
// virtual ~CompletionMatcher() = default; // don't know why but it crashed if I uncomment this like, investigating
Copy link
Owner

Choose a reason for hiding this comment

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

Strange, but this should have a virtual dtor for correct behavior (also, the = default should probably go in a .cc file)

Copy link
Contributor

Choose a reason for hiding this comment

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

The = default part can behave differently if moved to the .cc file.
https://accu.org/index.php/journals/2379

Copy link
Author

Choose a reason for hiding this comment

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

No idea whats happened but now it works fine. Probably some weird stuff on my side


namespace {
bool StartsWith(std::string_view text, std::string_view prefix) {
return text.find(prefix) == 0;
Copy link
Owner

Choose a reason for hiding this comment

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

There may be a function in utils.h to do this

Copy link
Author

Choose a reason for hiding this comment

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

Done. And also moved StartsWithIgnoreCase to utils

@ddovod
Copy link
Author

ddovod commented Sep 1, 2018

Hi guys. Sorry, was too busy all the week

@ddovod
Copy link
Author

ddovod commented Sep 1, 2018

Also which version of clang-format do you use to format code? Mine is 6.0.0 and I'm getting some diff on untouched files when formatting all src directory

@ddovod
Copy link
Author

ddovod commented Sep 1, 2018

I'm experiencing some weird stuff. Basically I've found out that cquery fuzzy matching algorithm works good enough. The thing is completion results are reordered, probably without respecting score_ parameter.
This is what I see with cquery matcher:
screenshot_2018-09-02_00-25-39

But if I add this piece of code

  int i = 0;
  for (auto& el : items) {
      el.label = std::to_string(++i) + " : " + el.label;
  }

right before finalize() is called in FilterAndSortCompletionResponse, I see this:
screenshot_2018-09-02_00-28-16

Looks like somebody sorts candidates after they was sorted once. Could you please reproduce it on your side?

@jacobdufault
Copy link
Owner

Looks like somebody sorts candidates after they was sorted once. Could you please reproduce it on your side?

If cquery is not explicitly sorting then that is almost certainly your completion client not honoring the priority.

@ddovod
Copy link
Author

ddovod commented Sep 25, 2018

Damn, there're soo many packages between me and code I'm trying to edit) maybe you can help me a bit? I just need to understand who is sorting candidates (lsp-mode, emacs-cquery, company-lsp). Maybe you can point me to some elisp code which receives completion candidates from cquery and gives it to some lsp-mode functions?

@jacobdufault
Copy link
Owner

Sorry, I'm not very familiar with how emacs LSP works.

@ddovod
Copy link
Author

ddovod commented Sep 25, 2018

Okay, no problem, I'll try to dig into it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants