-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add a warning emoji ⚠️ to the logger #588
Add a warning emoji ⚠️ to the logger #588
Conversation
Codecov Report
@@ Coverage Diff @@
## master #588 +/- ##
==========================================
- Coverage 88.85% 88.68% -0.17%
==========================================
Files 57 57
Lines 1749 1750 +1
==========================================
- Hits 1554 1552 -2
- Misses 195 198 +3
Continue to review full report at Codecov.
|
src/Logger.js
Outdated
@@ -42,7 +42,7 @@ class Logger { | |||
return; | |||
} | |||
|
|||
this.write(this.chalk.yellow(message)); | |||
this.status(emoji.warning, message, 'yellow'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduces a logging defect. If I'm using a logLevel
of 2, warn
should write. With this change, it will be silenced by the check in this.status
which requires a logLevel
of 3.
A test for such a regression is included in #562.
src/utils/emoji.js
Outdated
@@ -4,3 +4,4 @@ const supportsEmoji = process.platform !== 'win32' || process.env.VSCODE_PID; | |||
exports.progress = supportsEmoji ? '⏳' : '∞'; | |||
exports.success = supportsEmoji ? '✨' : '√'; | |||
exports.error = supportsEmoji ? '🚨' : '×'; | |||
exports.warning = supportsEmoji ? '⚠️' : '⚠'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided Windows fallback '⚠'
doesn't actually work on Windows and will render as []
. Please use ‼
(203C).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I pushed a new commit with both issues fixed.
e84433a
to
c069ee4
Compare
because we all love emojis, don't we?