Skip to content

Allow find and replace with an empty string #442

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

Closed
wants to merge 1 commit into from

Conversation

Shivix
Copy link
Contributor

@Shivix Shivix commented Jun 8, 2025

Currently if the replace field is empty, the write function will do nothing, and hitting enter will simply go through the occurrences replacing nothing.

Allowing the replace string to be empty, allows the user to use find and replace to rapidly erase text.

Since write specifically checks if string is empty, we could consider allowing an empty write, which does work in testing, but we can instead utilise extract_selection here instead. This is a bit more explicit and ensures this change only affects find and replace.

Closes #439

Since write specifically checks if string is empty, we could consider
allowing an empty write, which does work in testing, but we can instead
utilise extract_selection here instead. This is a bit more explicit
ensures this change only affects find and replace.
Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right approach. There's no inherent reason why write has to exit early for empty inputs. It's still meaningful if there's an active selection after all.

I've written an alternative approach over in #457 and I'd be happy if you could check it out.

@Shivix
Copy link
Contributor Author

Shivix commented Jun 10, 2025

Yeah fair enough, allowing write to work with an empty input was actually what I went with at first. I had spent some time testing that also and it works well. I just went with the change that has lower potential impact to make it easier to review.

@lhecker
Copy link
Member

lhecker commented Jun 10, 2025

#457 was just merged, so I'll close this PR for now. 🙂

@lhecker lhecker closed this Jun 10, 2025
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.

Unable to replace if replacement is an empty string
2 participants