c0r73x / neotags.nvim

Tag highlight in neovim
MIT License
122 stars 9 forks source link

Rewrote all tag handling, removed pcre2 dependency #29

Closed roflcopter4 closed 6 years ago

roflcopter4 commented 6 years ago

I meant to do this sooner but didn't quite have the time to do it properly. I've been fiddling with things quite a bit and ultimately with this commit things are quite a bit faster, and the C code is simpler and more robust. It now handles '.xz' files if desired too.

I rewrote the python tag handling and it is now just as fast as the C code for small projects. It's still slower for huge ones (like perl) but not by nearly as much as before. I also got the C to compile in Windows, but I don't think I'll bother including that.

I have changed a bunch of things here, so I should definitely see what you think before just merging with the master branch. I changed most of the highlight link names to make it easier to impose a consistent scheme across all languages. That change is probably a bit dubious, it could break people's configurations. The documentation also needs updating again. I'm curious to know what you think.

c0r73x commented 6 years ago

Hi, sounds nice but now I get some missing keys errors.

c0r73x commented 6 years ago

Seems like call InitVar('use_binary', 0) was missing, I added it and it seems to work fine, I just need to tweak my colors to match your changes :smile:

c0r73x commented 6 years ago

Humm, there seems to be some inconsistency between the python and the binary version when it comes to the tag priority?

When I use the python version it loads all my tags and everything looks as it should, but with the binary version my classes don't get highlighted at all. But if i disable functions from the tag order the classes gets loaded.

let g:neotags#cpp#order = 'cf' andlet g:neotags#cpp#order = 'fc' gives the same results with functions taking priority.

c0r73x commented 6 years ago

I found another issue, it seems the "notin" dosn't work anymore? now it highlights tags in strings and comments :(

roflcopter4 commented 6 years ago

Oh dear. I thought I pushed a few updates to the dev branch, not the master. And I definitely did not mean to merge this pull request. I'm sorry (maybe I should actually read the git manual before continuing to look like a fool...). This code is not really ready for release...

The tags order thing was a bug I knew about. I'll look at it asap. I was discarding all duplicate tags naively, ignoring their group info.

Also, regarding the notin stuff, in every test I tried I've had no problems with nvim highlighting things it shouldn't. The languages that had special 'notin' information defined (notably javascript) still use it, but I disabled it for the others because using it would force us to use syntax match for everything, thus massively slowing things down. You can't specify containedin for keywords. Nvim should automatically know where to expect a keyword. I could be wrong though. I don't know why it would highlight inside comments. Which language?

Again, sorry. I didn't actually think this code should go to the master branch. I'll see if I can fix things, but it might make sense to just revert everything for a little while.

c0r73x commented 6 years ago

Hehe it's ok, the merge works except for some minor things :nerd_face: don't think we need to revert the merge.

Ah it has to be something with my custom typescript file because i don't get highlights in comments or strings in C++

roflcopter4 commented 6 years ago

I'm glad things are at least mostly working. I was concerned about making the documentation largely wrong again, and especially in messing up people's config. I also know nothing about typescript, so that's a bit tough to test properly.

I think I fixed the ordering problems. I spent a while writing code sort the original linked list into arrays for each group which could then be pruned in order of priority before I realized that it's possible for duplicates to be allowable if that group has a prefix/suffix. So I threw that out and just pruned duplicates within groups. It seems to give the same results as the python code for me now.

Since I got the thing to compile in Windows (somehow) I think it might make sense to make some kind of release, and upload the binary there. It shouldn't be too difficult to have the Makefile download the thing with wget or curl or whatever is actually available on Windows. It could even be possible to do this for linux too. LZMA (ie .xz) support will only compile if you have the newest alpha release of liblzma, which probably close to 0% of people actually have.

c0r73x commented 6 years ago

I fixed the problem with javascript/typescript by adding String and Comment to notin.

Yeah the order seems to work as it should now :+1:

True that the documentation should be updated but the most important part imo. is that the default config works as expected, and from my checks it does.

Humm, maybe check the system version of LZMA on linux and if it's not up to date it can ask to download the binary instead. I don't like to force binaries onto people without them knowing. As for Windows i think that people who use Windows are used to run whatever binaries they can find :joy: so there it might be a good idea.

roflcopter4 commented 6 years ago

Sorry to be so late in replying here, I completely forgot. Regarding binaries I pretty much agree, downloading them on Unix-ey systems is pretty strange, but on Windows people probably wouldn't even notice. If I actually get the courage to boot into Windows again sometime soon I think I might put together some kind of binary release.

As for LZMA, the version required is definitely not going to be present on any systems yet, it's considered by the author to be an alpha. I used it because they added a bunch of very useful functions to the C api that make it a million times easier to use. Otherwise I'd probably have to actually scour the source code just to find out how to get the uncompressed size of a file which doesn't sound like fun. Still, gzip is generally good enough for this purpose, so I don't know if it makes sense to download a binary version of the library regardless.

Also I added threads to the binary. It now processes that Perl source file in a quarter of a second, which is practically not even noticeable. I should also really update the documentation.