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

WIP: Allow regexes to contain ␀ byte #713

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lluchs
Copy link

@lluchs lluchs commented Jun 19, 2018

As previously noted in #359, vis has some trouble with searching in binary files. I noticed that the optional TRE library does actually support NUL bytes in the regex, and started modifying vis to make use of that.

With this patch, it's possible to search for NUL bytes by yanking such a pattern into the "/ register and then using n/N. It's not possible yet to enter a pattern containing NUL bytes directly, as the command line works with zero-terminated strings. Actually entering the pattern works fine, but it discards everything after the NUL byte. I haven't figured out how to change that yet.

@mcepl
Copy link
Contributor

mcepl commented May 28, 2024

@rnpnr I really don’t know what to do with this one. Do we really care about binary files in vis?

@rnpnr
Copy link
Collaborator

rnpnr commented May 30, 2024

Do we really care about binary files in vis?

I'm not sure. It definitely doesn't really work right now. As for this patch its not really intrusive and any time I can use strings with a length instead of the NUL terminated mistake I prefer it.

Actually entering the pattern works fine, but it discards everything after the NUL byte. I haven't figured out how to change that yet.

I think you probably need to use tre_regnexec() instead of tre_regexec() when you actually go to perform the match. That also means that the length needs to be specified but that shouldn't be a problem. I would also prefer that this patch was modified so that tre_regncomp() is always used in text_regex_compile() and therefore the length is always required.

@lluchs if you are still interested in this I will look at any updates otherwise someone else can take this over. I don't think any of these changes should be too complicated.

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

3 participants