-
Notifications
You must be signed in to change notification settings - Fork 3
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
Plugin breaks if the filename contains a number range #15
Comments
Ah, parsing edge cases. The plugin tries to match grep output to figure out which part is a filename and which is a line number. I suppose it's unsurprising that it doesn't work that well for all cases. I'll see what I can do to fix the situation. I've got some ideas, but I'll need to experiment. |
I've pushed some commits that should fix the issue. Essentially, the problem is that Performance-wise, this shouldn't be a problem, although I admit it feels a bit like overkill. But there's no reliable way that I can see to get grep output in a more machine-readable way. Ripgrep has a In any case, could you test it out now and confirm it works correctly? |
Hello Andrew, Thanks for your work! I just started testing the new release but ran into something that looks like a completely new bug that wasn't there before. Here's a screenshot that shows the new issue: Here are the steps that I took that led to this new issue.
Here are 5 commands for quick copy/pasting that instantly lead to this new issue:
I'll be happy to do more testing at any time, just let me know. Peter |
Ah, I seem to have missed a particular case to test. Thanks for the detailed report. Could you try the most recent master? |
Hi Andrew, I just tested today's master (head 5dd96a5) but unfortunately another error has come up. I took the same steps to test it as the last time and got this error: For your convenience, here are the commands that I ran:
I'll be happy to test again, just let me know when. Peter |
Ah, yes, seems like I had managed to fix it only for a specific case, my bad. I've pushed another attempt, please try again. |
Hi Andrew, I just tried again with the latest commit 5644382 but now a new error has come up: I used the exact same 5 steps as in the previous message. I'll be happy to test again, just let me know, and thanks so much for all this effort! Peter |
That's strange, I can't replicate the error with these steps. Could you let me know what your Vim version is, and ideally share your .vimrc? There might be some configuration I'm accidentally relying on. |
Hi Andrew, I use the following vim version (default vim install on Debian 9.8 Stretch):
I just disabled all plugins except writable-search and here's a list of all default system plugins that were loaded:
My .vimrc is here: https://github.com/pkrumins/dotfiles/blob/master/.vimrc Please let me know if you need any other extra information. Peter |
Hm, I think I maybe sort of understand what might be the problem. The core of it is that running the command includes stderr, which means diagnostic messages also get placed in the buffer, like "permission denied". I think the reason I'm not getting the error with my normal Vim setup is because my backend is the I'll see about ignoring stderr, but I'm wondering if it would work for you if you
Either way, I do need to fix the stderr issue, just wondering if this is the core of the problem or if there's something else still. |
Update: The stderr issue should be fixed, so your unmodified test might work now as well. |
Hi Andrew, Thanks for figuring this out! I just tried the latest master at d6a408 and it's working well. The original issue and other issues are gone. Just one final question: When I run the Here's a screenshot that lists the open buffers after I It looks like these are all buffers that have the matches. Is that the expected behavior? I don't remember if it left all buffers with matches open before. Peter |
Yes, this has probably always been the case. To update the buffers, the plugin switches to them, applies the update, then switches back. Which is why it temporarily sets I've applied a fix, since it shouldn't be necessary to hide the original buffers. Let me know if it looks okay now. |
Hi Andrew, I just tested the new version (master f537c7) and it's almost perfect now except for one new issue related to buffers. Here are the steps to reproduce it:
Now change some "the" in some of files to something else like "moo". Now What will happen now, only I'll be happy to test again at any time. Peter |
Hm, I see, the problem is that I was too eager to mark buffers as deletable when switching back to the result buffer. I added a check for whether the buffer already exists or we're loading it just for the replacement. Could you check now? |
Hi Andrew, I just got to testing the latest changes and it looks like it's finally working without errors. Really great! There are just two more tiny issues which I don't think are related to this ticket but it would be nice to fix them as well. The first one is this – after doing a The second is this one – it shows a lot of grep "No such device or address" messages right after running ":WritableSearch foo". Like this: Both of these issues can be reproduced with the latest master and with all plugins disabled (only writable_search plugin is active). I'll be happy to test these last two issues at any time. Peter |
I'm not sure where the first message is coming from, but adding a |
Hi Andrew,
Thanks for creating this great plugin!
I was just using it on a file with a filename
readme-2000-2010-2.md
and the plugin broke. It thinks that all edits should happen on line 2000.I experimented some more and it works if the filename is
readme-2000.md
but breaks again if it'sreadme-2000-2010.md
.Also, if the pathname contains a range but the filename does not, then sometimes it breaks the same way. At the moment, it looks like it breaks if the pathname contains a range only if it's using
git grep
.Apparently, if there's something that looks like a number range in the pathname or filename, then something goes wrong.
I hope you can replicate it and fix it. Thanks once again.
Peter
The text was updated successfully, but these errors were encountered: