Shizcow / dmenu-rs

A pixel perfect port of dmenu, rewritten in Rust with extensive plugin support
GNU General Public License v3.0
197 stars 9 forks source link

Add fuzzy match. #16

Closed asdf8dfafjk closed 4 years ago

asdf8dfafjk commented 4 years ago

Add fuzzy match.

Example:

for i in Dmenu Rust ; do; echo $i; done | target/dmenu --fuzzy and enter Rt in the prompt, it will match Rust.

Shizcow commented 4 years ago

This functionality already exists on the development branch, and it's based on the same library. It just hasn't made it's way to master yet due to testing.

That said my algorithm is a bit different from yours. Mine collects scores for items and then sorts them based on that and a few other factors. I've found this gives more consistent results and smarter ranking for similar items (such as "Firefox" vs "FontForge"). When I have a moment I'll give this a try and see if it feels better.

asdf8dfafjk commented 4 years ago

Oh hey sorry I didn't check what you've got in works. Feel free to reject, my goal is only functionality.

There is something else I'd like to contribute but not sure if I have the right architecture in mind. I want to implement --auto which auto-selects if there is only one option

I plan to do this- Items::draw returns Result<Int,..> in place of Result<(),...> and in keyprocess we check if self.draw is Ok(1) in which case we call self.dispose() with last match and true. Does this sound okay to you?

Shizcow commented 4 years ago

All good. Thanks for contributing and being interested in this poroject regardless. If you want to test the development version, compile from the development branch or use the dmenu-rs-git package from the aur.

As for the auto feature, I like it. This would be dead simple to turn into a plugin, but I'm not sure the best way to do it. I'm in the process of refactoring plugins right now, so if you like I could put this on the roadmap to be completed within the next release or two.

asdf8dfafjk commented 4 years ago

Sorry I did not understand your point about autoselect feature. Do you mean to say I should go ahead and implement it the way I proposed, or that I should implement it as a plugin, ... or that you would implement it yourself?

Shizcow commented 4 years ago

You may implement it if you wish locally. Once I'm done refactoring I'll implement it as well.

asdf8dfafjk commented 3 years ago

Hi, please feel free to let me know if there is any way I can help with the auto-select feature (happy to implement it myself and submit a PR)

Shizcow commented 3 years ago

Thanks for the reminder. This is currently next thing I plan to implement.

What I'm stuck on is the most graceful method of doing this. It's a bit more complicated than I first thought. Ideally, some die method would be exposed, which calls dispose before force-exiting. This would allow calling of die anywhere, leading to exit on a certain condition.
An alternative (which I'm really leaning to here) is implementing a custom error type. Most of the project is pretty error-safe already, so this wouldn't be too hard to do. The main reason I'd like to let the program always naturally return from main for memory cleanup reasons. I'll think on this a bit more -- open to suggestions.

Combining this with a new plugin entry method format_stdin could make this pretty robust.

I think what I'll do here is take a look at the code tomorrow to see the best way to die. Once I have a plan of attack, I'll file a tracking issue for this to discuss progress and other thoughts. Once the framework is in place, I'll ping you so you can write an implementation. Sound good?

asdf8dfafjk commented 3 years ago

I am not familiar enough with code to discuss this with you but when I looked at the code- https://github.com/Shizcow/dmenu-rs/pull/16#issuecomment-654818576 was going to be my approach. When I say return Ok(1), I actually mean return Ok(<number of items currently displayed>)

asdf8dfafjk commented 3 years ago

There's something else I have in mind and I'm wondering if you'd be okay with that kinda feature-. Do you happen to use i3? I sometimes use dmenu not as a menu but as a text-input, somewhat of a not-so-ugly replacement for i3-input. Now i3-input has a feature where you can say --max-length=<n> and it automatically stops at text of that length without the need for pressing enter.

I wonder if this is something you'd like to see in your program. I am not particularly keen but it occurred to me to mention it so that if you like the idea, you can think about the design of both auto-select and length-limited-input together.

Let me know once you have a plan and I can go ahead and implement auto-select, and if you like length-limited-input.

Shizcow commented 3 years ago

I am not familiar enough with code to discuss this with you but when I looked at the code- #16 (comment) was going to be my approach. When I say return Ok(1), I actually mean return Ok(<number of items currently displayed>)

I think I understand. When you say Ok(<number of items currently displayed>) do you mean the number of items available, or the ones on screen? If it's the latter, this could cause an issue when scrolling to the right. If, after scrolling to the right, one item is on screen and --auto selects and disposes of that item, that might not work out too well.

I wonder if this is something you'd like to see in your program.

I don't see why not. I personally don't like it, but I could understand that some people do. I'll take a look but may need to consider a lot of additional things. I'll agree that i3-input is pretty ugly. If you have any more thoughts on length-limited-input, file a separate issue for discussion.

Shizcow commented 3 years ago

As for the framework I mentioned here, that is complete. In plugin_entry.rs, there now exists two new methods: format_stdin and postprocess_matches. Either could be useful in this case, but I think you're good to implement an auto-select plugin now. Just make sure you're basing this off of the develop branch.

I'd go about this by #[override_default]'ing postprocess_matches, checking the length, and if it's the right length dispose(item.text, true) before returning Die::Stdout(String::new()). If it's not of the right length, Ok(items) should work just fine. But this is up to you. I'm interested to see how you decide to add this feature.

I've also updated the plugin guide with some more useful info on creating a plugin. The password plugin should be a good template to go off of here.

asdf8dfafjk commented 3 years ago

Alright, I will take a look at the plugin guide and get back to you.

asdf8dfafjk commented 3 years ago

Hello, I checked out the develop branch and added fuzzy to the plugins line in config.mk. I did make afterwards but I cannot get --fuzzy flag to be recgnized, e.g.:

$for i in  option1 optino2
   do
  echo $i; 
 done | target/dmenu --fuzzy 
error: Found argument '--fuzzy' which wasn't expected, or isn't valid in this context

Am I enabling the plugins correctly?

Shizcow commented 3 years ago

The fuzzy plugin changes the default matching behavior, so fuzzy matching is always on. Therefore does not provide a flag.

This should be made more clear in the plugin description to clear up confusion. However, if you have a strong case for making fuzzy into a flag, I would consider it.

asdf8dfafjk commented 3 years ago

Thanks

Shizcow commented 3 years ago

As an addendum to the objectives here, I've added the functionality to turn off fuzzy search and clearer documentation on that. I'll be adding plugins to manpages as well soon.