fix(manual-import): match the rename naming table on casing and missing tokens - #869
Open
m4bard wants to merge 1 commit into
Open
fix(manual-import): match the rename naming table on casing and missing tokens#869m4bard wants to merge 1 commit into
m4bard wants to merge 1 commit into
Conversation
…ng tokens
ManualImportPathPlanner.BuildNamingVariables is a third hand-rolled copy of a table
that RenameService.BuildNamingVariables and FileNamingService also implement, and it
diverges from them in two ways that lose a pattern segment.
It keyed a bare Dictionary<string, object> where RenameService keys one with
StringComparer.OrdinalIgnoreCase. The token regex in FileNamingService is already
case-insensitive, so a pattern written {series} or {ASIN} resolved through rename
and library add and silently produced nothing through manual import.
SeriesNumber and Quality were absent from the table entirely while both are present
in the other two, so a pattern using either rendered under rename and lost the
segment here. Both are now inserted on the same terms as their neighbours: present
when known, absent when not, so the existing missing-variable cleanup still applies.
A third divergence is deliberately NOT changed. This table inserts keys only when
non-empty, while RenameService always inserts them. That difference is real, but the
behaviour here looks like the better of the two: an absent key yields a sentinel that
FileNamingService cleans up, stripping brackets and adjacent separators, which an
empty string does not get. Inserting empties would turn "{Series} - {Title}" into
" - Title" where it currently renders "Title". A test asserting the change was
written, did not fail when the change was reverted, and was removed rather than kept
as decoration. The remaining cost is a warning logged for the ordinary case of a book
with no series, which is noise rather than a wrong path, and the issue says so.
The tests cover the two changes that can be demonstrated: three casings of the same
pattern produce the same result, and a {SeriesNumber} pattern renders the number.
Both fail with the respective divergence restored.
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
1 participant
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.
Summary
ManualImportPathPlanner.BuildNamingVariablesis a third hand-rolled copy of the naming-variable table thatRenameServiceandFileNamingServicealso implement, and it sets twelve keys where rename sets fourteen. Two tokens are missing and the dictionary is case-sensitive, so patterns that resolve under rename silently lose a segment under manual import.Fixes #816, which reported the
{SeriesNumber}half.Changes
Fixed
SeriesNumberandQualityare added to the table. Both are present inRenameService.Helpers.csandFileNamingService.Helpers.csand were absent here.StringComparer.OrdinalIgnoreCase, matchingRenameService.Helpers.cs:193. The token regex atFileNamingService.cs:206already carriesRegexOptions.IgnoreCase, so a pattern written{series}matched the regex, missed the lookup, and took the missing-variable path. That made the divergence general rather than limited to two tokens: any token in any casing other than this table's exact spelling was dropped on import and kept on rename.Both new keys are inserted on the same terms as their neighbours, present when known and absent when not, so the existing missing-variable cleanup still applies to them.
What this does not change, having tried it
The third divergence is that this table inserts keys only when non-empty while
RenameServicealways inserts them. I changed that too, wrote a test for it, and the test passed with the change reverted, which is why it is not in this PR.Inserting empties is worse. An absent key yields the empty sentinel and
FileNamingService.cs:257-267strips brackets around it and separators beside it, so{Series} - {Title}rendersTitlefor a book with no series. An empty string gets none of that and renders- Title. #816 already describes that cleanup as the right behaviour, so manual import has the better of the two and rename is arguably the one out of step.What it does cost is a warning per affected file:
FileNamingService.cs:252logs at warning level for the ordinary case of a book with no series. That is noise rather than a wrong path, so it is described in the issue rather than changed here.Testing
ManualImportNamingVariableParityTestsasserts that three casings of the same pattern produce the same destination, and that a{SeriesNumber}folder pattern renders the number rather than dropping the segment.Verified as real guards: restoring the bare dictionary fails two of the three casing cases, and removing the two keys fails the
{SeriesNumber}case.Full suite: 3,033 passed, 0 failed, 125 skipped, against a 3,029 baseline on
03958c15.Note
The wider change is to stop having three copies of this table. That crosses an assembly boundary, since
ManualImportPathPlanneris inlistenarr.apiandRenameServiceinlistenarr.application, and it is a bigger decision than this. This makes the outlier agree with its neighbours on the two points where the disagreement loses data.