davidgranstrom / scnvim

Neovim frontend for SuperCollider.
GNU General Public License v3.0
208 stars 28 forks source link

fix for method documentation search #233

Closed sadguitarius closed 7 months ago

sadguitarius commented 8 months ago

When searching for documentation on a method, the regex doesn't find cases where the line starts with the class name and not just the method name. Also Windows path names are broken coming out of the quickfix window.

This PR should hopefully provide a fix. I added a check for whether Neovim is running on Windows before messing with backslashes, so the path fix shouldn't affect anything on Mac or Linux. Let me know if you need me to split this into a separate commit, I thought I'd just sneak this in since it's just a tiny change.

When you get a chance, can you check this and let me know if anything needs to be cleaned up? I restructured the on_open method and quickfix logic a little so the regex would be easier to parse. I don't think this will break anything, but let me know if I'm missing something and there's a better way to do it. Thanks!

davidgranstrom commented 8 months ago

Thank you for your PR, I'll try to review this as soon as possible!

An initial thought is that you could use the pre-computed value for the isWindows check, you will find it in the scnvim.path module.

sadguitarius commented 8 months ago

Ah I missed that function! Thought I'd done a proper search for something like that but apparently not. I swapped my function out for that one.

davidgranstrom commented 8 months ago

Got some time to try this out and it works great! I've added some comments in the review, please let me know if you have further questions. Thanks!

sadguitarius commented 8 months ago

Cool! I don't see anything showing up in reviews, but it's possible I'm looking in the wrong place.

davidgranstrom commented 8 months ago

Sorry, I had forgotten to submit the review!

sadguitarius commented 7 months ago

Ok I have a good head start on these changes and things seem to be working fine. Check my comment re: the quickfix list when you can, I'm not sure how best to implement your suggestion there. Thanks!

davidgranstrom commented 7 months ago

The new changes looks good to me! But there seems to be a regression from the last commit, when I search for method such as tanh and try to open Classes/SequenceableCollection .tanh the pattern matching end up on word SequenceableCollection rather than .tanh (which is further down that particular help file).

I haven't found a good answer to the pattern vs. text discussion earlier, so let's use text for now as it seems to be working on your end.

sadguitarius commented 7 months ago

I just fixed a wrong signature that was causing the method to fail. Should be ok now!

davidgranstrom commented 7 months ago

Thanks!