Skip to content
This repository has been archived by the owner on Apr 19, 2024. It is now read-only.

Fix re-rendering when prompt is exactly the width of the terminal #321

Merged
merged 3 commits into from
Dec 17, 2020

Conversation

mislav
Copy link
Collaborator

@mislav mislav commented Dec 17, 2020

The code that measures width of rendered lines of content and whether any have overflown in the terminal did not account for the possibility that the content could be exactly the width of the terminal. In that case, it shouldn't be counted as overflow.

Fixes #318
Followup to #291, #288

The code that measures width of rendered lines of content and whether
any have overflown in the terminal did not account for the possibility
that the content could be _exactly_ the width of the terminal. In that
case, it shouldn't be counted as overflow.
Comment on lines 21 to 22
c, _, err := vt10x.NewVT10XConsole(expect.WithStdout(stdout))
vt10x.ResizePty(c.Tty(), termWidth, 30)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test is marked as POSIX-only because it allocates a pseudo-terminal to emulate a scenario when the terminal is of a specific width. If anyone knows of a more lightweight approach to generate a terminal at a file descriptor that pretends to be a certain size, I'm all ears!

This skips the go-expect & vt10x dependencies when stubbing only
terminal size and goes straight to the `creack/pty` library, which is
already used by go-expect under its old name, `kr/pty`.
@AlecAivazis
Copy link
Owner

Thanks for fixing this @mislav - i verified the behavior on both apple and windows.

Thanks for being patient while we wrangle this! Maintaining this project alone takes a lot of time and attention, two things i haven't had much of in the past month.

@AlecAivazis AlecAivazis merged commit e64d9b6 into AlecAivazis:master Dec 17, 2020
@AlecAivazis
Copy link
Owner

On that note, are there any extra resources at GitHub that can help me test PRs and releases? There have been two regressions in the past month that are mostly due to me juggling multiple things at once, i would like to find a way to make sure that doesn't happen again

@mislav mislav deleted the count-lines-fix branch December 18, 2020 12:55
@mislav
Copy link
Collaborator Author

mislav commented Dec 18, 2020

@AlecAivazis Sure, let us know how we can help. We are just 3 devs and don't have infinite resources either, but we rely quite a lot on Survey and we'd like to have it be stable. A lot of problems that our users reported with GitHub CLI were fixed over the last few releases of Survey, and we appreciate that!

We are also wondering whether you would be open to contributions that improve accessibility by positioning the cursor at the right place, and possibly allowing custom keybindings to Prompt components like the Editor.

@mislav
Copy link
Collaborator Author

mislav commented Dec 18, 2020

BTW, thank you for making a patch release in such a short time! 🙇 ❤️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typing into prompt clears previous lines in terminal
2 participants