v2.6.19: 新增 Feature 机制，支持下载时通过 extra 参数附加导出 PDF/ZIP/长图等行为; Feature 参数根据 download_album/download_photo 来源自适应; 新增 Feature 教程文档#533
Conversation
…根据 download_album/download_photo 来源自适应; 新增 Feature 教程文档
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 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
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("after_album","download_album")?
Feature-->>Downloader: True
Downloader->>Feature: invoke(option, album=..., downloader=...)
Feature->>Feature: _adapt_kwargs(feature_from="download_album")
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
Comment on lines
+96
to
+103
| 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
Comment on lines
+35
to
+38
| ``` | ||
| ./ | ||
| ├── [本子标题].pdf ← 整本合并为 1 个 PDF | ||
| ``` |
There was a problem hiding this comment.
Add fence languages to the plain-text examples.
These blocks currently trip MD040. Labeling them as text keeps docs lint clean without changing the rendering.
🔧 Proposed fix
-```
+```text
./
├── [本子标题].pdf ← 整本合并为 1 个 PDF- +text
./
├── [章节标题].pdf ← 该章节导出为 1 个 PDF
-```
+```text
Feature (基类)
├── PluginFeature ← 封装插件调用，参数根据来源自适应
└── 你的自定义 Feature ← 继承 Feature，实现任意逻辑
- +text
api.download_album(extra=Feature.export_pdf)
│
├→ dler.add_features(pdf, 'download_album') # 注册: [(pdf, 'download_album')]
│
└→ dler.download_album(id)
│
├→ before_album(album)
│
├→ download_by_photo_detail(photo)
│ ├→ before_photo(photo)
│ ├→ download images ...
│ └→ after_photo(photo)
│ └→ _invoke_features_for('after_photo')
│ └→ pdf.should_invoke('after_photo', 'download_album') → False ✗ 跳过
│
└→ after_album(album)
└→ _invoke_features_for('after_album')
└→ pdf.should_invoke('after_album', 'download_album') → True ✓ 执行!
└→ _adapt_kwargs('download_album')
# Atitle 不变, Ptitle→Atitle, Pid→Aid, level→album
Also applies to: 83-86, 127-131, 140-161
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 35-35: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 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.
- 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
Comment on lines
+56
to
+62
| ```python | ||
| # 示例 1：指定输出目录和命名规则 | ||
| download_album('123', option, extra=Feature.export_pdf( | ||
| pdf_dir='D:/my_pdfs', # PDF 保存到 D:/my_pdfs 文件夹 | ||
| filename_rule='Ptitle', # 用章节标题作为文件名 | ||
| delete_original_file=True, # 合并完 PDF 后删除原图 | ||
| )) |
There was a problem hiding this comment.
Don't use Ptitle in the download_album example.
PluginFeature.__call__() treats explicit kwargs as user-owned, so this sample will pass filename_rule='Ptitle' through unchanged. In album scope that leaves the export plugin without a photo context for a Pxxx rule, so this “custom parameter” example becomes a runtime failure if copied as-is. Use an Axxx rule here, or move the snippet to download_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 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.
Comment on lines
+134
to
+166
| - **PluginFeature**：Feature 的子类，专门封装 jmcomic 插件。除了调用插件之外，还会根据调用来源动态适配 `filename_rule`、`level` 等参数。 | ||
|
|
||
| ### 执行流程 | ||
|
|
||
| Feature **自然嵌入到 downloader 的生命周期钩子**中自动触发： | ||
|
|
||
| ``` | ||
| api.download_album(extra=Feature.export_pdf) | ||
| │ | ||
| ├→ dler.add_features(pdf, 'download_album') # 注册: [(pdf, 'download_album')] | ||
| │ | ||
| └→ dler.download_album(id) | ||
| │ | ||
| ├→ before_album(album) | ||
| │ | ||
| ├→ download_by_photo_detail(photo) | ||
| │ ├→ before_photo(photo) | ||
| │ ├→ download images ... | ||
| │ └→ after_photo(photo) | ||
| │ └→ _invoke_features_for('after_photo') | ||
| │ └→ pdf.should_invoke('after_photo', 'download_album') → False ✗ 跳过 | ||
| │ | ||
| └→ after_album(album) | ||
| └→ _invoke_features_for('after_album') | ||
| └→ pdf.should_invoke('after_album', 'download_album') → True ✓ 执行! | ||
| └→ _adapt_kwargs('download_album') | ||
| # Atitle 不变, Ptitle→Atitle, Pid→Aid, level→album | ||
| ``` | ||
|
|
||
| > 💡 **关键点**： | ||
| > | ||
| > - **执行时机**：`PluginFeature` 根据注册来源自动推导（`download_album` → `after_album`，`download_photo` → `after_photo`）。自定义 Feature 默认在所有钩子都会执行，你可以覆写 `should_invoke` 来控制。 | ||
| > - **参数自适应**：`PluginFeature` 的 `filename_rule` 前缀（A/P）和 `level`（album/photo）会根据来源动态适配。用户显式传入的参数不会被覆盖。 |
There was a problem hiding this comment.
Update the flow docs to match the new ZIP implementation.
The prose and the sequence diagram still say PluginFeature / _adapt_kwargs() adapt level, but that no longer happens in code. level is now inferred inside ZipPlugin.invoke(), while PluginFeature._adapt_kwargs() only rewrites filename_rule. Leaving the old wording here will send readers to the wrong place when they debug hook behavior.
🔧 Proposed fix
-- **PluginFeature**：Feature 的子类，专门封装 jmcomic 插件。除了调用插件之外，还会根据调用来源动态适配 `filename_rule`、`level` 等参数。
+- **PluginFeature**：Feature 的子类，专门封装 jmcomic 插件。除了调用插件之外，还会根据调用来源动态适配 `filename_rule` 等参数；ZIP 的打包粒度则由插件在运行时根据上下文自动推导。
...
- └→ _adapt_kwargs('download_album')
- # Atitle 不变, Ptitle→Atitle, Pid→Aid, level→album
+ └→ _adapt_kwargs('download_album')
+ # Atitle 不变, Ptitle→Atitle, Pid→Aid
...
-- **参数自适应**：`PluginFeature` 的 `filename_rule` 前缀（A/P）和 `level`（album/photo）会根据来源动态适配。用户显式传入的参数不会被覆盖。
+- **参数自适应**：`PluginFeature` 的 `filename_rule` 前缀（A/P）会根据来源动态适配。ZIP 的打包粒度由插件根据上下文自动推导。用户显式传入的参数不会被覆盖。🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 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.
Comment on lines
+364
to
+377
| 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.
Suggested change
| 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.
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 by CodeRabbit
Version: 2.6.19