Skip to content

Commit

Permalink
chore: change terminal launch behaviour
Browse files Browse the repository at this point in the history
On Linux, the `TERM` variable is not really appropriate for selecting which terminal is in use, so
this has been removed. It can be set to a completely different value, like `xterm-256color`, which
does not necessarily correspond to the running terminal. The `sudo` was also removed from the
terminal launch mechanism.

On the Windows front, I updated Windows Terminal to not use the `/c` argument. It simply just passes
the command to the terminal.

Also fixed another couple of issues:
* Compile the node manager correctly on Windows.
* The use of `<service>` in doc comments was being flagged as an unclosed HTML tag.
  • Loading branch information
jacderida authored and joshuef committed May 10, 2024
1 parent c1266bc commit 94591d7
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 33 deletions.
40 changes: 8 additions & 32 deletions node-launchpad/src/bin/tui/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,32 +63,11 @@ pub(crate) fn detect_and_setup_terminal() -> Result<TerminalType> {
Ok(TerminalType::MacOS(PathBuf::from("osascript")))
}
} else {
get_linux_terminal()
get_available_linux_terminal()
}
}

fn get_linux_terminal() -> Result<TerminalType> {
match std::env::var("TERM") {
Ok(val) => {
if let Ok(path) = which(val.clone()) {
match val.as_str() {
"alacritty" => Ok(TerminalType::Alacritty(path)),
"gnome" => Ok(TerminalType::Gnome(path)),
"kitty" => Ok(TerminalType::Kitty(path)),
"konsole" => Ok(TerminalType::Konsole(path)),
"xterm" => Ok(TerminalType::Xterm(path)),
"xterm-256color" => Ok(TerminalType::Xterm(path)),
_ => Err(eyre!("Terminal '{val}' is not supported")),
}
} else {
try_available_linux_terminals()
}
}
Err(_) => try_available_linux_terminals(),
}
}

fn try_available_linux_terminals() -> Result<TerminalType> {
fn get_available_linux_terminal() -> Result<TerminalType> {
if let Ok(path) = which("alacritty") {
Ok(TerminalType::Alacritty(path))
} else if let Ok(path) = which("gnome-terminal") {
Expand Down Expand Up @@ -117,29 +96,26 @@ pub(crate) fn launch_terminal(terminal_type: &TerminalType) -> Result<()> {
TerminalType::Alacritty(path) => {
Command::new(path)
.arg("--command")
.arg("sudo")
.arg("sh")
.arg("-c")
.arg(launchpad_path)
.spawn()?;
Ok(())
}
TerminalType::Gnome(path) => {
Command::new(path)
.arg("--")
.arg("sudo")
.arg(launchpad_path)
.spawn()?;
Command::new(path).arg("--").arg(launchpad_path).spawn()?;
Ok(())
}
TerminalType::MacOS(_path) | TerminalType::ITerm2(_path) => {
// Mac automatically opens a new terminal window
// so nothing to do here.
Ok(())
}
TerminalType::WindowsCmd(path)
| TerminalType::WindowsPowershell(path)
| TerminalType::WindowsTerminal(path) => {
TerminalType::WindowsTerminal(path) => {
Command::new(path).arg(launchpad_path).spawn()?;
Ok(())
}
TerminalType::WindowsCmd(path) | TerminalType::WindowsPowershell(path) => {
Command::new(path).arg("/c").arg(launchpad_path).spawn()?;
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion sn_node_manager/src/bin/cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub enum SubCmd {
///
/// If not provided, the default location is platform specific:
/// - Linux/macOS (system-wide): /var/log/safenode
/// - Linux/macOS (user-mode): ~/.local/share/safe/node/<service>/logs
/// - Linux/macOS (user-mode): ~/.local/share/safe/node/*/logs
/// - Windows: C:\ProgramData\safenode\logs
#[clap(long, verbatim_doc_comment)]
log_dir_path: Option<PathBuf>,
Expand Down
2 changes: 2 additions & 0 deletions sn_node_manager/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub fn get_node_manager_path() -> Result<PathBuf> {

#[cfg(windows)]
pub fn get_node_manager_path() -> Result<PathBuf> {
use std::path::Path;
let path = Path::new("C:\\ProgramData\\safenode-manager");
if !path.exists() {
std::fs::create_dir_all(path)?;
Expand Down Expand Up @@ -86,6 +87,7 @@ pub fn get_node_registry_path() -> Result<PathBuf> {

#[cfg(windows)]
pub fn get_node_registry_path() -> Result<PathBuf> {
use std::path::Path;
let path = Path::new("C:\\ProgramData\\safenode-manager");
if !path.exists() {
std::fs::create_dir_all(path)?;
Expand Down

0 comments on commit 94591d7

Please sign in to comment.