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

Resolves [#22] Add CLI options to handle colouring #93

Merged
merged 13 commits into from
Jan 12, 2023

Conversation

marcellourbani
Copy link
Contributor

@marcellourbani marcellourbani commented Oct 16, 2022

Resolves #22

  • Add a config option for my application name
  • Add new module Iris.Cli.Colour
    • Enum type for always / auto / never options
  • CLI parsers for above-mentioned colours
  • Add this option by default to env
  • Add a new module Iris.Colour.Detect with the implementation of the colouring detection logic described in the beginning of this issue
  • There should be no separate option for stdout and stderr. In other words, env variables and CLI disable / enable

I had to change expectations of some tests in cliSpecParserConflicts to account for the new options, I wonde if they can be made more robust

Also, I guess I should also have added tests for conflicts of --no-colour and friends. Didn't feel like doing that today, and I'll be on holiday next week, so I thought was good enough for now

Additional tasks

  • Documentation for changes provided/changed
  • Tests added
  • Updated CHANGELOG.md

@chshersh chshersh added the hacktoberfest-accepted https://hacktoberfest.com/participation/ label Oct 16, 2022
@chshersh chshersh added the cli-options CLI options, parsers label Oct 19, 2022
@chshersh
Copy link
Owner

Hey @marcellourbani 👋🏻

Sorry for the delay in review. This PR is a lot of work and will take a while for me to review 😅

@marcellourbani
Copy link
Contributor Author

marcellourbani commented Oct 19, 2022 via email

Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Hey @marcellourbani 👋🏻

I finally have the capacity and motivation to do some work on Iris so I was able to review your PR 🙂 Huge work, thanks a lot for that! 👏🏻

I left some comments with potential improvements to this issue. Let me know if you're still interested in continuing working on this issue 🙂

Otherwise, I can take it over from here and finish, no problemo 👌🏻


Also, CI stopped working a while ago, so you need to rebase on top of the latest main branch to make it work again in this PR.

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
iris.cabal Outdated Show resolved Hide resolved
src/Iris/Cli/Colour.hs Outdated Show resolved Hide resolved
src/Iris/Cli/Colour.hs Outdated Show resolved Hide resolved
src/Iris/Cli/Colour.hs Outdated Show resolved Hide resolved
src/Iris/Colour/Detect.hs Outdated Show resolved Hide resolved
src/Iris/Colour/Detect.hs Outdated Show resolved Hide resolved
src/Iris/Colour/Detect.hs Outdated Show resolved Hide resolved
@marcellourbani
Copy link
Contributor Author

Thanks @chshersh .I'm quite interested and will look into all of these, on my own time. Probably later this week
Cheers
Marcello

@marcellourbani
Copy link
Contributor Author

Sorted all except the ColourOption constructors

Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot!

I'm currently focusing on merging all existing PRs and I'll prepare a new Iris release after that 🙂

@chshersh chshersh merged commit 574c474 into chshersh:main Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli-options CLI options, parsers hacktoberfest-accepted https://hacktoberfest.com/participation/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CLI options to handle colouring
2 participants