Skip to content
This repository has been archived by the owner on Jun 10, 2020. It is now read-only.

Don't include DeleteLine in progress length #227

Merged
merged 1 commit into from
Oct 7, 2019
Merged

Don't include DeleteLine in progress length #227

merged 1 commit into from
Oct 7, 2019

Conversation

eatkins
Copy link
Contributor

@eatkins eatkins commented Oct 7, 2019

I incorrectly included the DeleteLine in the progress line length and
this could cause certain progress lines to be incorrectly reported as
multi line when they actually fit on a single terminal line.

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

[warn] /home/travis/build/sbt/util/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala isn't formatted properly!

@eatkins
Copy link
Contributor Author

eatkins commented Oct 7, 2019

doh!

I incorrectly included the DeleteLine in the progress line length and
this could cause certain progress lines to be incorrectly reported as
multi line when they actually fit on a single terminal line.
@eatkins eatkins requested a review from eed3si9n October 7, 2019 18:45
@eed3si9n
Copy link
Member

eed3si9n commented Oct 7, 2019

Wouldn't it still be off if the original line contained some color stuff?

@eatkins
Copy link
Contributor Author

eatkins commented Oct 7, 2019

Yes, but why would the task name have color though? Is there an api that would tell you the printable length of a string including escape sequences? I think this is a reasonable approach that is better than the status quo. A more robust solution would be welcome though.

@eed3si9n
Copy link
Member

eed3si9n commented Oct 7, 2019

Yes, but why would the task name have color though?

oh I thought it's for log lines. For task names this should be ok.

Is there an api that would tell you the printable length of a string including escape sequences? I think this is a reasonable approach that is better than the status quo. A more robust solution would be welcome though.

Not sure how performant it is but there used to some code in sbt 0.13 - https://github.com/sbt/sbt/blob/v0.13.18/util/log/src/main/scala/sbt/ConsoleLogger.scala#L148-L150

@eatkins eatkins merged commit 166511c into sbt:develop Oct 7, 2019
@eatkins
Copy link
Contributor Author

eatkins commented Oct 7, 2019

If I get a chance, I'll take a look at that.

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.

None yet

2 participants