Update OneSnippet#move method to fix #96545.#108964
Conversation
|
I don't think this is the right fix (and btw a risky change). Also, you unit test doesn't use the snippet supplied in the issue. |
|
Thank you for your comment! I see. Should I close this PR? What I was able to find about the cause of the issue was that the cursor moves forward when the second placeholder is done. And yes, I had a feeling that this change might be risky... 😓 About the unit test, I thought the snippets are equivalent. (If you use the one in the issue, the test case is also passed.) |
jrieken
left a comment
jrieken
left a comment
There was a problem hiding this comment.
Actually, this is a good change... Only had the time now to properly look into this. Yeah, the challenge is moving placeholder while inserting the snippet. The OP used this snippet ${1:{}${2:fff}${1/[\\{]/}/} which slightly differs but it will work your change as well. Thanks
|
@jrieken Thanks for your time! 😃 |
2 participants
This PR fixes #96545.
I have changed
EditOperation.replaceMovetoEditOperation.replacein OneSnippet#move method and added a unit test.Although all the unit tests are passed, I'm not 100% sure about side effects. I would appreciate feedbacks!