first pass on cursor tracking select focus

[vilmibm]

Fixes #305

This solution is inelegant and brittle but it does work 😅 please let me know if I should add more test coverage.

[AlecAivazis]

Hey @vilmibm - thanks for submitting this! I'll try to find some time this weekend to pull it down and give it a shot.

It's been awhile since I've looked at the code for paginate but I think you should be able to use the second returned value to figure out how many lines to move up (assuming that the cursor is always starting at the bottom of a list pageSize long). If that doesn't work for some reason, then you would need to have some other way of identifying the correct line and your approach would make sense. Here are the tests for paginate so you can get a feeling for how it behaves.

EDIT: it might make sense to bake this directly into paginate so we get accessibility "for free" in the multi-line inputs 🤔

[vilmibm]

Using the paginate output is a great idea; I don't know why my eyes just glossed right past the output of paginate.

I'll add support for wide option lines and get this cleaned up.

EDIT: regarding adding to paginate; I believe that would require it having access to a Renderer, right? paginate is nicely independent of the prompts that make use of it right now.

[AlecAivazis]

Ah yea, good point. Maybe a reasonable solution is to add a method to Renderer that invokes and returns the values from paginate along with moving the cursor? That will keep the actual page logic pure and easily testable but still give us the behavior without having to implement it in multiple places. Thoughts?

[vilmibm]

Let me know what you think of the current approach; it's not the most elegant (the width() method for the prompts is a brittle thing) but I think it keeps things testable / comprehensible.

Unfortunately I broke multi-select rendering so I'll fix that tomorrow; if you are ok with this approach though in general I'll go ahead and add tests as needed.

[vilmibm]

I was waylaid by work stuff but finally marking this as non-draft. This solution is not the best but it does work; it will be a little annoying to support it in new prompt types, but it will weather icons of variable length when computing the width/word wrapping of long options.

[AlecAivazis]

Sorry i haven't gotten to this yet @vilmibm. work was really busy this week, will try to find some time to look at it over the weekend

[AlecAivazis]

Sorry this took me so long to look at! I'm buying a home and have been very busy running back and forth to wrangle all of the moving parts. Thanks for being patient

[vilmibm]

@AlecAivazis buying a house seems so stressful -- no rush on my part and good luck!

I followed up on those two comments, but I think I have some test fixing to do 😅 I guess because I'm injecting more control sequences the virtual terminal tests need to be reconciled? I'll start work on that; let me know if you have any protips.

EDIT: actually it looks like the failing tests from a previous commit of my may have been flakes? They pass fine locally on Ubuntu and looking at them in Actions it feels like an actions problem (availability blip or something). I guess we'll find out once the tests are approved to run again.

[AlecAivazis]

Oh damn sorry I didn't realize they were still pending. GitHub didn't notify me about your edit so I totally missed that. I just kicked them off

[AlecAivazis]

Finally found some time to pull this down and test it. The select and multi select tests all ran smoothly (located at tests/{select.go, multiselect.go} but if I ran another test afterwards, the prompt would print many lines above the starting line. I think there is a problem in the way the select and multiselect prompts clean up

[vilmibm]

hm, can you help me with some more context?

I'm running this locally:

go test -v github.com/AlecAivazis/survey/v2 github.com/AlecAivazis/survey/v2/core github.com/AlecAivazis/survey/v2/terminal -mod=vendor which is failing for me locally against both master and my PR for reasons seemingly unrelated to my changes. It seems like they are passing in CI, though?

How are you triggering the failure?

[AlecAivazis]

Ah yea sure. So there is this directory which contains scripts, not actual tests, that are meant to be run manually in order to verify that things behave in a real scenario. If you run the select and multi select tests everything behaves correctly but if I run the confirm test (just typing go run tests/confirm.go from the root of the repo) i see the issue

[vilmibm]

@AlecAivazis ah oops, I screwed up the tiny refactor and was restoring the cursor when I shouldn't have been by accident. Fixed now; running through the stuff in tests/ seems to work fine.

[AlecAivazis]

Awesome! Thanks for hunting that down! I finally found the time to pull down your branch and test it out. Sorry that took so long - was on vacation for 10 days.

I'm pretty sure this is good to go but I want to give it one more go around. Pretty excited we should be getting this in soon

[AlecAivazis]

I'm going to finally merge this in. Thanks so much for implementing this @vilmibm!

[vilmibm]

awesome news! thank you!!

[billygriffin]

Great news @AlecAivazis, thanks for working with us to get it across the finish line. And great work @vilmibm! 🎉 ❤️