AlecAivazis / survey

A golang library for building interactive and accessible prompts with full support for windows and posix terminals.
MIT License
4.08k stars 351 forks source link

[#101] fix incorrect line count on narrow terminals (Builds on #288) #291

Closed Zalgo2462 closed 4 years ago

Zalgo2462 commented 4 years ago

This PR builds on https://github.com/AlecAivazis/survey/pull/288 to fix issue #101 where questions would be repeated if the terminal causes the question to word wrap onto another line.

@coryb's PR introduced another bug where the prompt would climb up the screen backtracking over previous output. @coryb's approach to fixing the issue is to divide each line's length by the terminal width to figure out how many times the line will be wrapped.

This approach works well, but unfortunately, each line contains ANSI color codes which do not change the length of the displayed string. In this case, len(line) does not return the displayed string length. To fix this, I've changed the code to generate both colorful and colorless templates for each prompt. The renderer can then use the colorless version for layout planning while displaying the colored version to the user.

I ran through the integration test suite and found a bug in ask.go where the same prompt object was being passed to Ask twice. Since renderer.lineCount never gets cleared out after the first call, the renderer would backtrack up the console at the second call when it wasn't supposed to.

ethack commented 4 years ago

This works well for me on narrow windows.

There's one remaining edge case I encountered. It happens if you change the terminal size after a multi select prompt is already displayed and then move the selection with the arrow keys. It's not super likely but I could see someone starting a prompt on a narrow terminal and realizing the text is wrapping and make the window bigger.

Zalgo2462 commented 4 years ago

That edge case makes sense. Survey calculates how many lines to backtrack for a given prompt when it is written to the console. If the window size changes, the number of lines to backtrack may change as well. To get around that, we'd have to calculate the number of lines to backtrack for the last prompt rendered for each render call. Instead of calculating the number of lines to backtrack for the given prompt, storing that number, and then backtracking that many lines on the next render, we would store the first prompt, display the first prompt, call to render the second prompt, calculate how many lines are displayed for the previous (stored) prompt, backtrack over them, and finally display the second message.

AlecAivazis commented 4 years ago

Thank you for investigating this so thoroughly!

It sounds like you are right on the cusp of the final solution so I will hold back on reviewing the actual code until then. In general, the approach sounds reasonable and platform independent which is nice.

tomercy commented 4 years ago

For some reason the original issue still remains... when I enter a big answer, JWT string for example, the question and the answer gets duplicated

Zalgo2462 commented 4 years ago

I hadn't considered the fact that the user might provide word-wrapped input for the input, multiline, and password prompt types. I've pushed up a couple commits to remedy the issues posted by @ethack and @tomercy

Zalgo2462 commented 4 years ago

Tonight, I found a new bug where if you use tabs in the prompt, the wrapping calculation does not work. However, I'm not sure there is an easy way around that.

AlecAivazis commented 4 years ago

I finally got some time to pull this down and test it out. I'm super excited we have made so much progress on this. Thank you for pushing on this so hard @Zalgo2462. I'm going to merge this into cory's PR and we can continue the conversation there.

AlecAivazis commented 4 years ago

Whoops! I completely misread Github's UI and thought this was merging onto the original PR, not onto master.

AlecAivazis commented 4 years ago

Well overall it looks pretty good so i won't nitpick it too much. Thanks again!!