Combine split tool warning logs into a single line - #845
Merged
Conversation
The three media-info readers logged "Warning getting media info" and the raw tool stderr as two separate lines. Fold them into one line with the stderr passed through CleanForLog (joined with " | ", control characters dropped) and the file name last, matching how tool errors are logged elsewhere. Likewise merge the two-line Linux folder-tools advisory into one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refines log formatting in PlexCleaner by consolidating split stderr warning logs from media-info readers into a single structured Serilog line (with stderr normalized via CleanForLog), and by combining the Linux folder-tools advisory into one log message.
Changes:
- Combine
Warning getting media info+ raw stderr warning logs into a singleLog.Warningline inFfProbeTool,MediaInfoTool, andMkvMergeTool. - Normalize tool stderr in those warnings using
CleanForLogto keep multi-line output to a single structured value. - Merge the two Linux folder-tools advisory warnings in
Tools.VerifyTools()into one warning message.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| PlexCleaner/Tools.cs | Combines two Linux folder-tools advisory warnings into a single warning line. |
| PlexCleaner/MkvMergeTool.cs | Consolidates media-info stderr warning logging into a single structured warning line. |
| PlexCleaner/MediaInfoTool.cs | Consolidates media-info stderr warning logging into a single structured warning line. |
| PlexCleaner/FfProbeTool.cs | Consolidates media-info stderr warning logging into a single structured warning line. |
Capture and log tool error output for troubleshooting on a single line with the exit code, operation, and file name: - LogFailedResult takes the file name and logs the tool's error output, read from the stream the tool uses (stderr for the ffmpeg family and HandBrake, stdout for the mkvtoolnix tools and MediaInfo) with a fallback to the other captured stream so it is never lost. All tools now run buffered so output is always captured, fixing mkvpropedit and 7-Zip failures that logged no error text; the unused non-buffered Execute overload is removed. - Execute, its cancellation path, and LogFailedResult record the calling method as the operation via CallerMemberName, so a command ties to its purpose in a parallel log without correlating separate lines. - Removed per-method debug lines that only restated the command already logged by Execute, and the dead mkvmerge stderr warning branch. Move the Serilog import to a global using, and document the tool error streams and the failure-handling philosophy in AGENTS.md and the logging, DTS escalation, and idet parse changes in HISTORY.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback:
- The media-info warning path logged an empty "{Warning}" segment when
stderr held only whitespace; clean the output first and log only when
it is non-empty.
- AGENTS.md and HISTORY.md overstated the logging: output is consumed
either buffered (Execute) or streamed (ExecuteStreamStdErr for the
-f null verify), not "always buffered"; the mkvtoolnix tools override
GetErrorOutput to stdout while MediaInfo's stdout errors are caught by
the LogFailedResult fallback; 7-Zip errors are on stderr.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback: - GetClosedCaptions and GetStreamTimings logged a "Failed to get ..." line in addition to LogFailedResult, which now carries the operation; remove the extra line so a failure is one structured line. - Scope the HISTORY note to the buffered mkvpropedit and 7-Zip executions rather than claiming all tool output is captured. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Read both captured streams once and pick the tool's declared stream (GetErrorOutput), else the other non-empty stream, so the fallback no longer re-reads the primary stream and matches the documented intent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
IProcessPlugin.Logger is part of the public plugin contract, and a bare ILogger is ambiguous with Microsoft.Extensions.Logging.ILogger. Restore the fully qualified Serilog.ILogger and suppress the simplify-name analyzer with an attribute (not a pragma) so the public interface is unambiguous to plugin authors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ptr727
force-pushed
the
fix/combine-tool-warning-logs
branch
from
July 15, 2026 18:38
913c956 to
88d6143
Compare
- Use a semicolon in the Linux folder-tools advisory instead of a comma splice. - MergeToMkv logged the output temp path as the file name; log the primary source instead, consistent with the other mkvmerge operations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Debug level definition listed "Getting media info" as an example, contradicting the new guidance not to add a per-method debug line that restates the command; drop it and show the operation in the command example instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
The three media-info readers (FfProbe, MkvMerge, MediaInfo) logged
Warning getting media infoand the raw tool stderr as two separate log lines. Fold each into a single line with the stderr passed throughCleanForLog(joined with|, control characters dropped) and the file name last, matching how tool errors are already logged elsewhere in the codebase. Also merge the two-line Linux folder-tools advisory inTools.csinto one.No behaviour change, log formatting only.
🤖 Generated with Claude Code