tab-rs icon indicating copy to clipboard operation
tab-rs copied to clipboard

Add `tab --close-recursive` which closes a tab, and all it's subtabs

Open austinjones opened this issue 3 years ago • 8 comments

austinjones avatar Aug 28 '20 18:08 austinjones

Bumping to 0.5.0, as 0.4.0 will support tab -w foo bar baz.

austinjones avatar Oct 07 '20 02:10 austinjones

Bumping again, as there are much higher value items on main that need to be released.

austinjones avatar Oct 18 '20 01:10 austinjones

Thank you for the amazing project, I'd like to take this as my first issue (and contribution), is it okay with you ? @austinjones

AkechiShiro avatar Dec 24 '20 04:12 AkechiShiro

Absolutely. Go for it @AkechiShiro!

austinjones avatar Dec 24 '20 04:12 austinjones

@AkechiShiro Here are a few things that might help:

  • I'm not sure what the flag short name should be. I use tab -W for shutdown - so maybe this will only have a long name.
  • There is a function called normalize_name that can convert user input into a canonical tab name (with the trailing slash). A tab should be closed by this command if it equals, or starts with the canonical name. (e.g. tab --close-recursive foo should close foo/, foo/bar/, but not foobar/)
  • You'll need to edit the tab crate to define the CLI command, and the tab-command crate to implement the functionality. You can probably base this off the close_tabs.rs service. These services get launched in main.rs.
  • You can launch the project in debug mode using scripts/run.sh. Any local crates (other than tab) that you modify need to be patched in Cargo.toml.

austinjones avatar Dec 24 '20 04:12 austinjones

Thank you for the starter help @austinjones, definitely helps ! I've got some questions :

1 — About the short name, I was thinking that it would be neat to have something like rw, but it wouldn't be doable at the moment, I have seen that the Clap crate handles the arguments, thus I had a look at their documentation and only a single UTF-8 code point is allowed as a parameter of .short(), I'll follow your suggestion for now, leaving it with no short-hand, maybe if someone has a good suggestion, we'll take them onto their offer.

Here are the relevant lines from the doc :

NOTE: Any leading - characters will be stripped, and only the first non - character will be used as the short version To set short use a single valid UTF-8 code point. If you supply a leading - such as -c, the - will be stripped.

Also, I've used the .validator(validate_tab_name) for the new command, I think it validates that the tabs name given are valid.

2 — For the new command .min_values(1) is necessary ? Or I should suppose that the current tab (in focus) should be closed recursively ? (The latter is better since it makes the behavior consistent with the current traditional close-tab command that works that way as of today, hence making it friendly to use for current users).

3 — I'm trying to understand how close_tabs.rs works, I think that every command is run asynchronously, they're sent as a Request (as for instance Request::CloseTab(id)) ?

4 — Where comes from this Request::* along its functions, I saw no use XXX::Request statements or something similar hence my question on this ?

Thanks for any answer on these, sorry if I didn't catch on everything yet, I'm kinda of new to writing asynchronous Rust.

AkechiShiro avatar Dec 24 '20 06:12 AkechiShiro

@AkechiShiro I took another look, and it seems like the MainCloseTabsService provides a lot of what you need. It'd probably be best to enhance it to support this additional method of closing tabs. I think this could be implemented by adding a case to the existing MainCloseTabsService (e.g. if let MainRecv::CloseTabsRecursive(...)). You could split MainCloseTabsService::close_tabs into a piece that does the TAB_ID env check, and the other one that closes the tabs. From there you should be able to implement this.

Here are my comments on your questions:

1 - I'll let you know if I can think of anything :)

2 - Yeah, I think the 'no-arg' default to current tab is nice. It saves several keystrokes on the command-line, and it can be put in scripts that automatically operate on the current tab.

3 - Right, so tab is based on a library I wrote called lifeline. The bus is a distributor of async channel endpoints (tokio mpsc/broadcast/etc senders and receivers). There are two 'connections' that are important here.

MainRecv is the 'entrypoint' for the tab-command async code. It is sent in tab-command/src/lib.rs, which is where the command-line argument match is unpacked.

The Request type is sent over a websocket to the daemon. All it takes is tx.send(my_request) - and the message will be serialized and sent to the daemon.

4 - The daemon handles the CloseTab message in src/service/cli.rs. From there it's passed on and eventually triggers a shutdown of the PTY process that launched the shell.

Thanks for any answer on these, sorry if I didn't catch on everything yet

No worries. Tab is a big project! Thanks for offering to put this together :)

austinjones avatar Dec 24 '20 16:12 austinjones

Isn’t it much more useful if -w already closes subtabs? Like as if you kill a parent process (in linux etc.) all child processes get killed as well. There is a tradeoff for this though, you can’t only close the parent without closing the children. I guess it would be useful if it would be the other way around: -w for closing the tab including children, and -o or something to specify you (o)nly want to close the parent without its children.

dtomvan avatar Aug 28 '21 21:08 dtomvan