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

Improves the logic of AnsiLoggerWithFooter. #1340

Merged
merged 3 commits into from
Dec 12, 2018

Conversation

coollog
Copy link
Contributor

@coollog coollog commented Dec 12, 2018

Part of #1297

To be more compatible with logging methods with newline at end.

Copy link
Member

@chanseokoh chanseokoh left a comment

Choose a reason for hiding this comment

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

Looks good generally.

plainLogger.accept(footerBuilder.toString());
for (String newFooterLine : newFooterLines) {
plainPrinter.accept(newFooterPrefix + BOLD + newFooterLine + UNBOLD);
newFooterPrefix = "";
Copy link
Member

@chanseokoh chanseokoh Dec 12, 2018

Choose a reason for hiding this comment

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

Hmm... how unfortunate. What do you think of

plainPrinter.accept(TWO_TIMES_CURSOR_UP_SEQUENCE);
for (...) {
  plainPrinter.accept(BOLD + newFooterLine + UNBOLD);
}

? (Note "TWO_TIMES_".)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea that would be ideal, though somehow on Gradle, this sometimes causes a prior-logged line to disappear. Not too sure why that happens though...

Copy link
Member

@chanseokoh chanseokoh Dec 12, 2018

Choose a reason for hiding this comment

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

Ah, OK. Moving one line seems safer.


// If a previous footer was erased, the message needs to go up a line.
String messagePrefix = didErase ? CURSOR_UP_SEQUENCE : "";
messageLogger.accept(messagePrefix + message);
Copy link
Member

@chanseokoh chanseokoh Dec 12, 2018

Choose a reason for hiding this comment

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

Will this work when messageLogger itself is prepending, e.g., "[INFO] "? For example, I wonder if the end string might be "[INFO] + <cursor up> + message", in which case you leave "[INFO] " at the bottom and the actual message line loses "[INFO]".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it works since cursor up goes up in the same column. However, it does replace other message lines' prepended text like [DEBUG] with [INFO].

Copy link
Member

Choose a reason for hiding this comment

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

I see. I think we can tolerate that.

@coollog coollog merged commit 9137f2f into master Dec 12, 2018
@coollog coollog deleted the progress-8.3-improve-AnsiLoggerWithFooter branch December 12, 2018 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants