Skip to content

added trim() to complete command to allow file creation in ssh throug…#455

Merged
steelbrain merged 2 commits into
steelbrain:mainfrom
Nicram97:main
Jan 19, 2023
Merged

added trim() to complete command to allow file creation in ssh throug…#455
steelbrain merged 2 commits into
steelbrain:mainfrom
Nicram97:main

Conversation

@Nicram97

Copy link
Copy Markdown
Contributor

Hi I've spotted problem with command execution when You want to use node-ssh to pass for example yaml structure in command with EOT at the end and empty options array it adds not wanted "space" and it breaks file creation. For that reason I've added trim() to fix the issue if no options are available.

Thank You,
Nicram97

…h command. Empty parameters array would add empty space after EOT.

@steelbrain steelbrain left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for pointing this out, left a comment

Comment thread src/index.ts Outdated
}

const completeCommand = `${command} ${shellEscape(parameters)}`
const completeCommand = `${command} ${shellEscape(parameters)}`.trim();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Instead of .trim at the end, let's take an alternative route, and do something like the following

`${command}${parameters.length ? ` ${shellEscape(parameters)}` : ''}`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I pushed alternative way like You've suggested.
Thank You

@steelbrain steelbrain left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for working on this!

@steelbrain steelbrain merged commit c9252ec into steelbrain:main Jan 19, 2023
@steelbrain

Copy link
Copy Markdown
Owner

This changeset has been deployed in node-ssh@13.0.1

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.

2 participants