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

Side effects of s:MatchesInRange() #18

Closed
lacygoill opened this issue Oct 21, 2017 · 6 comments
Closed

Side effects of s:MatchesInRange() #18

lacygoill opened this issue Oct 21, 2017 · 6 comments

Comments

@lacygoill
Copy link

Inside the function s:MatchesInRange(), this command is executed:

 silent! execute a:range . 's///en' . gflag

I think it has several undesired effects, some of which could be mitigated.

It adds an entry in the jumplist: this could be solved with :keepjumps:

     silent! execute 'keepjumps '.a:range . 's///en' . gflag

It alters the [ and ] marks: this could be solved by saving/restoring the marks:

let marks_save = [ getpos("'["), getpos("']") ]
...
silent! execute a:range . 's///en' . gflag
...
call setpos("'[", marks_save[0])
call setpos("']", marks_save[1])

It makes you lose some metacharacters and flags like ~ and & (see :h :& and :h :~).
For this issue, I have no idea how to fix it. I've tried to think about mechanisms to save and restore them, but they are not reliable.

You would need to find a way to execute the substitution without Vim logging anything, but I don't know if it's possible.

@dbarnett
Copy link
Contributor

I would guess an implementation based on search() rather than a :substitute command would have a lot fewer side-effects, if we're not stuck w/ this approach for performance reasons or something. @rburny have we already looked into using search()?

@lacygoill
Copy link
Author

lacygoill commented Oct 22, 2017

@dbarnett Yes, initially I had the same idea as you. A while loop invoking search() which increments a counter (with a guard to avoid being stuck in an infinite one). Unfortunately, in my limited testing, it seemed that :substitute was sometimes much faster than search(). Maybe I made a mistake, or maybe I just had bad luck with some particular patterns though.

But if search() can, indeed, be much slower than :substitute, I prefer using the latter, because I value the speed of the plugin more than the lost flags/metacharacters, as I almost never use them.

Also, there are a few workarounds to the possible slowness of search(), including the 4th optional argument {timeout}.

@dbarnett
Copy link
Contributor

Alright cool, sounds like you've investigated in way more depth than I have. =)
So it sounds like the best path forward is to add the two improvements you mentioned along with some comments about why search() isn't an option for the benefit of anyone trying to improve it in the future.

@rburny
Copy link
Collaborator

rburny commented Oct 24, 2017

Thanks for investigating these!

Indeed, I have used :s command because it is much faster. In contrast, any solution that implements a loop in Vimscript (like search() loop) tends to be slow, especially on large files.

I agree with your assessment - let's fix what we can within the current solution. Pull requests are welcome :)

@rburny rburny closed this as completed in fb92b0f Feb 25, 2018
@liushapku
Copy link
Contributor

One side effect of silent! execute a:range . 's///en' . gflag is that it changes the last substitute string.

For example:

:s/red/blue
/green  (press enter here invokes <Plug>SearchIndex which sets the last substitute string to ''
:~

Without the plugin, the last :~ will replace green with blue. With the plugin, it will replace green with empty string.

The case can be fixed by changing the above mentioned line to

silent! execute a:range . 's//~/en' . gflag

A PR #20 is created for this.

@liushapku
Copy link
Contributor

The side effect of changing the last used flags is not addressed.

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

No branches or pull requests

4 participants