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: lazy init default logger #111

Merged
merged 2 commits into from Mar 4, 2024
Merged

fix: lazy init default logger #111

merged 2 commits into from Mar 4, 2024

Conversation

caarlos0
Copy link
Member

@caarlos0 caarlos0 commented Mar 4, 2024

The default logger will try to query background et al on startup, which might conflict with other libs trying to query it too.

It's easily reproducible with some of our tools that use the log package, e.g., VHS:

faketty vhs --help

It slows down the program quite a bit, and print some ansi escape sequences as well.

This patch changes it so it only inits the default logger when/if the default logger is used.

Per my testing, it seems to fix the original issue.

The default logger will try to query background et al on startup, which
might conflict with other libs trying to query it too.

It's easily reproducible with some of our tools that use the log
package, e.g., VHS:

```sh
faketty vhs --help
```

It slows down the program quite a bit, and print some ansi escape
sequences as well.

This patch changes it so it only inits the default logger when/if the
default logger is used.

Per my testing, it seems to fix the original issue.
@caarlos0 caarlos0 added the bug Something isn't working label Mar 4, 2024
@caarlos0 caarlos0 self-assigned this Mar 4, 2024
Copy link
Member

@maaslalani maaslalani left a comment

Choose a reason for hiding this comment

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

Nice!!! ❤️

@caarlos0
Copy link
Member Author

caarlos0 commented Mar 4, 2024

PS: this is not a faketty-only thing, it's producible if you create a PTY (with creak/pty for example) and run the same command - it's how I initially noticed the issue.

Copy link
Member

@aymanbagabas aymanbagabas left a comment

Choose a reason for hiding this comment

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

Thank you @caarlos0!

@caarlos0 caarlos0 merged commit 2819330 into main Mar 4, 2024
22 checks passed
@caarlos0 caarlos0 deleted the lazy branch March 4, 2024 18:44
caarlos0 added a commit to charmbracelet/freeze that referenced this pull request Mar 4, 2024
refs charmbracelet/log#111

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 added a commit to charmbracelet/mods that referenced this pull request Mar 4, 2024
refs charmbracelet/log#111

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 added a commit to charmbracelet/vhs that referenced this pull request Mar 4, 2024
refs charmbracelet/log#111

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
caarlos0 added a commit to charmbracelet/vhs that referenced this pull request Mar 4, 2024
refs charmbracelet/log#111

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
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.

None yet

3 participants