Skip to content

Conversation

@rafaScalet
Copy link
Contributor

What does this PR do

Close #1337
Add two more session mode, ReattachAlways and ReattachIfNotInSession.
These two modes, are similar to AttachAlways and AttachIfNotInSession, except they won't create a new session if one already exists.

Standards checklist

  • The PR title is descriptive
  • I have read CONTRIBUTING.md
  • Optional: I have tested the code myself
  • If this PR introduces new user-facing messages they are translated

For new steps

  • Optional: Topgrade skips this step where needed
  • Optional: The --dry-run option works with this step
  • Optional: The --yes option works with this step if it is supported by
    the underlying command

If you developed a feature or a bug fix for someone else and you do not have the
means to test it, please tag this person here.

let session_name = "topgrade";
let window_name = "topgrade";
let session = tmux.new_unique_session(session_name, window_name, &command)?;
// let session = tmux.new_unique_session(session_name, window_name, &command)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove commented-out code

let is_inside_tmux = env::var("TMUX").is_ok();
let err = match config.session_mode {
TmuxSessionMode::AttachIfNotInSession => {
session = tmux.new_unique_session(session_name, window_name, &command)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
session = tmux.new_unique_session(session_name, window_name, &command)?;
let session = tmux.new_unique_session(session_name, window_name, &command)?;

let window_name = "topgrade";
let session = tmux.new_unique_session(session_name, window_name, &command)?;
// let session = tmux.new_unique_session(session_name, window_name, &command)?;
let session: String;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not move this to L192?

Comment on lines +65 to +68
# - "attach_if_not_in_session" create and attach in a new unique session, only if isn't in another one
# - "attach_always" create and attach in a new unique session, even if in another one
# - "reattach_if_not_in_session" reuse or create a new session, and attach to if isn't in another one
# - "reattach_always" reuse or create a new session, and attach to, even if in another one)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only if isn't in another one

Either grammatically incorrect or extremely hard to read. Please reword these comments

Comment on lines 355 to +358
AttachIfNotInSession,
AttachAlways,
ReattachIfNotInSession,
ReattachAlways,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of expanding this enum, could you make a new tmux_reattach: bool (or a different name if you like)? This would also remove the duplication that is currently in the match block.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

More tmux session mode

2 participants