search

ndd7xv / heh

A terminal UI to edit bytes by the nibble.
MIT License
434 stars 18 forks source link

[FEATURE REQUEST] Search by hex #129

Open valpackett opened 1 month ago

valpackett commented 1 month ago

Is your feature request related to a problem? Please describe.

Seems like the / search only searches by decoded ASCII, which is not always what you want especially when patching things…

Describe the solution you'd like

Add a hex search mode

ndd7xv commented 4 weeks ago

Thanks for the issue! This would be a really useful addition.

I've been busy as of late, but when I get the time to work on this I promise this will be the first thing I address.

orhun commented 4 weeks ago

I can help with the implementation in my spare time @ndd7xv if you want! I think the only thing that needs clarification is that whether if we want another key binding for searching by hex or support searching for both ASCII and hex via /.

ndd7xv commented 3 weeks ago

I'd greatly appreciate all of your willingness to help this repo! Given everything you've done so far, I've invited you to be a collaborator so I'm not the bottleneck if you do work on it. You've also had some really good ideas and additions to heh thus far.

Honestly, I feel like key question is best left to the people who want this feature, as I no longer need to use a hex editor daily and consequently don't know what would be the best user experience.

If there's no preference, then I like the idea of having both searches through / since it seems intuitive.

orhun commented 3 weeks ago

Thank you for the invitation! Will do my best to maintain this awesome project 🦀

If there's no preference, then I like the idea of having both searches through / since it seems intuitive.

Yes, I agree with that. I will look into implementing it pretty soon!

orhun commented 3 weeks ago

Implemented in #131

@valpackett can you test if this works as expected?

@ndd7xv any ideas about how to get rid of hex dependency in that PR? or is it fine?

ndd7xv commented 3 weeks ago

I think having the dependency is fine; I'm not too irked by adding another dependency, but if we change our minds about it, we can just add this code to our repo, which I think is what the decode function is doing when we call it.

Thanks for the PR!

valpackett commented 3 weeks ago

Looks correct, yeah!