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

pulumi new show additional templates is unusable #10078

Closed
joeduffy opened this issue Jul 8, 2022 · 25 comments
Closed

pulumi new show additional templates is unusable #10078

joeduffy opened this issue Jul 8, 2022 · 25 comments
Assignees
Labels
area/cli UX of using the CLI (args, output, logs) impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone

Comments

@joeduffy
Copy link
Member

joeduffy commented Jul 8, 2022

If you select "Show additional templates", you get a super long list, and trying to move the cursor beyond the top one doesn't scroll the screen so you ca't actually see what you're trying to select:

image

This makes it basically unusable for selecting anything that doesn't fit on the screen.

@joeduffy joeduffy added the needs-triage Needs attention from the triage team label Jul 8, 2022
@iwahbe iwahbe added kind/bug Some behavior is incorrect or out of spec area/cli UX of using the CLI (args, output, logs) impact/usability Something that impacts users' ability to use the product easily and intuitively labels Jul 11, 2022
@iwahbe iwahbe added this to the 0.75 milestone Jul 11, 2022
@iwahbe iwahbe removed the needs-triage Needs attention from the triage team label Jul 12, 2022
@iwahbe
Copy link
Member

iwahbe commented Jul 12, 2022

It looks like like this a problem with the library we use to display choice lists. We should explore choosing a new library or dumping out of interactive mode once a certain number of options is displayed.

@AaronFriel
Copy link
Member

Related, the length of this list causes awful flickering on re-rendering in PowerShell on Windows:

@iwahbe
Copy link
Member

iwahbe commented Jul 12, 2022

Is causes flickering on the default terminal on Mac too.

@dixler
Copy link
Contributor

dixler commented Jul 12, 2022

I have some comments on help output here https://github.com/pulumi/home/pull/2164#discussion_r912160412__

Which is also bad

@aq17
Copy link
Contributor

aq17 commented Jul 13, 2022

@AaronFriel @iwahbe
I tried playing around with another library as well as updating our current library from v1 -> v2, and still got the same issue – it looks like the problem is actually that we try to display all options on one page, which probably exceeds the limit and causes the buggy behavior. We need to use something smaller (the default is 7) and paginate instead which should fix the issue

Not sure about the flickering, the other library also had it

@aq17 aq17 self-assigned this Jul 13, 2022
@aq17 aq17 mentioned this issue Jul 13, 2022
3 tasks
@dixler
Copy link
Contributor

dixler commented Jul 15, 2022

I'm surprised that changing the prompt library is on the table.

@iwahbe i think @aq17 was trying to tag you

@AaronFriel
Copy link
Member

@aq17 I'm OK with changing the prompt library if it flickers less. Could we do a side by side comparison?

@dixler
Copy link
Contributor

dixler commented Jul 17, 2022

Can we keep the output/coloration identical to the current?

@aq17
Copy link
Contributor

aq17 commented Jul 18, 2022

@AaronFriel @dixler I can work on swapping the library but would we want to change it for all pulumi CLI output, or just for pulumi new?

@AaronFriel
Copy link
Member

@aq17 Have you checked whether it solves the flickering issue for pulumi new first? If it does, then I'd say tentatively yes to replacing it everywhere.

@aq17
Copy link
Contributor

aq17 commented Jul 18, 2022

@AaronFriel the flickering issue persists even when using this other library , which leads me to believe it might not be a library specific issue...

@iwahbe
Copy link
Member

iwahbe commented Jul 19, 2022

The flickering is a fundamental property of most terminal emulators (Including Terminal.app). They can only process so many new writes at once. It can be avoided in some cases by taking full control of the screen and using cursor manipulation instead of rewriting sections. I think the only solution here is to limit the number of templates we list one command.

That being said, I'm not opposed to bumping v1 -> v2 if it fixes problems for us.

@aq17
Copy link
Contributor

aq17 commented Jul 19, 2022

The flickering is a fundamental property of most terminal emulators (Including Terminal.app). They can only process so many new writes at once. It can be avoided in some cases by taking full control of the screen and using cursor manipulation instead of rewriting sections. I think the only solution here is to limit the number of templates we list one command.

That being said, I'm not opposed to bumping v1 -> v2 if it fixes problems for us.

Thanks for confirming @iwahbe , I don't believe v1->v2 changes anything of consequence then

@mikhailshilkov mikhailshilkov modified the milestones: 0.75, 0.76 Jul 25, 2022
@AaronFriel
Copy link
Member

