feat: add log message color configuration

[Wafffle77]

Pull Request Template

Thank you for your contribution! Please ensure the following before submitting:

Checklist

• My code follows the style guidelines of this project
• I have added/updated comments where needed
• I have added tests that prove my fix is effective or my feature works
• I have run make test (or equivalent) locally and all tests pass
• DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
• REUSE Compliance:
  • Each new/modified source file has SPDX copyright and license headers
  • Any non-commentable files include a <filename>.license sidecar
  • All referenced licenses are present in the LICENSES/ directory

Description

Adds a --log-color option for controlling colored log output. In addition to "on" and "off", "auto" (the default) will detect whether or not stderr is a terminal and act accordingly.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

---

For more info, see Contributing Guidelines.

[synackd]

I tested this and it works well.

However, there is a missing piece that is required for config merging to work correctly. For instance, if I:

# Set value in user config
ochami config set log.color on

I can see it get changed in the user config:

$ ochami config show log.color --user
on

But the merged config uses the default value:

$ ./ochami config show log.color
auto

To fix this, we'll have to add a case for this config value in the custom config unmarshaller. There is already code there for timeout (case statement, default value setting) that can serve as a reference.

[synackd]

I adjusted the title of this PR to follow Conventional Commit format for the future squash merge.

[Wafffle77]

To fix this, we'll have to add a case for this config value in the custom config unmarshaller. There is already code there for timeout (case statement, default value setting) that can serve as a reference.

Is this desired behavior, though? The other two logging options don't have any default handling in that style, instead the program exits with an error if they aren't specified, and the defaulting appears to be handled by virtue of the options being specified in the default config. I've implemented a quick fix that solves the issue (outside of the config unmarshaling code, since the ConfigLog struct doesn't already have an UnmarshalYAML method).

[synackd]

Could we make this a switch-case?

	switch lc {
	case "", "auto":
		cw.NoColor = !term.IsTerminal(int(os.Stderr.Fd()))
	case "on":
		cw.NoColor = false
	case "off":
		cw.NoColor = true
	default:
		return fmt.Errorf("invalid log-color: %s", lc)
	}

[Wafffle77]

Fixed in 74d7856

[synackd]

To fix this, we'll have to add a case for this config value in the custom config unmarshaller. There is already code there for timeout (case statement, default value setting) that can serve as a reference.

Is this desired behavior, though? The other two logging options don't have any default handling in that style, instead the program exits with an error if they aren't specified, and the defaulting appears to be handled by virtue of the options being specified in the default config. I've implemented a quick fix that solves the issue (outside of the config unmarshaling code, since the ConfigLog struct doesn't already have an UnmarshalYAML method).

The desired behavior is to use the following precedence for configuration options:

1. CLI flag
2. Config from config file passed with --config
3. User config file (~/.config/ochami/config.yaml)
4. System config file (/etc/ochami/config.yaml)
5. Compiled defaults

What I am seeing now is that changing the log.color option in (3) does not precede (4) or (5), and the latter two currently take precedence over the former as implemented.

[synackd]

Also, would you mind rebasing onto main to resolve the latest changes?

[Wafffle77]

To fix this, we'll have to add a case for this config value in the custom config unmarshaller. There is already code there for timeout (case statement, default value setting) that can serve as a reference.

Is this desired behavior, though? The other two logging options don't have any default handling in that style, instead the program exits with an error if they aren't specified, and the defaulting appears to be handled by virtue of the options being specified in the default config. I've implemented a quick fix that solves the issue (outside of the config unmarshaling code, since the ConfigLog struct doesn't already have an UnmarshalYAML method).

The desired behavior is to use the following precedence for configuration options:

1. CLI flag

2. Config from config file passed with `--config`

3. User config file (`~/.config/ochami/config.yaml`)

4. System config file (`/etc/ochami/config.yaml`)

5. Compiled defaults

What I am seeing now is that changing the log.color option in (3) does not precede (4) or (5), and the latter two currently take precedence over the former as implemented.

This looks like a bug in the code that loads all the logging config. I'm able to reproduce the same behavior with the log.format and log.level options as well. A fix might be better relegated to its own PR.