FelipeLema / emacs-counsel-gtags

GNU Global with ivy completion
22 stars 5 forks source link

counsel-gtags-find-definition does not filter symbols #8

Closed xuhdev closed 5 years ago

xuhdev commented 5 years ago

In an empty directory, create the following file a.c:

int the_first_func() {
  return 0;
}

int the_second_func(int x) {
  return 0;
}

int the_third_func(int x) {
  return 0;
}

Then run gtags. global -c now outputs:

the_first_func
the_second_func
the_third_func

Now open a.c in Emacs and call counsel-gtags-find-definition. Type the_ in the minibuffer, and now the minibuffer looks like this, with nothing below this line:

(1/2) Find Definition: the_

However, one would expect all three symbols be filtered. Type the full symbol name followed by "Enter" still correctly take me to the definition.

xuhdev commented 5 years ago

@FelipeLema Thanks for the fix! Are you sure this is fixed? When I type the_, I now get:

(1/1) Find Definition: the_
the_first_func

The other functions are now shown in the filter.

FelipeLema commented 5 years ago

oh, crap... it looks like an environment issue... I'll try to add some environment tests so that we can nail this issue together

xuhdev commented 5 years ago

Thanks! Let me know what I can help with!

xuhdev commented 5 years ago

I ran (counsel-gtags--build-command-to-collect-candidates "the_" '("--result=ctags")) and the output seems correct to me (and I verified that they output all symbols starting with the_ on the command line):

"global -c -i --path-style\\=through --result\\=ctags | /usr/bin/grep the_"

Then the issue is more likely to be in connection with ivy, which is not in the newly written test case...

xuhdev commented 5 years ago

I also verified that (counsel-gtags--async-tag-query-process "the_") writes all three functions into a new buffer.

xuhdev commented 5 years ago

@FelipeLema Is it OK for you to try on the latest ivy?

FelipeLema commented 5 years ago

Just checked, all tests run with latest ivy.

Aaaaaand I just remembered this repo has CI integration after doing so.

FelipeLema commented 5 years ago

I'll try to add some extra unit tests that do call ivy, then...

FelipeLema commented 5 years ago

I had at one point used grep for filtering the results for tag listing since using counsel--async-filter as the default process filter was too slow. I then changed it to use "find definitions using this regex" so the query and results changed, but the tests continued to work since they were made to simulate ivy behaviour and didn't reflect the latest changes (test-read-tag tests its own code, not counsel-gtags--read-tag). I can't remember why I did that in the first place.

Sigh, this is why I argue that ivy-related packages have difficulties being tested: we work around so much around ivy that we end up duplicating code

@xuhdev give it a try, since I'm not 100% sure https://github.com/FelipeLema/emacs-counsel-gtags/commit/baac1a718aaa3ad6c439ab48903b12013de2cec0 works ("it works in my computer")

xuhdev commented 5 years ago

Yes, it works on my computer. Thanks for your work!

Regarding the previous code, I think my experiment has shown that the code is functioning until the point ivy is called upon?

What is your suggestion if ivy is too difficult to test?

FelipeLema commented 5 years ago

The problem was that the code testing the feature ("filtering results without ivy standard functions") was a duplication of the actual code doing the feature (counsel-gtags--read-tag).

At one point I decided to duplicate the code to test the feature because the setup for doing ivy-read on a process is so complex and so interwined that it would require lots of work to inject code to be able to test it. This is not a judgement call, it's a fact: see the code for counsel--async-command.

counsel--async-command returns a process, yes. But it has a process sentinel and process filter assigned, so you cannot define your own for testing purposes. You can try, but then again how do you ensure it's going to work inside a ivy-read call?

My proposal was the Ivy package to support handling generators. This would make a separation layer between Ivy and "filtering results", being the latter responsibility and liberty of the provider. You can do your own process handling, filtering, and sentinel handling. You can even cache results (without resorting to outside processes).

FelipeLema commented 5 years ago

BTW, I'm closing since the bug reporter says it now works.