I think to close this out, we should pick a maximum number of templates to display and allow that to be good enough.

@dixler I'm worried that the flickering is a bigger UX issue than not showing enough items, it's very distracting on Windows, it's OK in some terminals and worse in others. What do you think is a good # of templates to display, maximum?

@dixler
Copy link
Contributor

dixler commented Jul 26, 2022

I think to close this out, we should pick a maximum number of templates to display and allow that to be good enough.

@dixler I'm worried that the flickering is a bigger UX issue than not showing enough items, it's very distracting on Windows, it's OK in some terminals and worse in others. What do you think is a good # of templates to display, maximum?

45, but didn't we have terminfo to dynamically determine the terminal height and change the page size to the number of rows in height?

Could we put a flag to change the behavior for inefficient terminals/environments that we know of and tweak the number to make the experience less miserable for that edge case?

@AaronFriel
Copy link
Member

I don't think we can reliably know which terms are going to be slow to render - could even depend on what other programs are running, how quick their SSH connection is if remote, etc.

45 seems like a lot, the flickering is proportional to the # of lines (or characters?) written, so rendering 5 options is much snappier than 45.

@dixler
Copy link
Contributor

dixler commented Jul 26, 2022

45 seems like a lot, the flickering is proportional to the # of lines (or characters?) written, so rendering 5 > options is much snappier than 45.

Thanks for clarifying. I understood the question as absolute max.

We have ~100 templates.

It's really about the number of pages that a user will have to page through to find a template.

Does 5 result pages(~20 rows) in order to view everything seems like a reasonable goal?

Trying to interactively view ~100 templates is really clunky if you're going at 5 results at a time.

We could also just remove the selector and reconsider the prompt, BUT there's a lot of recordings and screenshots that depend on this selector display and users say it's unsettling when demos/examples don't match up with their experience.

I don't think we can reliably know which terms are going to be slow to render - could even depend on what other programs are running, how quick their SSH connection is if remote, etc.

My suggestion is to leave the UX the same for existing CLI user or users that are in environments that are unaffected and warn that we are running this command users when we're doing this(with a toggle to disable the warning)

We can identify known poorly performing terminal environments and handle the poor performers similar to our CI detection. They may run into other performance issues with rendering and it could make sense down the line to be able to use this flag.

tl;dr my suggestion is:

  • detect terminal environments where flickering is typical
  • warn the user that that we're altering the display for those environments and either
    • reduce page size
    • change the prompt

@AaronFriel
Copy link
Member

Perhaps we shouldn't display anything unless a user searches, and hint that they can type a language (csharp, yaml, go...) or a provider (AWS, Azure, ...)?

@iwahbe
Copy link
Member

iwahbe commented Jul 26, 2022

I agree that we should bias users towards search. We don't expect you to scroll through all the templates. I'm not confident that we can detect which terminals flicker, detecting terminal environments is quite difficult.

@dixler
Copy link
Contributor

dixler commented Jul 26, 2022

Perhaps we shouldn't display anything unless a user searches, and hint that they can type a language (csharp, yaml, go...) or a provider (AWS, Azure, ...)?

we could prompt for multiple selectors for prefered language(or none) and then cloud provider(or none) to search/filter the examples.

@aq17
Copy link
Contributor

aq17 commented Jul 26, 2022

I also like that idea @AaronFriel , maybe we could display 10 or so of the most commonly used templates with no further scrolling, and change the prompt to encourage users to search for additional ones

@ekini
Copy link
Contributor

ekini commented Aug 2, 2022

Could use something like https://github.com/sahilm/fuzzy to display templates and allow interactive selection.

@dixler
Copy link
Contributor

dixler commented Aug 2, 2022

Could use something like https://github.com/sahilm/fuzzy to display templates and allow interactive selection.

So afaik, you can type into the prompt for fuzzy finding string matching. It's just not very obvious because "type to filter" is vague.

Edit: I use Fzf so I corrected myself.

@aq17 aq17 closed this as completed Aug 10, 2022
@pulumi-bot pulumi-bot reopened this Aug 10, 2022
@pulumi-bot
Copy link
Contributor

Cannot close issue without required labels: resolution/

@aq17 aq17 closed this as completed Aug 10, 2022
@aq17
Copy link
Contributor

aq17 commented Aug 10, 2022

Closing now as original issue has been resolved – can track flickering in separate issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli UX of using the CLI (args, output, logs) impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

8 participants