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

Feat: don't change text if nothing to do #146

Merged
merged 4 commits into from
Feb 7, 2024

Conversation

backdround
Copy link
Contributor

If there was an accidental hitting on the wrong button and the code hasn't changed, the vim considers it a changed buffer (the modified option is set). It's a little annoying, so the pr is here.

The solution is to check for changes before setting the text.

I'm not sure:

  • should I use pcall to call nvim_buf_get_text? (I don't why there is pcall(vim.api.nvim_buf_set_tex... on the next line
  • should I notify.info that nothing has changed?

@Wansmer
Copy link
Owner

Wansmer commented Feb 7, 2024

This PR affects the functionality separately for split and separately for join, but not for toggle when the text is changed anyway.
So it doesn't make sense to do the check after the node is processed - better to do the check based on the passed mode and compare it with the auto-detected mode (e.g. here). At this point, you can notify the user that the current node is already splitted/joined.

(By the way, I originally wanted to add this functionality, but gave up. Unfortunately, I can't remember the reason why I didn't add it.)

@backdround
Copy link
Contributor Author

I didn't catch the thing about "it doesn't affect toggle". Should it affect toggle too? If it should then I don't understand the case.

Also I'm not sure that the check should be before the fallback test. What do you think?

@Wansmer
Copy link
Owner

Wansmer commented Feb 7, 2024

I didn't catch the thing about "it doesn't affect toggle". Should it affect toggle too? If it should then I don't understand the case.

No, you don't. The point was that you don't need to do an unnecessary comparison between the original and the processed text, because in the case of toggle it is always irrelevant. It is cheaper to do it before any processing.

Also I'm not sure that the check should be before the fallback test. What do you think?

Yes, you are absolutely right, it is more correct to check for fallback first. That is, just remove this check from the loop because its logic will not change.

@backdround
Copy link
Contributor Author

It should be ok now

@Wansmer
Copy link
Owner

Wansmer commented Feb 7, 2024

Thanks for PR!

@Wansmer Wansmer merged commit ecc6584 into Wansmer:main Feb 7, 2024
2 checks passed
@backdround
Copy link
Contributor Author

@Wansmer Thank for the plugin!

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.

None yet

2 participants