Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add support for env variables with SFTP client.#1445

Closed
di-ov wants to merge 12 commits intomscdex:masterfrom
di-ov:add-sftp-env-option
Closed

Add support for env variables with SFTP client.#1445
di-ov wants to merge 12 commits intomscdex:masterfrom
di-ov:add-sftp-env-option

Conversation

Copy link

di-ov commented Mar 4, 2025

This relates to and closes this issue: #1433

I basically saw how this was implemented in the exec() method in client.js and did a similar implementation.

I seems that it might be better to add an additional parameter to the sftp() method, i.e. in client.js it would appear as sftp(env, cb), I wasn't entirely sure, what do you think? Thanks!

angelparaskov, unbekanntes-pferd, schaeferm, fbHolzbaum, and aminya reacted with thumbs up emoji
Copy link

angelparaskov commented Mar 4, 2025

@mscdex we also would like to have such a solution.
May we expedite this one please?
Thanks

Copy link
Owner

mscdex commented Mar 4, 2025

I seems that it might be better to add an additional parameter to the sftp() method, i.e. in client.js it would appear as sftp(env, cb)

That would be the preferred approach.

Copy link
Author

di-ov commented Mar 5, 2025

@mscdex thanks for the help, I did the changes. If it looks okay, I'll have a look at updating the docs.

mscdex reviewed Mar 5, 2025
mscdex reviewed Mar 18, 2025
mscdex reviewed Mar 18, 2025
mscdex reviewed Mar 18, 2025
Dimitar Markov and others added 2 commits March 25, 2025 11:34
di-ov requested a review from mscdex March 25, 2025 09:40
mscdex reviewed Mar 25, 2025
di-ov and others added 2 commits March 25, 2025 16:45
Copy link
Owner

mscdex commented Mar 26, 2025

All that's needed now is a unit test. You'll probably have to use one similar to the last one in the SFTP test script to verify the environment is being sent properly before starting the subsystem.

di-ov reacted with thumbs up emoji

di-ov commented Mar 27, 2025
di-ov force-pushed the add-sftp-env-option branch from 9db57fe to 3659269 Compare March 28, 2025 10:02
mscdex reviewed Mar 28, 2025
mscdex reviewed Mar 28, 2025
Co-authored-by: mscdex
di-ov requested a review from mscdex March 31, 2025 06:33
Copy link
Author

di-ov commented Apr 1, 2025

@mscdex so this look okay right? Would you be able to include it in the next release?

Copy link

unbekanntes-pferd commented Apr 9, 2025

@mscdex I'd also be interested in this solution. Anything that can be done to support this / move this forward?

Copy link

angelparaskov commented Apr 23, 2025

Is there anything we could do, to speed this up?

Copy link

vmiroslav commented May 15, 2025

Yea, I really need this ASAP , so if we can help you somehow to speed the process let us know

Copy link
Author

di-ov commented May 27, 2025

@mscdex it's been quite a while since this is open, would you able to at least give some pointers on when you plan to work this project? thanks!

Copy link

angelparaskov commented Aug 12, 2025

Is this repo still alive @mscdex ?
Is there someone else that could approve merge requests?

mscdex pushed a commit that referenced this pull request Aug 20, 2025
Copy link
Owner

mscdex commented Aug 20, 2025

Landed in c19a821.

angelparaskov reacted with rocket emoji

mscdex closed this Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mscdex Awaiting requested review from mscdex

1 more reviewer

angelparaskov angelparaskov left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants