hrkfdn / ncspot

Cross-platform ncurses Spotify client written in Rust, inspired by ncmpc and the likes.
BSD 2-Clause "Simplified" License
4.94k stars 205 forks source link

Return to search when viewing search results #390

Closed Qluxzz closed 3 years ago

Qluxzz commented 3 years ago

With the changes that came from separating the search and search results in commit https://github.com/hrkfdn/ncspot/commit/18dc6c6bf84a5529c200376020eda48bd0ea7557 searches now opens a new view with the search results.

If you navigate a bit through the search results and navigate to albums and artists it takes multiple keypresses to return to the search view.

My solution is if you're viewing the search results and press to focus the search view again, the search results are closed and you're returned to the search view.

@hrkfdn, could you maybe give me some hints on how to be able to pop the view or otherwise programmatically close the search results?

I've tried reusing the same logic as pushing the view from the on_submit handler in the on_command handler but from that I can see pop_view is never called so I guess call_on_name doesn't work there.

fn on_command(&mut self, s: &mut Cursive, cmd: &Command) -> Result<CommandResult, String> {
    if let Command::Focus(view) = cmd {
        if view == &"search" && self.showing_results {
            s.call_on_name("main", move |v: &mut Layout| v.pop_view());
            self.showing_results = false;
        }

        self.edit_focused = true;
        self.clear();
        return Ok(CommandResult::Consumed(None));
    }

    Ok(CommandResult::Ignored)
}
hrkfdn commented 3 years ago

This would probably have to be implemented in Layout, as it consumes the Command::Focus event. Or the behavior needs to be changed so that Focus events are passed on to subviews.

For the former approach a possible way to implement it could be: If Command::Focus("search") is emitted when the search is already open, the view stack for search is cleared.

Does this help?

Qluxzz commented 3 years ago

This would probably have to be implemented in Layout, as it consumes the Command::Focus event.

From what I understood from the layout the focus event focuses the screen but the event is then passed down to the focused screen as well? https://github.com/hrkfdn/ncspot/blob/f507ef6cedd9dc492380a5077e2b3ecf4be3b66a/src/ui/layout.rs#L320-L328

Since it clears the search bar if pressed while on the search view.

So the event is passed down to the search view but not to the search results. Would it not then be possible to pop the view from the search command handler or does it have to be done in the search results command handler?

hrkfdn commented 3 years ago

From what I understood from the layout the focus event focuses the screen but the event is then passed down to the focused screen as well?

Ah, sorry. Looks that way, yes. Didn't have the source at hand.

Would it not then be possible to pop the view from the search command handler or does it have to be done in the search results command handler?

As long as you can hang onto the Focus event and know the current screen, you should be able to do it in the search command handler. However, I think for now you can only pop single views via Layout. So you'll need to implement something like clear_screen(name) in Layout to clear the entire view stack for the the given screen name.

Qluxzz commented 3 years ago

I think I'll have to rethink this, trying to get a hold of the layout in the on_command handler for the search gives me a crash with a BorrowMutError.

Found this blog post which explains the error I'm getting https://cafbit.com/post/cursive_writing_terminal_applications_in_rust/

hrkfdn commented 3 years ago

Do you have this pushed to a branch somewhere? I could take a look if you want.

Qluxzz commented 3 years ago

Thanks for taking the time to help me.

I've added an example here, just trying to call pop_view on layout in the on_command handler. https://github.com/Qluxzz/ncspot/commit/b3122a76103c46fef010515635b23f6cfafd811d

hrkfdn commented 3 years ago

Ah, right. https://github.com/hrkfdn/ncspot/blob/6e0f00824ae0d123ff7c8bbd7e81a48a953b449a/src/commands.rs#L271-L273 is where the command handling for sub-views starts. The command is passed on to Layout via s.call_on_name. This creates the first mutable reference to Layout.

In the search sub-view in SearchView.on_command you are trying to create the second mutable reference to Layout. Rust doesn't allow multiple mutable borrows at the same time, thus the crash. So this probably needs to be implemented in Layout.