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

Allow to force enable/disable colored output #350

Merged
merged 1 commit into from
Jun 24, 2020
Merged

Allow to force enable/disable colored output #350

merged 1 commit into from
Jun 24, 2020

Conversation

sienkiewiczkm
Copy link
Contributor

Hello!

In most cases click library figures out correctly if colored output should be used, but sometimes it's wrong. For example when watch command is used click is unable to determine this and chooses uncolored output:

watch --color 'watson aggregate'

This change allows user to explicitly enable/disable coloring with --color and --no-color options:

 watch --color 'watson --color aggregate'

Related issue: pallets/click#1090

@jmaupetit
Copy link
Contributor

Thanks for this contribution @sienkiewiczkm 🙏 It seems hard to test...

@szborows
Copy link

szborows commented May 5, 2020

hi, any updates? @k4nar @jmaupetit

I only searched for this in issues and found nothing. I've created #373 on my own, which I have closed when I discovered this PR.

I would also like to see such option in Watson.

@jmaupetit
Copy link
Contributor

LGTM. @k4nar WDYT?

watson/cli.py Outdated Show resolved Hide resolved
k4nar
k4nar previously approved these changes Jun 5, 2020
@sienkiewiczkm
Copy link
Contributor Author

Whoops, I clicked "Update branch" by mistake (so merged Watson master to this commit). What's your usual workflow, should I rebase the change and wait for merge?

@jmaupetit
Copy link
Contributor

This would be perfect to rebase your branch, and I'll merge it right after that! 🙏

@sienkiewiczkm
Copy link
Contributor Author

This would be perfect to rebase your branch, and I'll merge it right after that!

My rebase+squash dismissed the review. I'm a bit new to GitHub pull requests, sorry about that!

@jmaupetit
Copy link
Contributor

Arf, one more thing: can you fill the CHANGELOG please? 🙏

In most cases click library figures out correctly if colored output should be
used, but sometimes it's wrong. For example when `watch` command
is used click is unable to determine this and chooses uncolored output:
  `watch --color 'watson aggregate'`

This change allows user to explicitly enable/disable coloring with --color and
--no-color options:
  `watch --color 'watson --color aggregate'`
@sienkiewiczkm
Copy link
Contributor Author

Arf, one more thing: can you fill the CHANGELOG please? 🙏

Done, I'm sorry that I'm very unresponsive lately. I hope that's not too inconvenient 😉

@jmaupetit jmaupetit merged commit 60c6f0a into jazzband:master Jun 24, 2020
@jmaupetit
Copy link
Contributor

Don't be sorry to have a life @sienkiewiczkm 🙏

jmaupetit added a commit that referenced this pull request Jul 3, 2020
Added:

- Log output order can now be controlled via the `--reverse/--no-reverse` flag
  and the `reverse_log` configuration option (#369)
- Add `--at` flag to the `start` and `restart` commands (#364).
- Add `--color` and `--no-color` flags to force output to be colored or not
  respectively (#350).

Changed:

- Require latest Arrow version 0.15.6 to support ISO week dates (#380)

Fixed:

- Make after-edit-check ensure that edited time stamps are not in the future
  (#381)
@jmaupetit jmaupetit mentioned this pull request Jul 3, 2020
jmaupetit added a commit that referenced this pull request Jul 3, 2020
Added:

- Log output order can now be controlled via the `--reverse/--no-reverse` flag
  and the `reverse_log` configuration option (#369)
- Add `--at` flag to the `start` and `restart` commands (#364).
- Add `--color` and `--no-color` flags to force output to be colored or not
  respectively (#350).

Changed:

- Require latest Arrow version 0.15.6 to support ISO week dates (#380)

Fixed:

- Make after-edit-check ensure that edited time stamps are not in the future
  (#381)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants