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

Ignore empty items in DEFMT_LOG #771

Merged
merged 2 commits into from
Aug 11, 2023
Merged

Ignore empty items in DEFMT_LOG #771

merged 2 commits into from
Aug 11, 2023

Conversation

GrantM11235
Copy link
Contributor

There are a several ways that the DEFMT_LOG environment variable could have an empty item, for example:

  • DEFMT_LOG=
  • DEFMT_LOG=info,
  • DEFMT_LOG=info,,app::noisy=error

Previously, an empty item would be incorrectly parsed as a module path, causing the macro to panic.

With this patch, empty items are simply ignored.

Copy link
Member

@Urhengulas Urhengulas left a comment

Choose a reason for hiding this comment

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

I first was a bit unsure if we should maybe throw an explicit error in case a user accidentally adds a spare ,, but I like the "it just works" approach more.

@Urhengulas Urhengulas added this pull request to the merge queue Aug 11, 2023
Merged via the queue into knurling-rs:main with commit ca161bf Aug 11, 2023
15 checks passed
@GrantM11235 GrantM11235 deleted the ignore-empty-defmt-log branch August 11, 2023 21:53
@Urhengulas
Copy link
Member

Thank you for your work @GrantM11235!

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.

2 participants