cantino / mcfly

Fly through your shell history. Great Scott!
MIT License
6.89k stars 178 forks source link

implemented feature to switch result sort by key binding #263

Closed navazjm closed 2 years ago

navazjm commented 2 years ago

Closes #238

impl MenuMode {
    fn text(&self, interface: &Interface) -> &str {
        match *self {
            MenuMode::Normal => match interface.settings.key_scheme {
                KeyScheme::Emacs => match interface.result_sort {
                    ResultSort::Rank => "McFly | ESC - Exit | ⏎ - Run | TAB - Edit | F1 - Sort (Time) | F2 - Delete",
                    ResultSort::LastRun => "McFly | ESC - Exit | ⏎ - Run | TAB - Edit | F1 - Sort (Rank) | F2 - Delete",
                }
                KeyScheme::Vim => {
                    if interface.in_vim_insert_mode {
                        match interface.result_sort {
                            ResultSort::Rank => "McFly (Ins) | ESC - Cmd | ⏎ - Run | TAB - Edit | F1 - Sort (Time) | F2 - Delete",
                            ResultSort::LastRun => "McFly (Ins) | ESC - Cmd | ⏎ - Run | TAB - Edit | F1 - Sort (Rank) | F2 - Delete",
                        }
                    } else {
                        match interface.result_sort {
                            ResultSort::Rank => "McFly (Cmd) | ESC - Exit | ⏎ - Run | TAB - Edit | F1 - Sort (Time) | F2 - Delete",
                            ResultSort::LastRun => "McFly (Cmd) | ESC - Exit| ⏎ - Run | TAB - Edit | F1 - Sort (Rank) | F2 - Delete",
                        }
                    }
                }
            },
            MenuMode::ConfirmDelete => "Delete selected command from the history? (Y/N)",
        }
    }

Also, I think by rewriting this function, reduces the number of places we have to add new key bindings. Above, we would have to change all 6 of the different strings. While with the new rewrite, the programmer has to figure out where to append the new keybinding.

Let me know your thoughts @cantino

cantino commented 2 years ago

Hey @tehmj, thanks for the contribution! When I tried it, pressing F1 updates the menu, but it doesn't actually resort the list until I start typing. Do you think it should immediately trigger a resort?

navazjm commented 2 years ago

I think it should trigger a resort. I mentioned this in the original issue, but did not receive an answer and forgot to include it in my PR. I will try to make the changes and get them committed. @cantino

cantino commented 2 years ago

Sorry for the delay. Thanks for this nice addition @navazjm!

cantino commented 2 years ago

Released in 0.6.1!