Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be explicit that agent/container index is being requested #36

Merged
merged 2 commits into from
Dec 31, 2024

Conversation

andyjdavis
Copy link
Contributor

Connect to which container?

It was not immediately clear to me that the index rather than name is being requested on the command line.

STDIN.gets.strip.to_i

This code happily accepts a container name although the to_i turns it into a zero.

This PR modifies the command line prompt to explicitly state that an index should be entered.

Copy link
Member

@orien orien left a comment

Choose a reason for hiding this comment

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

👍

@@ -101,7 +101,7 @@ def select_agent(auto: false)

output_agents

puts "\nConnect to which agent?"
puts "\nConnect to which agent? (INDEX)"
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like the following?

Enter the INDEX number and press ENTER to connect:

My intention is to make the instruction more explicit and also get rid of the unnecessarily mentioning agents or containers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that version. I have updated to it. Commit pushed.

Copy link
Contributor

@domon-envato domon-envato left a comment

Choose a reason for hiding this comment

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

👍

@andyjdavis andyjdavis merged commit 98c2ec9 into master Dec 31, 2024
7 checks passed
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