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

NO_COLOR requires a non-empty string #171

Merged
merged 3 commits into from Jan 18, 2023
Merged

Conversation

pellared
Copy link
Contributor

@pellared pellared commented Nov 13, 2022

Fixes #172

  1. Change the handling of NO_COLOR so that the empty string value as equivalent to being unset
  2. Some opportunistic fixes

@fatih
Copy link
Owner

fatih commented Jan 18, 2023

I would like to merge it, but it looks like a breaking change. NOCOLOR="" ./foo is working currently, but with your change we would break this contract. But then, I think the reasoning you provide at #172 make sense.

@pellared
Copy link
Contributor Author

I would like to merge it, but it looks like a breaking change. NOCOLOR="" ./foo is working currently, but with your change we would break this contract. But then, I think the reasoning you provide at #172 make sense.

It can be seen as a breaking change but for me it is a bugfix for the contract. Especially that https://no-color.org/ is mentioned in the README.md 😉 For sure the change should be clearly (and probably loudly) mentioned in the GitHub release notes. It would be even good to note that setting NOCOLOR="" will have different behavior.

PS. There is no rush for me to merge it. You can consider asking some users for opinion.

@fatih
Copy link
Owner

fatih commented Jan 18, 2023

You're right. Let's merge it. I'll cut a release in the following weeks.

@fatih fatih merged commit 6378f62 into fatih:main Jan 18, 2023
@pellared pellared deleted the no-color-empty-val branch January 18, 2023 18:30
mend-for-github-com bot added a commit to DelineaXPM/dsv-cli that referenced this pull request Feb 6, 2023
#68)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [github.com/fatih/color](https://togithub.com/fatih/color) | require |
minor | `v1.13.0` -> `v1.14.1` |

---

### Release Notes

<details>
<summary>fatih/color</summary>

### [`v1.14.1`](https://togithub.com/fatih/color/releases/tag/v1.14.1)

[Compare
Source](https://togithub.com/fatih/color/compare/v1.14.0...v1.14.1)

#### What's Changed

- Update to Go 1.17 by [@&#8203;pellared](https://togithub.com/pellared)
in
[fatih/color#184

**Full Changelog**:
fatih/color@v1.14.0...v1.14.1

### [`v1.14.0`](https://togithub.com/fatih/color/releases/tag/v1.14.0)

[Compare
Source](https://togithub.com/fatih/color/compare/v1.13.0...v1.14.0)

#### What's Changed

- Bump github.com/mattn/go-colorable from 0.1.12 to 0.1.13 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[fatih/color#165
- Bump go-isatty from 0.0.16 to 0.0.17 by
[@&#8203;fatih](https://togithub.com/fatih) in
[fatih/color#178
- Spelling and grammar fixes by
[@&#8203;pattmax00](https://togithub.com/pattmax00) in
[fatih/color#181
- NO_COLOR requires a non-empty string by
[@&#8203;pellared](https://togithub.com/pellared) in
[fatih/color#171
- color: expose `SetWriter` and `UnsetWriter` by
[@&#8203;fatih](https://togithub.com/fatih) in
[fatih/color#182

#### New Contributors

- [@&#8203;pattmax00](https://togithub.com/pattmax00) made their first
contribution in
[fatih/color#181
- [@&#8203;pellared](https://togithub.com/pellared) made their first
contribution in
[fatih/color#171

**Full Changelog**:
fatih/color@v1.13.0...v1.14.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 3am on Monday" (UTC),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM0LjEwMC4wIn0=-->

Co-authored-by: mend-for-github-com[bot] <50673670+mend-for-github-com[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NO_COLOR and empty string value
2 participants