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
Color output and --no-color option #307
Conversation
This commit if included will: 1. Adds color to warnings, 2. Adds a --no-color option to switch off colored output, 3. Adds a test case to cover --no-color option
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.
@Nikhil0487 Thank you for your contribution! 🔥
I have left some comments. Please look at them 👀
…inter into dotenv-color Pulling remote and master changes to local
@mgrachev Suggested changes added. Please verify |
While pulling master, I get already up to date. But after pushing I get a conflict. |
I have noticed the issue colored-rs/colored#59 that it doesn't wotk on Windows. |
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
==========================================
- Coverage 97.07% 97.05% -0.03%
==========================================
Files 33 33
Lines 2363 2379 +16
==========================================
+ Hits 2294 2309 +15
- Misses 69 70 +1
Continue to review full report at Codecov.
|
@mgrachev I have gone through the issue colored-rs/colored#59. Seems this can be fixed by enabling virtual terminal flag. I have added it but I have no Windows machine to check it manually. |
@mgrachev will check other suggested changes. |
This commit if included will: 1. Change file line and number to italics, 2. Code review changes to common_output function, 3. Test cases changes for points 1, 2
@mgrachev Suggested changes except --no-color bug, and a way to test --no-color option pushed. Will figure out changes w.r.t --no-color. |
@Nikhil0487 Thank you! I'm going to merge this PR after v2.2.1 release. |
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 for your PR! 👍
I will also try to return to the topic of testing. But at the moment I have no complete ideas 🤔
@Nikhil0487 Please resolve conflicts. |
…into dotenv-color
I have fixed some tests and resolved conflicts. Please review. Also, I have colored "No problems found" text to green. Is that ok. |
Since we're coloring texts, can we consider the following:
That is in short since we're applying colors anyway, can we audit coloring for all texts and make it more colorful? |
I think we can do it a bit later. Please open issues for each items with screenshots and we will discuss them. |
I don't have any ideas right now. Maybe @dotenv-linter/core you have? |
@Nikhil0487 Awesome work 🔥 Thank you! 👍 Please add information about this feature to dotenv-linter.github.io. |
This commit if included will:
✔ Checklist: