feat(custom): add separate Outline field to Custom Movie Scraper#1976
feat(custom): add separate Outline field to Custom Movie Scraper#1976Arny80Hexa wants to merge 2 commits intoKomet:masterfrom
Conversation
bugwelle
left a comment
bugwelle
left a comment
There was a problem hiding this comment.
Cool! Definitely a good addition. I have a few remarks regarding the movie merger. Would be great if you could incorporate that. :)
| << MovieScraperInfo::TvShowLinks | ||
| << MovieScraperInfo::Outline; |
There was a problem hiding this comment.
| << MovieScraperInfo::TvShowLinks | |
| << MovieScraperInfo::Outline; | |
| << MovieScraperInfo::TvShowLinks // | |
| << MovieScraperInfo::Outline; |
FYI: Please run ./scripts/run_clang_format.sh and/or ./scripts/quick_checks.sh once. The trailing comment is a "hack" to make clang-format auto-format these lines.
| if (!source.outline().isEmpty()) { | ||
| target.setOutline(source.outline()); | ||
| } else if (usePlotForOutline) { | ||
| } else if (!source.overview().isEmpty()) { |
There was a problem hiding this comment.
Why remove the usePlotForOutline check?
| } else if (!source.overview().isEmpty()) { | |
| } else if (target.outline().isEmpty() && usePlotForOutline && !source.overview().isEmpty()) { |
| { | ||
| Movie copy; | ||
| original->setOutline(""); | ||
| copyDetailsToMovie(copy, *original, allMovieScraperInfos(), true, true); |
There was a problem hiding this comment.
Could you copy this test case and test:
- no outline requested
- "usePlotForOutline" being true
=> ensure that outline isn't set
| if (details.contains(MovieScraperInfo::Outline)) { | ||
| copyDetailToMovie(target, source, MovieScraperInfo::Outline, usePlotForOutline, ignoreDuplicateOriginalTitle); | ||
| } else if (usePlotForOutline && target.outline().isEmpty() && !target.overview().isEmpty()) { | ||
| target.setOutline(target.overview()); | ||
| } |
There was a problem hiding this comment.
I'd prefer this logic to be inside copyDetailToMovie instead of here. usePlotForOutline is rather a usePlotForOutlineIfOutlineIsEmpty :D
Allow users to assign a different scraper for the movie outline (short summary) than for the plot. The Kodi NFO writer already outputs both as separate <outline> and <plot> tags — this change exposes that capability in the Custom Movie Scraper settings. When no dedicated Outline scraper is configured, the existing "use plot for outline" fallback setting continues to work as before. Fixes Komet#1975 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Movie.cpp: add trailing // comment for clang-format alignment - MovieMerger: restore usePlotForOutline check in Outline case, add target.outline().isEmpty() guard, move all logic into copyDetailToMovie (remove special handling from caller) - Tests: add test case for outline-not-requested with usePlotForOutline=true Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Arny80Hexa
force-pushed
the
feat/custom-scraper-outline
branch
from
d613543 to
bd6917a
Compare
|
Addressed all review comments:
Also rebased on current master. Ran Side note: we noticed that |
2 participants
Allow users to assign a different scraper for the movie outline (short summary) than for the plot. The Kodi NFO writer already outputs both as separate
<outline>and<plot>tags — this change exposes that capability in the Custom Movie Scraper settings.When no dedicated Outline scraper is configured, the existing "use plot for outline" fallback setting continues to work as before.
Fixes #1975
Developed with AI assistance (Claude Code / Opus 4.6).