Adds create option to attach #587
Conversation
- Adds create option to attach that will create a new session if one doesn't already exists with the given name - Small refactor to remove code duplication
pedrohjordao
force-pushed
the
create-on-attach
branch
from
71dd4ef to
1cb9b5f
Compare
There was a problem hiding this comment.
Hey @pedrohjordao! Thank you for taking this up 🙂! It is a much-needed feature.
This PR is going in the right direction, but there are a couple of cases that need to be handled differently. I've mentioned them below. Let me know if more clarifications are needed.
| let client_info = match opts.command.clone() { | ||
| Some(Command::Sessions(Sessions::Attach {mut session_name, force, create})) => { | ||
| let is_new_session = match (session_name.as_ref(), create) { | ||
| (Some(_), true) => true, |
There was a problem hiding this comment.
In this case, it looks like it will always create a new session. We need to check if a session exists or not. In case it exists attach to it, otherwise create a new one.
You will probably have to change the assert_session() function (and maybe rename it) to return a bool corresponding to whether a session exists or not. And we can possibly remove the assert_session_ne() method in favour of this one.
| (None, _) => { | ||
| session_name = Some(get_active_session()); | ||
| false |
There was a problem hiding this comment.
In this case, we need to consider the --create flag as well-
- Flag is not present- If a single session exists attach to it, otherwise exit with a message.
- Flag is present- Is a single session exists, attach to it. If no session exists, create a new one with a random name (generated using
names::Generator). If >1 sessions exist, exit with a message.
|
Thanks, I'll take your comments and update the pr soon |
|
Just a heads up: I still plan on working on this, but I'm in the middle of a change of jobs and country, so it might take a couple of weeks until a can pick this up again. If anyone wants to work on it, feel free. |
|
Thank you for the work on this feature! |
Closes #541