Skip to content

Automatic code setup: Further improvements#1223

Open
t-reents wants to merge 1 commit intoaiidateam:mainfrom
t-reents:imp/auto-code-setup-escape
Open

Automatic code setup: Further improvements#1223
t-reents wants to merge 1 commit intoaiidateam:mainfrom
t-reents:imp/auto-code-setup-escape

Conversation

@t-reents
Copy link
Collaborator

@t-reents t-reents commented Dec 5, 2025

Addressing remaining aspects in #1156 (in addition to the changes in #1222)

  • Add info that connection might take a while
  • Escape executable name when searching for it on remote computer

* Add info that connection might take a while
* Escape executable name when searching for it on remote computer
@t-reents t-reents requested a review from mbercx December 5, 2025 13:39
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

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

Thanks for the work @t-reents! I have a few questions before merging. :)


try:
echo.echo_report(
'Locating executables on the remote computer. Depending on the connection, this may take a while...'
Copy link
Member

Choose a reason for hiding this comment

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

The computer isn't necessarily remote though? I suppose we only need to warn the user here in case the transport isn't core.local? That should not take a while. ^^

Nitpick/bike shedding: should we use echo_info instead?

combined_prepend_text = f'{computer.get_prepend_text()}\n{prepend_text}'
return_value, stdout, stderr = transport.exec_command_wait(
command=f'. /dev/stdin > /dev/null && which {executable}', stdin=combined_prepend_text
command=f'. /dev/stdin > /dev/null && which {escape_for_bash(executable)}',
Copy link
Member

Choose a reason for hiding this comment

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

If we already add validation on the executables in #1222, is this still necessary?

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