Add a command mode

[khs26]

Still a WIP, but this PR will contain my changes to implement a command mode for mosaic.

[imsnif]

Hey @khs26 - great work on this.

I'm playing around with this and I have a concern about usability. So wanted to consult you because I'm honestly not 100% sure about it.

In tmux, when we press ctrl-b and then a command we immediately go back to normal mode. Here, we stay in command mode until we explicitly give an instruction to go back to normal mode (either ctrl-g or ESC).
This has the benefit that once you get what's going on, it's very powerful. You can easily resize stuff to your liking without having to go back-and-forth all the time, as well as move between panes, etc.

I'm not sure what is the more intuitive way, but this kind of threw me off... what do you think?

Like, my thoughts were to have both of these mode... Command mode and temporary command mode? One being ctrl-g and one being ctrl-gg? I'm not sure. Would love to hear your thoughts.

About the tests:
I think we need to adjust the tests in order to get them to work with this change. The tests use fake commands which simulate stdin and are defined here: https://github.com/mosaic-org/mosaic/blob/main/src/tests/utils.rs#L38

Each test imports these commands and attaches them to stdin like this: https://github.com/mosaic-org/mosaic/blob/main/src/tests/integration/basic.rs#L130

I think (once we answer the usability questions above) we'll need to adjust them. I'd be happy to give you a hand with that because this might be a bit of repetitive work :)

Looking forward to getting this merged!

[khs26]

I'm playing around with this and I have a concern about usability. So wanted to consult you because I'm honestly not 100% sure about it.

In tmux, when we press ctrl-b and then a command we immediately go back to normal mode. Here, we stay in command mode until we explicitly give an instruction to go back to normal mode (either ctrl-g or ESC).
This has the benefit that once you get what's going on, it's very powerful. You can easily resize stuff to your liking without having to go back-and-forth all the time, as well as move between panes, etc.

I'm not sure what is the more intuitive way, but this kind of threw me off... what do you think?

Like, my thoughts were to have both of these mode... Command mode and temporary command mode? One being ctrl-g and one being ctrl-gg? I'm not sure. Would love to hear your thoughts.

Yeah, that all makes sense. I think for now we should stick with a temporary command mode that just executes a single command. I'm a screen user, but it has a similar behaviour except when explicitly entering a longer term mode (e.g. searching).

About the tests:
I think we need to adjust the tests in order to get them to work with this change. The tests use fake commands which simulate stdin and are defined here: https://github.com/mosaic-org/mosaic/blob/main/src/tests/utils.rs#L38

Each test imports these commands and attaches them to stdin like this: https://github.com/mosaic-org/mosaic/blob/main/src/tests/integration/basic.rs#L130

I think (once we answer the usability questions above) we'll need to adjust them. I'd be happy to give you a hand with that because this might be a bit of repetitive work :)

Sure. I think I'd not properly thought through the due diligence to get this PR in (since I was originally just sticking with it being a draft). I'm more than happy to amend the tests for whatever the eventual solution ends up being. It'd be good for me to have a scan through our tests anyway.

[imsnif]

Sounds good.

So about usability:
I agree that a temporary screen/tmux-like command mode is best for now. Let's do that, but in that case maybe also remove the leader characters (where possible)? So it won't be ctrl-g+ctrl-j but rather ctrl-g+j.

About the tests:
Cool. Do let me know if you change your mind and want a hand. I got quite good at vim-macroing these things ;P

[khs26]

@imsnif there do still seem to be some test failures and it's a little arcane to me at the moment. Might be worth you having a quick look over things. I'm going to keep tidying, but won't have much time until the weekend (I'm "doing lunch break wrong" apparently :D ).

[imsnif]

@khs26 - sure. I took a look and it seems to me like you're definitely doing the right thing with the tests.

I think the issue with the tests you're seeing is a magnification of some issues we've been having with atomicity (both in the app and the tests). I fixed it (I hope!) in a different PR which I'm about to merge here: #59

I'm going to merge it into your PR and see if it does the trick. If not, I'll play with it a little to see what can be done. Will keep you posted :)

[khs26]

Cool - I'll leave the PR as it is now then and wait for you to respond

[imsnif]

Alright, fixed. The merge didn't fully solve it, so I made one more fix and found a bug in the tests which I also fixed.

The small fix: There were two test cases to which the COMMAND_TOGGLE fake stdin input wasn't added. I added it.

The bug in the tests: there is one case in which we quit the app not through the quit command. This is by closing the last pane. We had a hard-coded quit command in the stdin fake, which meant the test would always pass even if the feature wasn't working (false positive). I fixed that now and adjusted the fake stdin loop not to panic when it doesn't have any input left (which is needed in this case because it checks for input before it has a chance to die - see the comment I left there for more info).

@khs26 - I really like the functionality of this feature, and the implementation is top-notch. I especially like the transitive/non-transitive command mode. I think it's pretty cool. :) I think we need to adjust the README though, would you like to do it or shall I?

[khs26]

Thanks for the extra fixes! I'll merge them into my local branch so I can give it a cross-review. As for the README, I'll fix that at lunch. :)

[imsnif]

Awesome. Just to make sure though: there's no rush. I wouldn't want you to do lunch wrong again if you don't want to :)