Update from upstream #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See #185 for justification.
These test parse_spec() against strings that contain only a legit log level name. Several case variants are exercised, and confirmed to obtain the same result.
Add tests that elaborate on the existing parse_default() unit test. These new tests exercise all of log::Level variants (plus the "OFF" pseudo log level), and also serve as minimal examples of how to set the default log level. For example, they exercise both lowercase and uppercase variants. There is one mixed case variant test thrown in, too, since that is specifically allowed for by the external log::Level::from_str() implementation on which we depend. There is also a test to ensure all log::Level variants are accounted for in the above tests. That one is intended to avoid the false sense of comfort from passing tests in the unlikely event that a new variant is added to the externally defined log::Level enum. (Note that this compile-time check does not include the "OFF" pseudo log level, as it exists outside of the log::Level enum.)
The fact that user-specified logging level names are handled in a case-insensitive fashion is now noted in both the 'README.md' file and in the crate-level API docs.
The first mention of the 'RUST_LOG' environment variable in the prose in both the 'README.md' file and in the crate-level API docs now uses bold text markdown. This is intended to draw attention to it for both first time readers and for those skimming the docs quickly.
For symmetry with the 'README.md' file, the crate-level API docs now note that 'env_logger' can be configured by means other than via environment variables, and (like README.md) directs the user to the examples in the GitHub repo. This is useful information for somebody first discovering the 'env_logger' crate on crates.io; it provides a more comprehensive picture of how the crate is intended to be used.
The "Enabling logging" section of the crate-level docs are here
updated to clarify the default behavior as it pertains to the
logging level.
Previously, the first paragraph stated that all logging is disabled
except for the default level, but the third paragraph stated that if
no log level was specified then /all/ logging was enabled.
It turns out that the third paragraph was really intended as a
continuation of the second paragraph, the applicable context of
which is limited to those scenarios in which a logging directive has
been supplied.
This changeset reworks the paragraph structure slightly to make the
intent more clear, and also to make it easy to quickly identify the
most important aspects:
A. Old para 1 split into two separate paras:
- first states clearly the default behavior (now in bold)
- second introduces 'RUST_LOG' and logging directives
B. Old para 3 split into two separate paras:
- first now qualifies the statement with "When specifying
the crate name or a module path..."
- second introduces log level names
C. The existing statement that describes the behavior when only
a log level is provided (it sets the global log level) is
now in bold markdown.
It might make sense to refine this further at some point so that the
flow of the text answers in order the user's questions:
1. "What is the default behavior?" (Already first; good.)
2. "How do I set the log level for the entire app?"
3. "How do I set the log level more surgically?"
Our current flow is: (1), (3), (2)
The 'README.md' and crate-level API docs are both updated to show
the available logging levels in a bulleted list (rather then a
comma-separated list in the prose), and to note that they correspond
to the externally defined log::Level enum in the 'log' crate.
For the README.md file, this is an addition. The universe of valid
log levels was not previously listed there.
The order of the log levels is also now sorted in the list highest
precedence to lowest:
error, warn, info, debug, trace
The previous order in which they were presented was:
debug, error, info, warn, trace
Add markdown to emphasize (typical rendering would be italics) the term "logging directive" upon the first mention in the text.
Since our convention throughout the docs is to use the lower case form of log level names (e.g., "info" rather than "INFO"), users might mistakenly infer that we are implying that only the lower case forms are legit. Careful readers might even suspect that the handful of departures are typos rather than deliberate examples. This change adds a blurblet to the README.md and to the top-level API docs explaining the convention and the motivation for using it (consistency).
These unit tests demonstrate filter::parse_spec() accepting as legit
empty and blank spec strings that it should be ignoring:
$ cargo test 'filter::tests::'
...
failures:
filter::tests::parse_spec_blank_level_isolated
filter::tests::parse_spec_blank_level_isolated_blank_comma
filter::tests::parse_spec_blank_level_isolated_comma_blank
test result: FAILED. 21 passed; 3 failed; 0 ignored; 0 measured; 11 filtered out
Fix to come in follow-up commit.
Fix issue demonstrated by unit tests in commit 2ff2bf1. While parsing comma-separated substrings, parse_spec() now trims the values to ensure empty and blank substrings (which are invalid) are ignored. Previously, only-comma-separated empty substrings were being ignored. This change extends that intent to also ignore the empty string (no comma), stand-alone blank strings (no comma), and comma- separated variations of empty and/or blank substrings.
add unit tests, docs related to available log level names
filter::parse_spec() ignore bogon empty, blank (sub)strings
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.