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

fix to make the string being attached to coloredLogStream colored again #192

Merged
merged 2 commits into from
May 20, 2022

Conversation

sahara-ooga
Copy link
Collaborator

@sahara-ooga sahara-ooga commented May 19, 2022

Fixed a bug caused in PR #165 and #173 where strings for output-colored.log were no longer colored.

this fixes #184.

@sahara-ooga sahara-ooga self-assigned this May 19, 2022
@sahara-ooga sahara-ooga added the bug Something isn't working label May 19, 2022
@sahara-ooga sahara-ooga marked this pull request as ready for review May 19, 2022 16:41
Comment on lines 309 to 313
let coloredOption: Option = {
var op = option
op.colored = true
return op
}()
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't a code like the following suffice?

Suggested change
let coloredOption: Option = {
var op = option
op.colored = true
return op
}()
let coloredOption = option
coloredOption.colored = true

Copy link
Collaborator Author

@sahara-ooga sahara-ooga May 20, 2022

Choose a reason for hiding this comment

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

Swift compiler says for the suggested above:

cannot assign to property: 'coloredOption' is a 'let' constant

var is ok:

var coloredOption = option
coloredOption.colored = true

Copy link
Owner

Choose a reason for hiding this comment

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

Oops, I wrong.
Use var instead of let is corrected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@YusukeHosonuma
I've refactored in 7582ec5.

@sahara-ooga sahara-ooga added this to the 1.4.0 - TBD milestone May 20, 2022
Copy link
Owner

@YusukeHosonuma YusukeHosonuma left a comment

Choose a reason for hiding this comment

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

👍

@sahara-ooga
Copy link
Collaborator Author

thx!

@sahara-ooga sahara-ooga merged commit 1ad0d60 into main May 20, 2022
@sahara-ooga sahara-ooga deleted the fix/colored-log branch May 20, 2022 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output-colored.log is no longer coloring.
2 participants