Skip to content

Conversation

@whywaita
Copy link
Contributor

@whywaita whywaita commented Sep 12, 2021

Hi, I found this action works very well! Thank you for creating this! 😊

Now, action-tmate only supports using ssh.tmate.io.
This change adds inputs about using other tmate servers.

I referred "Host your own tmate servers" in https://tmate.io/.

Copy link
Collaborator

@dscho dscho left a comment

Choose a reason for hiding this comment

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

I assume you tested this?

Also, since this is relevant only when starting the session, we could potentially pass the custom settings via the command-line, yes?

lib/index.js Outdated
await execShellCommand(`echo 'set +e' >/tmp/tmate.bashrc`);
const setDefaultCommand = `set-option -g default-command "bash --rcfile /tmp/tmate.bashrc" \\;`;

const tmateConfPath = external_path_default().join(external_os_default().homedir(), ".tmate.conf")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use /tmp/tmate.conf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

/tmp directory hasn't not a not UNIX-like OS (e.g. Windows). So I used ${HOME} dir.
But I found that this action uses MSYS2 and MSYS2 can uses /tmp.

I will change to use /tmp/tmate.conf.

Copy link
Contributor Author

@whywaita whywaita Sep 12, 2021

Choose a reason for hiding this comment

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

Also, since this is relevant only when starting the session, we could potentially pass the custom settings via the command-line, yes?

I see. I will change to add setDefaultCommand.

lib/index.js Outdated
const tmateConfPath = external_path_default().join(external_os_default().homedir(), ".tmate.conf")
if (core.getInput("tmate-server-host") !== "") {
core.debug("Configure your tmate server")
await execShellCommand(`echo 'set -g tmate-server-host ${core.getInput("tmate-server-host")}' >> ${tmateConfPath}`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to use $HOME/.tmate.conf, you'll have to quote the path.

@whywaita
Copy link
Contributor Author

@dscho I fixed. Please review again!

@whywaita
Copy link
Contributor Author

I'm sorry, bae5c13 has a mistake. I fixed it in 3aee467

@mxschmitt mxschmitt requested a review from dscho October 4, 2021 12:54
@mxschmitt mxschmitt merged commit 19a49ed into mxschmitt:master Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants