-
-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chaika plugin adds more tags #749
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this indeed looks like it could be useful to some people! 👍
I have a few suggestions to make the code a bit leaner, if you'd be inclined.
The matching test at tests/LANraragi/Plugin/Metadata/Chaika.t
should also be updated. (See CI error log)
|
||
# Try text search if it fails | ||
if ( $newtags eq "" ) { | ||
$logger->info("No results, falling back to text search."); | ||
( $newtags, $newtitle ) = search_for_archive( $lrr_info->{archive_title}, $lrr_info->{existing_tags} ); | ||
( $newtags, $newtitle ) = search_for_archive( $lrr_info->{archive_title}, $lrr_info->{existing_tags}, $addother, $addsource ); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of carrying the $addsource
variable across all subs, you could just check for it here:
if ($addsource ne "") {
$newtags .= ", source:" . $addsource;
}
(I think the plugin system is lenient enough to allow the comma at the start even if no other tags came in, but might be good to check)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good catch, thanks. I will put it after the no tags check on line 65 so adding strings like that should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it was awkward to test so I decided to carry $addsource across the subs after all.
my $download = $json->{"download"} ? $json->{"download"} : $json->{"archives"}->[0]->{"link"}; | ||
my $gallery = $json->{"gallery"} ? $json->{"gallery"} : $json->{"id"}; | ||
my $timestamp = $json->{"posted"}; | ||
if ( $tags ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
People might not want those additional tags, so it'd be good to gate them behind a preference.
(Maybe this can just use $addother
as well? I feel like if you want those extra tags you also probably want the E-H style behavior of namespacing everything)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I will add another parameter $addextra in order to make it flexible for users, even at the cost of having to carry it all the way to parse_chaika_json.😅
The |
Looks good, thank you very much once again! Here are two Holobytes. |
The json from Chaika contains a few more useful info that I wanted to add as tags, namely download URL, gallery ID, category and timestamp. This commit add those, as well as 2 new options. First option allows adding tags without namespace to the 'other:' namespace instead, mimicking the behaviour of E-Hentai. Second option allows tagging galleries found on Chaika with a custom 'source:' tag.
This doesn't fix any existing issue and is mainly for my own use, but I thought others might find it useful as well.