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

Prompt to select instance from list #40

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jcam
Copy link

@jcam jcam commented Jan 28, 2023

This is a feature I find myself using pretty often, because the instance "name" is typically the same for multiple instances in an autoscaling group.

@mludvig
Copy link
Owner

mludvig commented Feb 21, 2023

Hi Jesse
Thanks for the PR. Two thoughts on that:

  1. If this is merged it should be consistent across all the tools and all the modes of operation. For example it's quite counter-intuitive to support this only when more than one instance matches. Why not support interactive selection without any instance selection at all (i.e. list them all and let me pick). And it should work the same in ecs-session when selecting containers to connect to.
  2. It should be an optional behaviour, e.g. with --interactive / -i or maybe optionally disabled with --no-interactive for use in scripts.

Also I believe there are some libraries that can make such menu selection more user friendly, something like the node.js menu system where one can use cursor keys to make the selection.

What do you think?

@jcam
Copy link
Author

jcam commented Mar 20, 2023

I guess I was trying to make only limited scope changes. if exactly one instance matches then why prompt to pick it?
Interactive selection without any select (list all) seems like a reasonable change
making it optional (so it doesn't break scripts) seems reasonable as well.

is it just this and ecs-session that would leverage this? i could update that to match.

As for more-user-friendly menu selection, if you have one you recommend that won't add a massive amount to the library, sure, i was going for simplest with what is already in it.

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