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

fix: list_menu not accounting for index + indicator #428

Merged
merged 1 commit into from
May 23, 2022

Conversation

ahkrr
Copy link
Contributor

@ahkrr ahkrr commented May 22, 2022

The problem:

reedline_bug.mp4

The code didn't account for the index and the indicator in its calculation for the number of lines in the list_menu.
Now the unicode_width of the indicator and the number of digits of the index are accounted for.

The code didn't account for the index
and the indicator in its calculation for
the number of lines in the list_menu.
Now the unicode_width of the indicator and the
number of digits of the index are accountet for.
@sholderbach
Copy link
Member

Good catch! The fix looks a bit gnarly but seems to work for me.

@ahkrr
Copy link
Contributor Author

ahkrr commented May 23, 2022

I'm not that attached to my code. If someone has a better solution to fixing the problem, then this PR can be closed and a different solution implemented.

@sholderbach
Copy link
Member

I think with the current setup this is the way to go for now. Refactoring the menu layouting code can come later

@sholderbach sholderbach merged commit c49845e into nushell:main May 23, 2022
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