Use log level to manage logging verbosity #261#313
Use log level to manage logging verbosity #261#313thompson-tomo wants to merge 8 commits intothlorenz:masterfrom
Conversation
111e24d to
b4c9291
Compare
|
Sorry, looks like some conflicts need to be resolved now. |
|
No worries @AndrewSouthpaw i have resolved the conflicts. |
There was a problem hiding this comment.
Pull request overview
Adds configurable logging verbosity to doctoc so users can reduce noisy output (e.g., only show “Found nothing …” in more verbose modes), addressing #261.
Changes:
- Introduces
--loglevel/-lsupport via thelogleveldependency and routes most CLI output through log levels. - Adjusts stdout-related tests/fixtures to account for loglevel-controlled output.
- Updates docs to describe log level configuration.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
doctoc.js |
Adds loglevel, wires --loglevel, and shifts many console.* calls to level-based logging. |
lib/file.js |
Moves directory scan messages to debug/trace to reduce default noise. |
package.json |
Adds loglevel dependency. |
package-lock.json |
Locks loglevel dependency. |
test/transform-stdout.js |
Updates/extends stdout tests to cover default vs trace logging. |
test/fixtures/stdout_trace.log |
New expected stdout output for trace logging. |
test/fixtures/stdout_info.log |
New expected stdout output for default info logging. |
test/transform.js |
Switches helper output to use logger (but currently missing logger import). |
README.md |
Documents --loglevel usage and supported levels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
doctoc.js:160
- There are still several
console.error(...)calls in argument validation / fatal error paths (e.g., TOC title padding and--stdoutmisuse), while other errors were migrated tolog.error. This makes logging behavior inconsistent and bypasses the new--loglevelplumbing. Consider switching these remainingconsole.errorcalls tolog.error(or centralizing throughprintUsageAndExit(true)where appropriate) for consistent output behavior.
var padBeforeTitle = argv.toctitlepaddingbefore;
if (padBeforeTitle && isNaN(padBeforeTitle) || padBeforeTitle < 0) { console.error('Padding before title specified is not a positive number: ' + padBeforeTitle), printUsageAndExit(true); }
else if (padBeforeTitle && padBeforeTitle > 1) { console.error('Padding before title: ' + padBeforeTitle + ' is not currently supported as greater than 1'), printUsageAndExit(true); }
else if (padBeforeTitle || notitle) { padTitle = true; }
var maxHeaderLevel = argv.m || argv.maxlevel;
var logLevel = argv.l || argv.loglevel || "info";
log.setLevel(logLevel, false);
if (maxHeaderLevel && isNaN(maxHeaderLevel)) { log.error('Max. heading level specified is not a number: ' + maxHeaderLevel), printUsageAndExit(true); }
var minHeaderLevel = argv.minlevel || 1;
if (minHeaderLevel && isNaN(minHeaderLevel) || minHeaderLevel < 0) { log.error('Min. heading level specified is not a positive number: ' + minHeaderLevel), printUsageAndExit(true); }
else if (minHeaderLevel && minHeaderLevel > 2) { log.error('Min. heading level: ' + minHeaderLevel + ' is not currently supported as greater than 2'), printUsageAndExit(true); }
if (maxHeaderLevel && maxHeaderLevel < minHeaderLevel) { log.error('Max. heading level: ' + maxHeaderLevel + ' is less than the defined Min. heading level: ' + minHeaderLevel), printUsageAndExit(true); }
if (argv._.length > 1 && stdOut) {
console.error('--stdout cannot be used to process multiple files/directories. Use --dryrun instead.');
process.exitCode = 2;
return;
}
for (var i = 0; i < argv._.length; i++) {
var target = cleanPath(argv._[i]),
stat = fs.statSync(target);
if (stat.isDirectory() && stdOut) {
console.error('--stdout cannot be used to process a directory. Use --dryrun instead.');
process.exitCode = 2;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AndrewSouthpaw
left a comment
There was a problem hiding this comment.
A couple small things to address, but otherwise lgtm
| catch (e) { | ||
| log.error('Unknown log level: ' + logLevel); | ||
| log.error('Supported options: trace, debug, info, warn, error'); | ||
| log.setLevel("info", false); |
There was a problem hiding this comment.
We should probably throw from here, rather than quietly continue operation. I think invalid flags usually trigger an abort from the command. WDYT?
There was a problem hiding this comment.
I have no strong opinion, the reason why I went with a default is that the setting has 0 impact on the output hence if we continue the produced output wouldn't be any different.
| if (maxHeaderLevel && isNaN(maxHeaderLevel)) { log.error('Max. heading level specified is not a number: ' + maxHeaderLevel), printUsageAndExit(true); } | ||
|
|
||
| if (maxHeaderLevel && maxHeaderLevel < minHeaderLevel) { console.error('Max. heading level: ' + maxHeaderLevel + ' is less than the defined Min. heading level: ' + minHeaderLevel), printUsageAndExit(true); } | ||
| var minHeaderLevel = argv.minlevel || 1; | ||
| if (minHeaderLevel && isNaN(minHeaderLevel) || minHeaderLevel < 0) { log.error('Min. heading level specified is not a positive number: ' + minHeaderLevel), printUsageAndExit(true); } | ||
| else if (minHeaderLevel && minHeaderLevel > 2) { log.error('Min. heading level: ' + minHeaderLevel + ' is not currently supported as greater than 2'), printUsageAndExit(true); } | ||
| if (maxHeaderLevel && maxHeaderLevel < minHeaderLevel) { log.error('Max. heading level: ' + maxHeaderLevel + ' is less than the defined Min. heading level: ' + minHeaderLevel), printUsageAndExit(true); } |
There was a problem hiding this comment.
Can you explain the changes here? They seem unrelated to supporting different log levels. (It's fine to roll them into the same PR, seems like a net improvement.)
There was a problem hiding this comment.
There isn't any change other than the logger used. diff is bigger than expected due to it shifting lines down because of the log level validation.
Closes #261
This makes the logging level configurable to reduce verbosity at low levels