search

wookayin / semshi

🌈 Semantic Highlighting for Python in Neovim
79 stars 5 forks source link

fix handling multiple windows for a buffer - fix #11 #12

Open FelipeLema opened 1 year ago

FelipeLema commented 1 year ago

fixes #11

FelipeLema commented 1 year ago

it's working now, although it's missing tests... but you may want to add those in a separate PR

FelipeLema commented 1 year ago

cause I'm uncertain of when can I add them

FelipeLema commented 1 year ago

oh, I see ... it seems fetching the viewports from python can be costly (not sure how bad)

the change to obtaining them from python was rather because I prefer to have as little vimscript as possible... I don't mind making the changes to fetch all viewports from vimscript. will do just that

FelipeLema commented 1 year ago

ready to go

I re-implemented the get_viewports() function in lua, but it could be written in vimscript in the future

wookayin commented 11 months ago

Thanks for the contribution again. Actually I'm having a hard time reproducing the bug. I tried this session but haven't had the bug. Can you make a more clean step please, possibly using neither mksession nor absolute (user-specific) paths, starting from nvim --clean? Or writing a unit test will be even better, but I can take care of it as long as it's easily reproducible.

FelipeLema commented 11 months ago

lemme try and add an unit test

It's probably not going to be ready this week. Hopefully this month

FelipeLema commented 11 months ago

oh, I see that we're not testing highlights. the to-be-implemented test points to https://github.com/neovim/neovim/issues/6412, which has been solved

we may want to address that TODO first

wookayin commented 11 months ago

I see. I will work on refactoring to switch to extmarks from the legacy highlight API soon. Adding a test is not a big priority.

BTW using the classic vim API synIDattr(synID(line, col, 1), 'name') would still work.

wookayin commented 10 months ago

Actually I don't mind we don't have tests yet as we're lacking a infra for testing highlights, but I still would like to reproduce the issue.

FelipeLema commented 10 months ago

Will pick up the MVE-as-test, although I was shifting my focus into implementing an LSP server that only does semantic highlight using code from this repo

anyone reading this can chip in with such test in the meantime