Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.

Add new rich error messages #150

Merged
merged 1 commit into from
Oct 21, 2020
Merged

Add new rich error messages #150

merged 1 commit into from
Oct 21, 2020

Conversation

cmoog
Copy link
Contributor

@cmoog cmoog commented Oct 21, 2020

Inspired by the cargo message formatting.

Screen Shot 2020-10-21 at 1 08 31 PM

Screen Shot 2020-10-21 at 1 34 53 AM

Screen Shot 2020-10-21 at 1 13 48 PM

@cmoog cmoog marked this pull request as ready for review October 21, 2020 18:09
@cmoog cmoog requested review from coadler and fuskovic October 21, 2020 18:11
Copy link
Contributor

@coadler coadler left a comment

Choose a reason for hiding this comment

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

Looks really good!

}

// LogInfo prints the given info message to stderr.
func LogInfo(header string, lines ...string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused why LogInfo and LogSuccess are the only ones prefixed with Log

Copy link
Contributor Author

@cmoog cmoog Oct 21, 2020

Choose a reason for hiding this comment

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

Yeah this is definitely confusing. The issue I'm having is that Error, Warn, and Fatal return errors. So it would potentially be even more confusing for Info and Success to be the only ones with side-effects.

It gets pretty tricky when we actually do want to log errors as they happen: for example when iterating through a series of stop actions.

Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh i see now, I was skimming over the returns. Considering the others return errors this makes more sense

- show rich log output for Info and Success logs
- preserve rich CLI output for errors when possible
@cmoog cmoog merged commit f6bc6da into master Oct 21, 2020
@cmoog cmoog deleted the clog branch October 21, 2020 21:15
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.

2 participants