[TASK] Fix Kagane#3562
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the Kagane manga site implementation by updating HTML element selectors and enhancing the replaceLinebreaks utility function. The changes address issues with scraping data from the Kagane website, likely due to changes in the site's structure.
Changes:
- Updated Kagane implementation to use different HTML selectors (
titleelement instead of OpenGraph meta tags,meta[name="description"]instead ofog:description) - Enhanced
replaceLinebreaksfunction to also collapse multiple spaces and trim results - Added service worker bypass in headless tests
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/headless/test.js | Adds service worker bypass to prevent interference during headless testing |
| src/pages-chibi/implementations/Kagane/main.ts | Updates selectors to match current Kagane website structure and switches from reading meta tags to constructing API URLs for images |
| src/chibiScript/utilities/stringUtilities.ts | Enhances the replaceLinebreaks utility to fix regex usage and add space collapsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| replaceLinebreaks: ($c: ChibiGenerator<string>, replacement: ChibiParam<string> = ' ') => { | ||
| return $c.replaceAll('(\n|\r)', replacement); | ||
| return $c.replaceRegex('(\n|\r)', replacement).replaceRegex(' +', ' ').trim(); |
There was a problem hiding this comment.
The regex pattern (\n|\r) does not match Windows-style line breaks \r\n correctly. It will replace both \r and \n separately, resulting in two replacements for a single Windows line break. The pattern should be (\r\n|\n|\r) or [\r\n]+ to properly handle all line break types.
| return $c.replaceRegex('(\n|\r)', replacement).replaceRegex(' +', ' ').trim(); | |
| return $c.replaceRegex('[\\r\\n]+', replacement).replaceRegex(' +', ' ').trim(); |
| */ | ||
| replaceLinebreaks: ($c: ChibiGenerator<string>, replacement: ChibiParam<string> = ' ') => { | ||
| return $c.replaceAll('(\n|\r)', replacement); | ||
| return $c.replaceRegex('(\n|\r)', replacement).replaceRegex(' +', ' ').trim(); |
There was a problem hiding this comment.
The change from replaceAll to replaceRegex alters the behavior of the replaceLinebreaks function. The new implementation now also collapses multiple spaces into single spaces and trims the result, which was not part of the original behavior. This could break existing code that depends on the original behavior, particularly if spacing is significant in the context where this function is used. Consider whether this behavioral change is intentional and documented.
| return $c.replaceRegex('(\n|\r)', replacement).replaceRegex(' +', ' ').trim(); | |
| return $c.replaceAll('\n', replacement).replaceAll('\r', replacement); |
| replaceLinebreaks: ($c: ChibiGenerator<string>, replacement: ChibiParam<string> = ' ') => { | ||
| return $c.replaceAll('(\n|\r)', replacement); | ||
| return $c.replaceRegex('(\n|\r)', replacement).replaceRegex(' +', ' ').trim(); | ||
| }, |
There was a problem hiding this comment.
The modified replaceLinebreaks function lacks test coverage. Since this function's behavior has been changed to include additional operations (collapsing multiple spaces and trimming), tests should be added to verify the new behavior works correctly for various inputs including different line break types, multiple consecutive spaces, and strings with leading/trailing whitespace.
close #3559