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.

Sign up for GitHub

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

Jump to bottom

Correct multi-line description wrapping #94

Merged
merged 2 commits into from
Aug 3, 2023
Merged

Correct multi-line description wrapping #94

merged 2 commits into from
Aug 3, 2023

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Aug 2, 2023

Fixes #93.

@jonhoo
Copy link
Contributor Author

jonhoo commented Aug 2, 2023

Hmm, I tried adding a changelog entry with slap changelog add, but got the error:

error: cannot add changelog because the feature must be enabled in the config

🤔

tetigi
tetigi approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@tetigi tetigi left a comment

Choose a reason for hiding this comment

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

One nit; leave styling choice to your discretion ;)

@tetigi
Copy link
Contributor

tetigi commented Aug 2, 2023

Hmm, I tried adding a changelog entry with slap changelog add, but got the error:

error: cannot add changelog because the feature must be enabled in the config

🤔

Likely you ran it from the root dir, and not the sub-dir (or vice versa)?

@jonhoo
Copy link
Contributor Author

jonhoo commented Aug 2, 2023

That was exactly it, thanks! Force-pushed so that we get a single nicely-formatted commit.

@tetigi tetigi merged commit c94504a into kraken-build:develop Aug 3, 2023
13 checks passed
@jonhoo jonhoo deleted the wrap-subsequent-lines branch August 3, 2023 14:26
jonhoo added a commit to jonhoo/kraken-build that referenced this pull request Aug 4, 2023
@jonhoo
kraken-core/: fix: Rework task description wrapping
3c115e5 
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.
@jonhoo jonhoo mentioned this pull request Aug 4, 2023
Merged
ntrinquier pushed a commit that referenced this pull request Aug 4, 2023
@jonhoo
kraken-core/: fix: Rework task description wrapping (#98)
c3a8dff 
* kraken-core/: fix: Rework task description wrapping

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.

* Updated PR references in 1 changelogs.

skip-checks: true

---------

Co-authored-by: GitHub Action <github-action@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NiklasRosenstein NiklasRosenstein NiklasRosenstein left review comments

@tetigi tetigi tetigi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect line wrapping for tasks with long descriptions
3 participants
@jonhoo @tetigi @NiklasRosenstein