Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for method documentation search #233

Merged

Conversation

sadguitarius
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

lua/scnvim/help.lua Show resolved Hide resolved
lua/scnvim/help.lua Outdated Show resolved Hide resolved
lua/scnvim/help.lua Outdated Show resolved Hide resolved
@davidgranstrom
Copy link
Owner

Sorry, I had forgotten to submit the review!

@sadguitarius
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

@davidgranstrom
Copy link
Owner

Thanks!

@davidgranstrom davidgranstrom merged commit 94e49fa into davidgranstrom:main Apr 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants