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

kraken-core/: fix: Rework task description wrapping #98

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Aug 4, 2023

This is a follow-up to #94. It started because I noticed a boundary-condition bug where descriptions that came all the way up to right side of the terminal seemed to have an extra newline added to them. That turned out to be because every line got an extra space added to it, so all the lines ended up one character too long, and then regular terminal wrapping kicked in.

This caused me to dig further into how exactly the description-rendering code was set up, which led me to find some more oddities. So I decided to make two bigger changes to the code.

First, it no longer joins line by spaces, but instead joins by \n, with every string-append to a line prefixing its own separator (+= " "). This makes it easier to determine where line breaks actually come from, and avoids spurious spaces from appearing.

And second, if there is so little room remaining on a line that it would look weird when wrapped (see code comment for how that's evaluated), the description is now moved to the next line and wrapped independently to 80 characters wide. If this happens, extra vertical spacing is also added before the next task.

I've tested this by adjusting the description and task status of a number of tasks and running kraken q ls to verify that the output (subjectively) looks readable and sane. That's also how I landed on 48 characters as the "right" place to flip between column and next-line mode.

Some screenshots:
Screenshot 2023-08-04 at 11 00 33
Screenshot 2023-08-04 at 11 01 55
Screenshot 2023-08-04 at 11 02 25

jonhoo and others added 2 commits August 4, 2023 10:55
This is a follow-up to kraken-build#94. It started because I noticed a
boundary-condition bug where descriptions that came all the way up to
right side of the terminal seemed to have an extra newline added to
them. That turned out to be because every line got an extra space added
to it, so all the lines ended up one character too long, and then
regular terminal wrapping kicked in.

This caused me to dig further into how exactly the description-rendering
code was set up, which led me to find some more oddities. So I decided
to make two bigger changes to the code.

First, it no longer joins `line` by spaces, but instead joins by `\n`,
with every string-append to a line prefixing its own separator (`+= "
"`). This makes it easier to determine where line breaks actually come
from, and avoids spurious spaces from appearing.

And second, if there is so little room remaining on a line that it would
look weird when wrapped (see code comment for how that's evaluated), the
description is now moved to the next line and wrapped independently to
80 characters wide. If this happens, extra vertical spacing is also
added before the next task.

I've tested this by adjusting the description and task status of a
number of tasks and running `kraken q ls` to verify that the output
(subjectively) looks readable and sane. That's also how I landed on 48
characters as the "right" place to flip between column and next-line
mode.
Copy link
Contributor

@ntrinquier ntrinquier left a comment

Choose a reason for hiding this comment

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

Thanks @jonhoo!

@ntrinquier ntrinquier merged commit c3a8dff into kraken-build:develop Aug 4, 2023
@jonhoo jonhoo deleted the better-desc-wrapping branch August 4, 2023 10:43
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.

None yet

2 participants