v2.6.19: 新增 Feature 机制，支持下载时通过 extra 参数附加导出 PDF/ZIP/长图等行为; Feature 参数根据 download_album/download_photo 来源自适应; 新增 Feature 教程文档#533
Conversation
…根据 download_album/download_photo 来源自适应; 新增 Feature 教程文档
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Feature system for post-download export behaviors, integrates feature registration/invocation into downloader hooks, forwards an ChangesFeature System + Integration
Sequence Diagram(s)sequenceDiagram
participant User as User
participant API as download_album/photo
participant Downloader as JmDownloader
participant Hook as after_album/after_photo
participant Feature as Feature/PluginFeature
participant Plugin as ExportPlugin
User->>API: download_album(..., extra=Feature.export_pdf)
API->>Downloader: add_features(extra, "download_album")
Downloader->>Downloader: store feature in _feature_list
API->>Downloader: start download
Downloader->>Hook: after_album(album)
Hook->>Downloader: _invoke_features_for("after_album", album=..., downloader=...)
Downloader->>Feature: should_invoke(feature_from="download_album", when="after_album")?
Feature-->>Downloader: True
Downloader->>Feature: invoke(option, feature_from="download_album", when="after_album", album=..., downloader=...)
Feature->>Plugin: option.invoke_plugin(... adapted kwargs ...)
Plugin-->>Feature: export complete
Feature-->>Downloader: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
tests/test_jmcomic/test_jm_feature.py (1)
89-112: ⚡ Quick winAdd a batch-ID coverage case for
extraexecution.Current tests validate scalar IDs, but not iterable batch input. A small batch test would guard against
extrabeing skipped indownload_batchdelegation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_jmcomic/test_jm_feature.py` around lines 89 - 112, Add a test that exercises the `extra` Feature when using batch input: create a `MyCounterFeature` (subclassing `Feature`) that increments a counter in `invoke`, then call `jmcomic.download_batch([...], self.option, extra=counter_feature)` with an iterable of IDs and assert the counter increased for each expected `after_photo`/`after_album` invocation; this mirrors the existing `test_download_use_feature` flow (which calls `download_album` and `download_photo`) but targets `download_batch` to ensure `extra` is not skipped when handling iterable inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/docs/sources/tutorial/13_export_and_feature.md`:
- Around line 88-90: The link fragment for the inline tip doesn't match the
generated heading anchor for the target heading "### 2.5 智能适配规则"; update the
fragment in the tip (the [智能适配规则](#...) link) to match the heading's generated
anchor (include the "2.5" portion, e.g. change to "#2-5-智能适配规则" or the exact
generated anchor your docs tool emits) so the internal link points to the "###
2.5 智能适配规则" heading.
In `@README.md`:
- Line 33: Update the README entry for the tutorial so the link text reflects
the full coverage ("PDF/ZIP/长图"); specifically change the markdown link label
"[教程：下载后转为 PDF / ZIP]" to "[教程：下载后转为 PDF / ZIP / 长图]" (the target URL
./assets/docs/sources/tutorial/13_export_and_feature.md remains the same) so the
README matches the actual tutorial scope.
In `@src/jmcomic/api.py`:
- Around line 68-69: The batch branch in download_album and download_photo
ignores the extra parameter so features in extra never run for iterable IDs;
update the calls to download_batch(...) to forward the extra argument (e.g.,
change download_batch(download_album, jm_album_id, option, downloader) to
download_batch(download_album, jm_album_id, option, downloader, extra)) and do
the same for the corresponding call in download_photo so download_batch receives
and passes extra through to individual downloads.
In `@src/jmcomic/jm_config.py`:
- Around line 597-599: The code currently calls _os.system('') under the
sys.platform == 'win32' branch, which spawns a shell; replace this with direct
Win32 console mode API calls: use GetStdHandle, GetConsoleMode, SetConsoleMode
and enable the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag (0x0004) for the
STD_OUTPUT_HANDLE to turn on ANSI processing. Locate the platform check
(sys.platform == 'win32') and the _os.system('') call in jm_config.py and swap
it for a ctypes-based implementation that retrieves the stdout handle, reads the
current console mode, ORs in ENABLE_VIRTUAL_TERMINAL_PROCESSING, and writes it
back, handling errors/return codes appropriately.
In `@src/jmcomic/jm_downloader.py`:
- Around line 290-298: The registration code in add_features/feature handling
appends unknown `features` types to self._feature_list and defers errors until
hook invocation; validate types at registration time by checking if `features`
is a list, a FeatureChain (inspect FeatureChain._features), or a supported
single feature type (e.g., the expected Feature class/interface) and raise a
clear TypeError for any unsupported `features` value instead of appending it;
update the branch that currently does self._feature_list.append((features,
feature_from)) to perform an explicit isinstance/issubclass check (or
duck-typing validation) and raise with a helpful message referencing the
offending value.
In `@src/jmcomic/jm_feature.py`:
- Around line 96-103: The current __call__ in PluginFeature overwrites
_user_keys with only the latest kwargs keys, losing previously recorded explicit
keys; change it to merge the previous instance's _user_keys with the new kwargs
keys (e.g., new_instance._user_keys = (getattr(self, "_user_keys", set()) |
set(kwargs.keys()))) so earlier explicit parameters are preserved across chained
PluginFeature(...) calls; ensure you reference __call__, PluginFeature,
_user_keys and kwargs when making this change.
---
Nitpick comments:
In `@tests/test_jmcomic/test_jm_feature.py`:
- Around line 89-112: Add a test that exercises the `extra` Feature when using
batch input: create a `MyCounterFeature` (subclassing `Feature`) that increments
a counter in `invoke`, then call `jmcomic.download_batch([...], self.option,
extra=counter_feature)` with an iterable of IDs and assert the counter increased
for each expected `after_photo`/`after_album` invocation; this mirrors the
existing `test_download_use_feature` flow (which calls `download_album` and
`download_photo`) but targets `download_batch` to ensure `extra` is not skipped
when handling iterable inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 397052b3-440e-4769-968d-68868f23af61
📒 Files selected for processing (11)
README.mdassets/docs/mkdocs.ymlassets/docs/sources/index.mdassets/docs/sources/tutorial/13_export_and_feature.mdsrc/jmcomic/__init__.pysrc/jmcomic/api.pysrc/jmcomic/jm_config.pysrc/jmcomic/jm_downloader.pysrc/jmcomic/jm_feature.pysrc/jmcomic/jm_option.pytests/test_jmcomic/test_jm_feature.py
| def __call__(self, **kwargs): | ||
| """带自定义参数，返回新实例（继承默认参数）""" | ||
| new_kwargs = self.kwargs.copy() | ||
| new_kwargs.update(kwargs) | ||
| new_instance = PluginFeature(self.plugin_key, **new_kwargs) | ||
| # 记录用户显式传入的参数名，这些参数不被动态适配 | ||
| new_instance._user_keys = set(kwargs.keys()) | ||
| return new_instance |
There was a problem hiding this comment.
Preserve explicit user keys across chained PluginFeature(...) calls.
Line 102 currently tracks only keys from the latest call, so earlier explicit keys can be lost and later auto-adapted unexpectedly.
🔧 Proposed fix
def __call__(self, **kwargs):
"""带自定义参数，返回新实例（继承默认参数）"""
new_kwargs = self.kwargs.copy()
new_kwargs.update(kwargs)
new_instance = PluginFeature(self.plugin_key, **new_kwargs)
# 记录用户显式传入的参数名，这些参数不被动态适配
- new_instance._user_keys = set(kwargs.keys())
+ new_instance._user_keys = set(self._user_keys) | set(kwargs.keys())
return new_instance🧰 Tools
🪛 Ruff (0.15.12)
[warning] 97-97: Docstring contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF002)
[warning] 97-97: Docstring contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF002)
[warning] 97-97: Docstring contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF002)
[warning] 101-101: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jmcomic/jm_feature.py` around lines 96 - 103, The current __call__ in
PluginFeature overwrites _user_keys with only the latest kwargs keys, losing
previously recorded explicit keys; change it to merge the previous instance's
_user_keys with the new kwargs keys (e.g., new_instance._user_keys =
(getattr(self, "_user_keys", set()) | set(kwargs.keys()))) so earlier explicit
parameters are preserved across chained PluginFeature(...) calls; ensure you
reference __call__, PluginFeature, _user_keys and kwargs when making this
change.
修复内容： 1. [文档修正] 修正 13_export_and_feature.md 中的失效锚点链接 2. [文档完善] 补全 README.md 中的教程支持范围描述（增加长图） 3. [核心修复] 修复批量下载时没有透传 extra 参数导致 Feature 无法执行的严重 Bug 4. [性能优化] 改用 ctypes 启用 Windows ANSI 控制台支持，移除低效且不安全的 os.system 进程调用 5. [错误捕获] 提前在 jm_downloader.py 的 add_features 方法中对 extra 进行类型校验 6. [认知升级] 更新代码中的 docstring，强化 Feature 作为“基于生命周期和调用来源的动态自适应机制”的定位 未修改内容及原因： 1. jm_feature.py 链式调用丢失 _user_keys 问题 - 经与开发者确认，暂时先不修改此逻辑。
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/jmcomic/api.py (1)
6-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReserve
callbackparameter explicitly before forwarding**kwargs.Line 10 accepts
**kwargswhich are forwarded todownload_api()on line 41. However, line 40 already passescallback=callbackto the same function. If a caller passescallback=throughdownload_batch(), Python raisesTypeError: got multiple values for keyword argument 'callback'before the download runs, making this forwarding surface unsafe for that parameter.🔧 Proposed fix
def download_batch(download_api, jm_id_iter: Union[Iterable, Generator], option=None, downloader=None, + callback=None, **kwargs, ) -> Set[__DOWNLOAD_API_RET]: @@ - def callback(*ret): + def collect_result(*ret): result.add(ret) + if callback is not None: + callback(*ret) @@ - apply_each_obj_func=lambda aid: download_api(aid, - option, - downloader, - callback=callback, - **kwargs, - ), + apply_each_obj_func=lambda aid: download_api( + aid, + option, + downloader, + callback=collect_result, + **kwargs, + ), wait_finish=True )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jmcomic/api.py` around lines 6 - 10, The download_batch function currently accepts **kwargs and also passes callback=callback to download_api, which can cause a TypeError if caller supplies callback in **kwargs; update the download_batch signature to explicitly accept callback (e.g., def download_batch(..., callback=None, **kwargs)) and when forwarding to download_api use callback=callback (or ensure you pop 'callback' from kwargs before calling) so there are no duplicate keyword arguments; reference the download_batch function and the download_api call when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/docs/sources/tutorial/13_export_and_feature.md`:
- Around line 35-38: Add explicit fence language "text" to the plain-text code
blocks in this markdown so they no longer trigger MD040: replace each
triple-backtick fence (```) around the directory/tree and flow
examples—specifically the blocks containing the "./" tree, the "[章节标题].pdf"
example, the "Feature (基类)" block, and the "api.download_album(...)" flow—so
each opens with ```text; do the same for the other occurrences referenced
(blocks around lines 83-86, 127-131, 140-161) to keep rendering unchanged while
satisfying the linter.
---
Outside diff comments:
In `@src/jmcomic/api.py`:
- Around line 6-10: The download_batch function currently accepts **kwargs and
also passes callback=callback to download_api, which can cause a TypeError if
caller supplies callback in **kwargs; update the download_batch signature to
explicitly accept callback (e.g., def download_batch(..., callback=None,
**kwargs)) and when forwarding to download_api use callback=callback (or ensure
you pop 'callback' from kwargs before calling) so there are no duplicate keyword
arguments; reference the download_batch function and the download_api call when
making this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e060789-bafd-4c19-9756-e717a2c75a29
📒 Files selected for processing (6)
README.mdassets/docs/sources/tutorial/13_export_and_feature.mdsrc/jmcomic/api.pysrc/jmcomic/jm_config.pysrc/jmcomic/jm_downloader.pysrc/jmcomic/jm_feature.py
- ZipPlugin: level 默认值改为 None，根据上下文自动推导（有 album 则合并，只有 photo 则单章） - Feature: export_zip 不再传递 level，_adapt_kwargs 移除 level 适配逻辑 - JmOption: compatible_with_old_versions 中新增 _migrate_zip_level，自动将旧 yaml 中的 level 配置等价迁移到正确的钩子位置 - 文档: 更新 option_file_syntax.md 和 13_export_and_feature.md 中 level 相关说明
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
assets/docs/sources/option_file_syntax.md (1)
237-253:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe
after_albumZIP examples still use photo-scoped placeholders.This section is configuring
zipunderafter_album, butfilename_rule: Ptitleand thedir_ruleexample withPid/Ptitleboth require aphotocontext. With the new hook-driven behavior there is no per-photo naming context here, so these examples now contradict the added guidance on Lines 239-241 and will break if users copy them verbatim. UseAxxxplaceholders in this section, or move the photo-scoped examples underafter_photo.🔧 Proposed fix
- filename_rule: Ptitle # 压缩文件的命名规则 + filename_rule: Atitle # 压缩文件的命名规则 ... - # rule: 'Bd / {Atitle} / [{Pid}]-{Ptitle}.zip' # 设置压缩文件夹规则，中间Atitle表示创建一层文件夹，名称是本子标题。[{Pid}]-{Ptitle}.zip 表示压缩文件的命名规则(需显式写出后缀名) - # 使用此方法指定压缩包存储路径则无需和所在钩子对应 + # rule: 'Bd / {Atitle} / [{Aid}]-{Atitle}.zip' # 设置压缩文件夹规则，中间Atitle表示创建一层文件夹，名称是本子标题。[{Aid}]-{Atitle}.zip 表示压缩文件的命名规则(需显式写出后缀名) + # 使用此方法指定压缩包存储路径时，DSL 里的 A/P 变量仍需和所在钩子对应🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/docs/sources/option_file_syntax.md` around lines 237 - 253, The examples under the after_album section use photo-scoped placeholders (filename_rule: Ptitle and dir_rule examples with {Pid}/{Ptitle}) which are invalid in album scope; update those examples to use album-scoped placeholders (Axxx) instead or move the photo-scoped examples into the after_photo section; specifically, change filename_rule Ptitle to an Axxx variant (e.g., Atitle/Aid), update the dir_rule example to use {Atitle}/{Aid} and an album-level filename pattern, and ensure the comments mention that filename_rule, zip_dir and suffix are ignored when dir_rule is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/docs/sources/tutorial/13_export_and_feature.md`:
- Around line 56-62: The example using download_album passes
filename_rule='Ptitle' which PluginFeature.__call__ treats as user-owned and
will be evaluated without a photo context in album scope, causing a runtime
error; update the snippet to use an album-scoped rule (e.g.,
filename_rule='Atitle') or move this example into download_photo so the Pxxx
rule has a photo context, referencing the download_album call and
Feature.export_pdf usage to locate the code to change.
- Around line 134-166: The docs incorrectly state that
PluginFeature/_adapt_kwargs adapt the `level`; update the prose and sequence
diagram to reflect that `level` is now inferred inside ZipPlugin.invoke() and
that PluginFeature._adapt_kwargs() only rewrites `filename_rule` (not `level`),
and adjust the example flow where `_adapt_kwargs('download_album')` is shown to
only remap filename prefixes (A/P) while `ZipPlugin.invoke()` determines
`level`; mention that user-provided params are still preserved and keep
references to PluginFeature, _adapt_kwargs, ZipPlugin.invoke, should_invoke,
`filename_rule`, and `level` so readers know where to look in the code.
In `@src/jmcomic/jm_option.py`:
- Around line 364-377: The migration currently only moves zip with level=photo
from after_album→after_photo but fails to handle after_photo entries with
level='album'; update the migration in jm_option.py (the block handling
group/level, plugin_list, plugins, pinfo, i, and jm_log) to detect when group ==
'after_photo' and level == 'album' and perform the opposite migration: append
pinfo to plugins.setdefault('after_album', []), remove the entry from
plugin_list (plugin_list.pop(i)), emit a jm_log message describing the migration
to after_album, and avoid simply deleting the level; ensure the index i is only
incremented when you do not pop the current item so loop behavior remains
correct.
---
Outside diff comments:
In `@assets/docs/sources/option_file_syntax.md`:
- Around line 237-253: The examples under the after_album section use
photo-scoped placeholders (filename_rule: Ptitle and dir_rule examples with
{Pid}/{Ptitle}) which are invalid in album scope; update those examples to use
album-scoped placeholders (Axxx) instead or move the photo-scoped examples into
the after_photo section; specifically, change filename_rule Ptitle to an Axxx
variant (e.g., Atitle/Aid), update the dir_rule example to use {Atitle}/{Aid}
and an album-level filename pattern, and ensure the comments mention that
filename_rule, zip_dir and suffix are ignored when dir_rule is present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bf0bd43-acd7-4ed9-b8a8-58c95eb5ea48
📒 Files selected for processing (5)
assets/docs/sources/option_file_syntax.mdassets/docs/sources/tutorial/13_export_and_feature.mdsrc/jmcomic/jm_feature.pysrc/jmcomic/jm_option.pysrc/jmcomic/jm_plugin.py
| if group == 'after_album' and level != 'album': | ||
| # after_album + level=photo → 等价迁移到 after_photo | ||
| plugins.setdefault('after_photo', []).append(pinfo) | ||
| plugin_list.pop(i) | ||
| jm_log('option.migrate', | ||
| f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。' | ||
| f'已自动将 after_album 下的 zip(level={level!r}) 等价迁移到 after_photo。' | ||
| f'等价写法：将 zip 插件从 after_album 移至 after_photo 并删除 level 配置项。') | ||
| else: | ||
| if level != 'photo': | ||
| jm_log('option.migrate', | ||
| f'[zip 插件迁移] level 参数已废弃，已自动移除。' | ||
| f'打包粒度由所在钩子自动推导（{group} → {level}）。') | ||
| i += 1 |
There was a problem hiding this comment.
after_photo + level: album is not migrated to after_album.
The docstring says album-level ZIP configs should “确保在 after_album”, but this branch only handles the opposite direction. For an old config like after_photo: [{plugin: zip, kwargs: {level: album}}], the code just removes level and keeps the plugin in after_photo, silently changing behavior from one merged archive to per-photo archives.
🔧 Proposed fix
- if group == 'after_album' and level != 'album':
+ if group == 'after_album' and level != 'album':
# after_album + level=photo → 等价迁移到 after_photo
plugins.setdefault('after_photo', []).append(pinfo)
plugin_list.pop(i)
jm_log('option.migrate',
f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。'
f'已自动将 after_album 下的 zip(level={level!r}) 等价迁移到 after_photo。'
f'等价写法：将 zip 插件从 after_album 移至 after_photo 并删除 level 配置项。')
+ elif group == 'after_photo' and level == 'album':
+ # after_photo + level=album → 等价迁移到 after_album
+ plugins.setdefault('after_album', []).append(pinfo)
+ plugin_list.pop(i)
+ jm_log('option.migrate',
+ f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。'
+ f'已自动将 after_photo 下的 zip(level={level!r}) 等价迁移到 after_album。'
+ f'等价写法：将 zip 插件从 after_photo 移至 after_album 并删除 level 配置项。')
else:
if level != 'photo':
jm_log('option.migrate',
f'[zip 插件迁移] level 参数已废弃，已自动移除。'
f'打包粒度由所在钩子自动推导（{group} → {level}）。')
i += 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if group == 'after_album' and level != 'album': | |
| # after_album + level=photo → 等价迁移到 after_photo | |
| plugins.setdefault('after_photo', []).append(pinfo) | |
| plugin_list.pop(i) | |
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。' | |
| f'已自动将 after_album 下的 zip(level={level!r}) 等价迁移到 after_photo。' | |
| f'等价写法：将 zip 插件从 after_album 移至 after_photo 并删除 level 配置项。') | |
| else: | |
| if level != 'photo': | |
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已废弃，已自动移除。' | |
| f'打包粒度由所在钩子自动推导（{group} → {level}）。') | |
| i += 1 | |
| if group == 'after_album' and level != 'album': | |
| # after_album + level=photo → 等价迁移到 after_photo | |
| plugins.setdefault('after_photo', []).append(pinfo) | |
| plugin_list.pop(i) | |
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。' | |
| f'已自动将 after_album 下的 zip(level={level!r}) 等价迁移到 after_photo。' | |
| f'等价写法：将 zip 插件从 after_album 移至 after_photo 并删除 level 配置项。') | |
| elif group == 'after_photo' and level == 'album': | |
| # after_photo + level=album → 等价迁移到 after_album | |
| plugins.setdefault('after_album', []).append(pinfo) | |
| plugin_list.pop(i) | |
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已废弃，打包粒度由所在钩子自动推导。' | |
| f'已自动将 after_photo 下的 zip(level={level!r}) 等价迁移到 after_album。' | |
| f'等价写法：将 zip 插件从 after_photo 移至 after_album 并删除 level 配置项。') | |
| else: | |
| if level != 'photo': | |
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已废弃，已自动移除。' | |
| f'打包粒度由所在钩子自动推导（{group} → {level}）。') | |
| i += 1 |
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 369-369: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 371-371: String contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF001)
[warning] 375-375: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 376-376: String contains ambiguous （ (FULLWIDTH LEFT PARENTHESIS). Did you mean ( (LEFT PARENTHESIS)?
(RUF001)
[warning] 376-376: String contains ambiguous ） (FULLWIDTH RIGHT PARENTHESIS). Did you mean ) (RIGHT PARENTHESIS)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jmcomic/jm_option.py` around lines 364 - 377, The migration currently
only moves zip with level=photo from after_album→after_photo but fails to handle
after_photo entries with level='album'; update the migration in jm_option.py
(the block handling group/level, plugin_list, plugins, pinfo, i, and jm_log) to
detect when group == 'after_photo' and level == 'album' and perform the opposite
migration: append pinfo to plugins.setdefault('after_album', []), remove the
entry from plugin_list (plugin_list.pop(i)), emit a jm_log message describing
the migration to after_album, and avoid simply deleting the level; ensure the
index i is only incremented when you do not pop the current item so loop
behavior remains correct.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
src/jmcomic/jm_feature.py (1)
98-105:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winTypo
_user_0keyssilently breaks the explicit-kwargs tracking; also merge prior keys.Two issues stacked here:
new_instance._user_0keys = ...is a typo for_user_keys(note the stray0). The attribute set on__init__(self._user_keys: set = set()) is never overwritten, so_user_keysalways stays empty and any “explicit user kwargs” logic that relies on it (now or in future) is dead.- Even after fixing the typo, this assignment overwrites previously recorded user keys when
PluginFeature(...)is chained, which the previous review already raised. Merge withself._user_keys.🔧 Proposed fix
def __call__(self, **kwargs): """带自定义参数，返回新实例（继承默认参数）""" new_kwargs = self.kwargs.copy() new_kwargs.update(kwargs) new_instance = PluginFeature(self.plugin_key, **new_kwargs) - # 记录用户显式传入的参数名，这些参数不被动态适配 - new_instance._user_0keys = set(kwargs.keys()) + # 记录用户显式传入的参数名，这些参数不被动态适配 + new_instance._user_keys = set(self._user_keys) | set(kwargs.keys()) return new_instance🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jmcomic/jm_feature.py` around lines 98 - 105, In PluginFeature.__call__, fix the typo that sets new_instance._user_0keys (should be _user_keys) and merge existing keys instead of overwriting them: compute the union of self._user_keys and the keys from kwargs and assign that to new_instance._user_keys so chained calls preserve prior explicit-kwargs tracking; keep the rest of the new_kwargs logic the same.assets/docs/sources/tutorial/13_export_and_feature.md (2)
73-80:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
filename_rule='Ptitle'in adownload_albumexample will fail at runtime.Current
PluginFeature._adapt_plugin_kwargsonly fills a default viasetdefault, so the explicitPtitleis forwarded unchanged intoimg2pdfinvoked from theafter_albumhook — nophotois in scope there, so the rule resolution will blow up. Use anAxxxrule here, or move the snippet todownload_photo.🔧 Proposed fix
download_album('123', option, extra=Feature.export_pdf( # 下面是自定义参数 pdf_dir='D:/my_pdfs', # PDF 保存到 D:/my_pdfs 文件夹 - filename_rule='Ptitle', # 用章节标题作为文件名 + filename_rule='Atitle', # 用本子标题作为文件名 delete_original_file=True, # 合并完 PDF 后删除原图 ))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/docs/sources/tutorial/13_export_and_feature.md` around lines 73 - 80, The example uses filename_rule='Ptitle' inside download_album which will fail because PluginFeature._adapt_plugin_kwargs only setdefaults and the after_album hook calls img2pdf without a photo in scope; change the example to use an album-level rule (e.g. an Axxx rule) or move the snippet into download_photo, and/or update PluginFeature._adapt_plugin_kwargs to resolve/translate filename_rule for album contexts before forwarding to after_album; specifically adjust the example referencing Feature.export_pdf and filename_rule or alter PluginFeature._adapt_plugin_kwargs so it resolves Ptitle when used in after_album or rejects/rewrites it to an A-prefixed rule.
177-184:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFlow diagram and bullet still claim
levelis adapted by_adapt_kwargs.
levelis no longer adapted byPluginFeature._adapt_plugin_kwargs(it was migrated out, andZipPlugininfers granularity at runtime). Also,_adapt_plugin_kwargsonly sets a defaultfilename_rule; it does not actually swapPtitle→Atitle/Pid→Aidas the comment in the diagram (# Atitle 不变, Ptitle→Atitle, Pid→Aid) suggests. Either align the prose to “fills a defaultfilename_ruleif not provided; ZIP packaging level is inferred by the plugin” or add the prefix-swap logic in code (see related comment onjm_feature.py).src/jmcomic/jm_option.py (1)
364-380:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
after_photo + level: albumis still not migrated.The else-branch on Line 376 just removes
leveland leaves the plugin inafter_photo, silently turning a legacyafter_photo: [{plugin: zip, kwargs: {level: album}}]config from “one merged archive per album” into “one archive per photo”. Mirror theafter_album → after_photomigration so the inverse case rehomes toafter_albuminstead of changing semantics.🔧 Proposed fix
if group == 'after_album' and level != 'album': # after_album + level=photo → 等价迁移到 after_photo plugins.setdefault('after_photo', []).append(pinfo) plugin_list.pop(i) jm_log('option.migrate', ...) + elif group == 'after_photo' and level == 'album': + # after_photo + level=album → 等价迁移到 after_album + plugins.setdefault('after_album', []).append(pinfo) + plugin_list.pop(i) + jm_log('option.migrate', + f'[zip 插件迁移] level 参数已过时，已自动将 after_photo 下的 ' + f'zip(level={level!r}) 等价迁移到 after_album。') else: if level != 'photo': jm_log('option.migrate', ...) i += 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jmcomic/jm_option.py` around lines 364 - 380, The migration currently only moves plugins when group == 'after_album' and level != 'album', but it fails to handle the inverse legacy case where group == 'after_photo' and level == 'album'; instead it leaves the plugin in plugin_list and logs removing the level which changes semantics. Update the logic in jm_option.py around the block using variables group, level, plugins, plugin_list, pinfo and i so that when group == 'after_photo' and level == 'album' you rehome the plugin to plugins.setdefault('after_album', []).append(pinfo), remove it from plugin_list (plugin_list.pop(i)), emit a jm_log message mirroring the existing '[zip 插件迁移]' guidance (suggest the equivalent after_album config) and do not increment i for that branch; keep the existing behavior for other cases (including logging and i += 1) intact.
🧹 Nitpick comments (1)
src/jmcomic/jm_feature.py (1)
75-96: 💤 Low value
PluginFeature.kwargsis shared mutable state between instances created via__call__.
__init__keeps a reference to the caller-suppliedkwargsdict (self.kwargs = kwargs). For the predefined module-level instances on Lines 170-172 (Feature.export_pdfetc.), the dict is the literal passed in at module import.__call__doesself.kwargs.copy()so derived instances are safe, but anyone mutatingFeature.export_pdf.kwargsdirectly (or via_adapt_plugin_kwargsif it stopped copying) would scribble on the singleton. Recommend defensive-copying in__init__and usingMappingProxyTypeif you want immutability.🔧 Suggested hardening
def __init__(self, plugin_key, **kwargs): self.plugin_key = plugin_key - self.kwargs = kwargs + self.kwargs = dict(kwargs) # 用户通过 __call__ 显式传入的参数名，这些参数不会被 _adapt_kwargs 动态适配 self._user_keys: set = set()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jmcomic/jm_feature.py` around lines 75 - 96, PluginFeature currently stores a direct reference to the caller-supplied kwargs in PluginFeature.__init__ (self.kwargs = kwargs), which creates shared mutable state for module-level feature singletons like Feature.export_pdf and can be mutated by callers or helpers such as _adapt_plugin_kwargs; change __init__ to defensively copy the mapping (e.g., use kwargs.copy()) or wrap it in an immutable MappingProxyType to prevent external mutations, and ensure the existing __call__ and _adapt_plugin_kwargs behavior still operate on copies rather than the original stored dict.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/docs/sources/tutorial/13_export_and_feature.md`:
- Around line 55-58: The OR-syntax example uses a fullwidth vertical bar (｜)
which is not Python's bitwise-or operator; update the example in the
download_album call to use the ASCII pipe character so it reads
extra=Feature.export_pdf | Feature.export_zip (refer to the download_album(...)
example and the symbols Feature.export_pdf and Feature.export_zip).
In `@src/jmcomic/jm_feature.py`:
- Around line 19-21: Remove the problematic unused import "LiteralString" from
the top of src/jmcomic/jm_feature.py (the line "from typing import
LiteralString") because it breaks Python 3.9/3.10 CI; simply delete that import
so the file only imports the plugin via "from .jm_plugin import *". If you later
actually need LiteralString, import it from typing_extensions and add a
version/gating check instead of importing it directly from typing.
- Around line 125-132: _adapt_plugin_kwargs currently forces full filename_rule
strings and ignores user overrides; update it to use bare rule defaults (e.g.,
'Atitle' for albums, 'Ptitle' for photos) and perform prefix swapping only for
keys not in self._user_keys: if feature_from == 'download_album' set default
filename_rule to 'Atitle' (and map any default rules by replacing leading
'P'→'A'), if feature_from == 'download_photo' set default to 'Ptitle' (and
replace leading 'A'→'P'); do not modify filename_rule (or any other key) when
that key exists in self._user_keys, and continue to base defaults on
self.kwargs.copy().
In `@src/jmcomic/jm_option.py`:
- Around line 378-379: In jm_option.py remove the unnecessary f-string prefix in
the jm_log call: replace jm_log('option.migrate', f'[zip 插件迁移] level
参数已过时，你可以直接删除该参数，不会有任何影响') with a plain string literal (jm_log('option.migrate',
'[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响')) so the f prefix is dropped and
Ruff F541 is avoided.
In `@tests/test_jmcomic/test_jm_feature.py`:
- Around line 63-85: The test references an undefined symbol "when" and asserts
filename_rule values that don't match PluginFeature._adapt_plugin_kwargs
behavior; define or import a valid when value (e.g., when='after_album' or
import the constant used by jm_feature) before calling
Feature.export_pdf/_adapt_plugin_kwargs and then update the assertions to match
the current implementation's outputs (e.g., expect '[JM{Aid}]{Atitle}' or
'[JM{Pid}]{Ptitle}' etc) OR alternatively change
jm_feature.PluginFeature._adapt_plugin_kwargs to return the bare adapted tokens
(like 'Atitle'/'Ptitle'/'Aid') if you prefer to keep the original
assertions—pick one approach and make the change consistently for
Feature.export_pdf, Feature.export_zip, Feature.export_long_img and the custom
Feature.export_zip(filename_rule=...) test.
---
Duplicate comments:
In `@assets/docs/sources/tutorial/13_export_and_feature.md`:
- Around line 73-80: The example uses filename_rule='Ptitle' inside
download_album which will fail because PluginFeature._adapt_plugin_kwargs only
setdefaults and the after_album hook calls img2pdf without a photo in scope;
change the example to use an album-level rule (e.g. an Axxx rule) or move the
snippet into download_photo, and/or update PluginFeature._adapt_plugin_kwargs to
resolve/translate filename_rule for album contexts before forwarding to
after_album; specifically adjust the example referencing Feature.export_pdf and
filename_rule or alter PluginFeature._adapt_plugin_kwargs so it resolves Ptitle
when used in after_album or rejects/rewrites it to an A-prefixed rule.
In `@src/jmcomic/jm_feature.py`:
- Around line 98-105: In PluginFeature.__call__, fix the typo that sets
new_instance._user_0keys (should be _user_keys) and merge existing keys instead
of overwriting them: compute the union of self._user_keys and the keys from
kwargs and assign that to new_instance._user_keys so chained calls preserve
prior explicit-kwargs tracking; keep the rest of the new_kwargs logic the same.
In `@src/jmcomic/jm_option.py`:
- Around line 364-380: The migration currently only moves plugins when group ==
'after_album' and level != 'album', but it fails to handle the inverse legacy
case where group == 'after_photo' and level == 'album'; instead it leaves the
plugin in plugin_list and logs removing the level which changes semantics.
Update the logic in jm_option.py around the block using variables group, level,
plugins, plugin_list, pinfo and i so that when group == 'after_photo' and level
== 'album' you rehome the plugin to plugins.setdefault('after_album',
[]).append(pinfo), remove it from plugin_list (plugin_list.pop(i)), emit a
jm_log message mirroring the existing '[zip 插件迁移]' guidance (suggest the
equivalent after_album config) and do not increment i for that branch; keep the
existing behavior for other cases (including logging and i += 1) intact.
---
Nitpick comments:
In `@src/jmcomic/jm_feature.py`:
- Around line 75-96: PluginFeature currently stores a direct reference to the
caller-supplied kwargs in PluginFeature.__init__ (self.kwargs = kwargs), which
creates shared mutable state for module-level feature singletons like
Feature.export_pdf and can be mutated by callers or helpers such as
_adapt_plugin_kwargs; change __init__ to defensively copy the mapping (e.g., use
kwargs.copy()) or wrap it in an immutable MappingProxyType to prevent external
mutations, and ensure the existing __call__ and _adapt_plugin_kwargs behavior
still operate on copies rather than the original stored dict.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ac8b097-2eec-408c-9b14-09650ab81b2a
📒 Files selected for processing (6)
assets/docs/sources/option_file_syntax.mdassets/docs/sources/tutorial/13_export_and_feature.mdsrc/jmcomic/jm_downloader.pysrc/jmcomic/jm_feature.pysrc/jmcomic/jm_option.pytests/test_jmcomic/test_jm_feature.py
| # 也支持列表语法，|语法 | ||
| download_album('123', option, extra=[Feature.export_pdf, Feature.export_zip]) | ||
| download_album('123', option, extra=Feature.export_pdf ｜ Feature.export_zip) | ||
| ``` |
There was a problem hiding this comment.
Replace fullwidth ｜ with ASCII | in the OR-syntax example.
Line 57 uses ｜ (U+FF5C, FULLWIDTH VERTICAL LINE), which is not the Python bitwise-or operator. A user copy-pasting this snippet will hit a SyntaxError.
🔧 Proposed fix
# 也支持列表语法，|语法
download_album('123', option, extra=[Feature.export_pdf, Feature.export_zip])
-download_album('123', option, extra=Feature.export_pdf ｜ Feature.export_zip)
+download_album('123', option, extra=Feature.export_pdf | Feature.export_zip)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@assets/docs/sources/tutorial/13_export_and_feature.md` around lines 55 - 58,
The OR-syntax example uses a fullwidth vertical bar (｜) which is not Python's
bitwise-or operator; update the example in the download_album call to use the
ASCII pipe character so it reads extra=Feature.export_pdf | Feature.export_zip
(refer to the download_album(...) example and the symbols Feature.export_pdf and
Feature.export_zip).
| from typing import LiteralString | ||
|
|
||
| from .jm_plugin import * |
There was a problem hiding this comment.
Drop the LiteralString import — it breaks Python 3.9/3.10 CI and is unused.
typing.LiteralString was added in 3.11; on 3.9/3.10 the existing from typing import LiteralString raises ImportError at import time, which is exactly what the pipeline run logs show for Run Test (API) / test (3.9, …) and (3.10, …). The symbol is also not referenced anywhere in this file.
🔧 Proposed fix
-from typing import LiteralString
-
from .jm_plugin import *If you actually need LiteralString somewhere later, import it from typing_extensions and gate by version.
🧰 Tools
🪛 GitHub Actions: Run Test (API) / 2_test (3.10, ubuntu-latest).txt
[error] 19-19: ImportError: cannot import name 'LiteralString' from 'typing'. The code uses 'from typing import LiteralString', which is not available in Python 3.10; use 'typing_extensions.LiteralString' or remove usage.
🪛 GitHub Actions: Run Test (API) / 3_test (3.9, ubuntu-latest).txt
[error] 19-19: ImportError: cannot import name 'LiteralString' from 'typing'. In Python 3.9, LiteralString is not available; consider using typing_extensions.LiteralString or refactor to a compatible approach. This occurred while running 'python -m unittest'.
🪛 GitHub Actions: Run Test (API) / test (3.10, ubuntu-latest)
[error] 19-19: ImportError: cannot import name 'LiteralString' from 'typing' during unit tests ('python -m unittest'). In Python 3.10, LiteralString is not available in typing. Use 'typing_extensions.LiteralString' or replace with an appropriate type (e.g., str).
🪛 GitHub Actions: Run Test (API) / test (3.9, ubuntu-latest)
[error] 19-19: ImportError: cannot import name 'LiteralString' from 'typing' (Python 3.9). Use typing_extensions.LiteralString or adjust type usage.
🪛 Ruff (0.15.12)
[error] 21-21: from .jm_plugin import * used; unable to detect undefined names
(F403)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jmcomic/jm_feature.py` around lines 19 - 21, Remove the problematic
unused import "LiteralString" from the top of src/jmcomic/jm_feature.py (the
line "from typing import LiteralString") because it breaks Python 3.9/3.10 CI;
simply delete that import so the file only imports the plugin via "from
.jm_plugin import *". If you later actually need LiteralString, import it from
typing_extensions and add a version/gating check instead of importing it
directly from typing.
| def _adapt_plugin_kwargs(self, feature_from: str, when: str) -> dict: | ||
| """ | ||
| 根据feature_from和when动态确定以下插件参数: | ||
| filename_rule | ||
| """ | ||
| kwargs = self.kwargs.copy() | ||
| kwargs.setdefault('filename_rule', '[JM{Aid}]{Atitle}' if feature_from == 'download_album' else '[JM{Pid}]{Ptitle}') | ||
| return kwargs |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm there's no other prefix-adaptation logic we might be missing.
rg -n -C2 'filename_rule|_adapt_plugin_kwargs|_user_keys'Repository: hect0x7/JMComic-Crawler-Python
Length of output: 15148
_adapt_plugin_kwargs doesn't match the documented contract or tests.
Current behavior: unconditionally sets 'filename_rule' to '[JM{Aid}]{Atitle}' or '[JM{Pid}]{Ptitle}'.
Expected behavior (per tests):
- Use bare rule names (
'Atitle','Ptitle','Aid','Pid') - Swap P↔A prefixes when switching context:
'Ptitle'→'Atitle'indownload_albummode,'Atitle'→'Ptitle'indownload_photomode - Skip adaptation for rules explicitly passed by the user (check
if k not in self._user_keys)
The proposed implementation in the review correctly addresses this by:
- Using bare defaults aligned with plugin signatures (e.g.,
'Atitle'for album,'Ptitle'for photo) - Swapping prefixes only for non-user-supplied rules
- Honoring user-explicit parameters via the
_user_keyscheck
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jmcomic/jm_feature.py` around lines 125 - 132, _adapt_plugin_kwargs
currently forces full filename_rule strings and ignores user overrides; update
it to use bare rule defaults (e.g., 'Atitle' for albums, 'Ptitle' for photos)
and perform prefix swapping only for keys not in self._user_keys: if
feature_from == 'download_album' set default filename_rule to 'Atitle' (and map
any default rules by replacing leading 'P'→'A'), if feature_from ==
'download_photo' set default to 'Ptitle' (and replace leading 'A'→'P'); do not
modify filename_rule (or any other key) when that key exists in self._user_keys,
and continue to base defaults on self.kwargs.copy().
| jm_log('option.migrate', | ||
| f'[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响') |
There was a problem hiding this comment.
Drop the unused f prefix.
The string has no placeholders, so the f prefix is dead and trips Ruff F541. Plain string literal is enough.
🔧 Proposed fix
- jm_log('option.migrate',
- f'[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响')
+ jm_log('option.migrate',
+ '[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jm_log('option.migrate', | |
| f'[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响') | |
| jm_log('option.migrate', | |
| '[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响') |
🧰 Tools
🪛 Ruff (0.15.12)
[error] 379-379: f-string without any placeholders
Remove extraneous f prefix
(F541)
[warning] 379-379: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
[warning] 379-379: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jmcomic/jm_option.py` around lines 378 - 379, In jm_option.py remove the
unnecessary f-string prefix in the jm_log call: replace jm_log('option.migrate',
f'[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响') with a plain string literal
(jm_log('option.migrate', '[zip 插件迁移] level 参数已过时，你可以直接删除该参数，不会有任何影响')) so the f
prefix is dropped and Ruff F541 is avoided.
| def test_adapt_kwargs(self): | ||
| """测试 PluginFeature 参数动态适配""" | ||
| # download_album 模式：P前缀 → A前缀, level → album | ||
| pdf = Feature.export_pdf | ||
| adapted = pdf._adapt_plugin_kwargs('download_album', when) | ||
| self.assertEqual(adapted['filename_rule'], 'Atitle') # A开头不变 | ||
|
|
||
| zip_f = Feature.export_zip | ||
| adapted = zip_f._adapt_plugin_kwargs('download_album', when) | ||
| self.assertEqual(adapted['filename_rule'], 'Atitle') # Ptitle → Atitle | ||
|
|
||
| long_img = Feature.export_long_img | ||
| adapted = long_img._adapt_plugin_kwargs('download_album', when) | ||
| self.assertEqual(adapted['filename_rule'], 'Aid') # Pid → Aid | ||
|
|
||
| # download_photo 模式：A前缀 → P前缀, level → photo | ||
| adapted = pdf._adapt_plugin_kwargs('download_photo', when) | ||
| self.assertEqual(adapted['filename_rule'], 'Ptitle') # Atitle → Ptitle | ||
|
|
||
| # 用户显式传入的参数不被动态适配 | ||
| custom = Feature.export_zip(filename_rule='Ptitle') | ||
| adapted = custom._adapt_plugin_kwargs('download_album', when) | ||
| self.assertEqual(adapted['filename_rule'], 'Ptitle') # 用户显式指定，不适配 |
There was a problem hiding this comment.
when is undefined; assertions also disagree with the current _adapt_plugin_kwargs implementation.
Two compounding problems:
whenis referenced in every_adapt_plugin_kwargs(...)call but never defined or imported (the file only doesfrom . import *). The test will fail withNameError: name 'when' is not definedbefore any assertion runs.- Even after passing
when(e.g.'after_album'),PluginFeature._adapt_plugin_kwargsinjm_feature.pyreturnsfilename_rule='[JM{Aid}]{Atitle}'(or'[JM{Pid}]{Ptitle}') — not the bare'Atitle' / 'Ptitle' / 'Aid'these tests assert. Either the implementation needs to add the documentedP↔Aprefix swap, or these expectations need to be rewritten to match the actual default rule. See the related comment onjm_feature.py:_adapt_plugin_kwargs.
🔧 Minimal fix for the undefined name (assertions still need to be reconciled with the implementation)
def test_adapt_kwargs(self):
"""测试 PluginFeature 参数动态适配"""
# download_album 模式：P前缀 → A前缀, level → album
pdf = Feature.export_pdf
- adapted = pdf._adapt_plugin_kwargs('download_album', when)
+ adapted = pdf._adapt_plugin_kwargs('download_album', 'after_album')
self.assertEqual(adapted['filename_rule'], 'Atitle') # A开头不变
...🧰 Tools
🪛 Ruff (0.15.12)
[warning] 65-65: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF003)
[warning] 78-78: Comment contains ambiguous ： (FULLWIDTH COLON). Did you mean : (COLON)?
(RUF003)
[warning] 85-85: Comment contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_jmcomic/test_jm_feature.py` around lines 63 - 85, The test
references an undefined symbol "when" and asserts filename_rule values that
don't match PluginFeature._adapt_plugin_kwargs behavior; define or import a
valid when value (e.g., when='after_album' or import the constant used by
jm_feature) before calling Feature.export_pdf/_adapt_plugin_kwargs and then
update the assertions to match the current implementation's outputs (e.g.,
expect '[JM{Aid}]{Atitle}' or '[JM{Pid}]{Ptitle}' etc) OR alternatively change
jm_feature.PluginFeature._adapt_plugin_kwargs to return the bare adapted tokens
(like 'Atitle'/'Ptitle'/'Aid') if you prefer to keep the original
assertions—pick one approach and make the change consistently for
Feature.export_pdf, Feature.export_zip, Feature.export_long_img and the custom
Feature.export_zip(filename_rule=...) test.
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
Version: 2.6.19