Skip to content

setup.sh color variables added#1879

Merged
casperklein merged 2 commits intodocker-mailserver:masterfrom
casperklein:setup.sh
Apr 1, 2021
Merged

setup.sh color variables added#1879
casperklein merged 2 commits intodocker-mailserver:masterfrom
casperklein:setup.sh

Conversation

@casperklein
Copy link
Copy Markdown
Member

Description

To make the source more readable, color variables have been added.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change that does improve existing functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or ENVIRONMENT.md or the documentation)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@casperklein
Copy link
Copy Markdown
Member Author

@aendeavor There are two escape sequences in use. See $RESET and $RESET2. I think it's safe to only use $RESET2. Can you confirm that? In my test, I did not notice any difference.

@wernerfred
Copy link
Copy Markdown
Member

wernerfred commented Apr 1, 2021

Just Reading this on my mobile but didnt we Had Something similar from @aendeavor which was reverted then as colorized Output Breaks with current bats?
Ist this resolved now? Didnt have the PR # in my mind but there was a discussion about that already.
I will See If i can find it again later

@georglauterbach
Copy link
Copy Markdown
Member

@aendeavor There are two escape sequences in use. See $RESET and $RESET2. I think it's safe to only use $RESET2. Can you confirm that? In my test, I did not notice any difference.

Yes, it's safe to use \e[0m only. This sequence removes all applied attributes, which is desired.

\e[39m sets the Color to the default value again - this is why there is no difference here, as we're just working with colours.

@georglauterbach
Copy link
Copy Markdown
Member

Just Reading this on my mobile but didnt we Had Something similar from @aendeavor which was reverted then as colorized Output Breaks with current bats?

Not quite the same. In our case, the problem was supervisord not handling the escape sequences properly. As a result, the bats were not able to handle the output.

Ist this resolved now? Didnt have the PR # in my Kind nur there was a discussion about that already.

Supervisor can still not handle color... which makes me angry.

@casperklein casperklein marked this pull request as ready for review April 1, 2021 11:50
@casperklein casperklein requested review from a team and removed request for georglauterbach April 1, 2021 11:50
@georglauterbach
Copy link
Copy Markdown
Member

Very nice. From my PoV, this can be merged:)

@casperklein casperklein merged commit f3652e5 into docker-mailserver:master Apr 1, 2021
@casperklein casperklein deleted the setup.sh branch April 1, 2021 13:36
@wernerfred wernerfred added this to the v10.0.0 milestone May 18, 2021
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.

3 participants