feat：增加播客插件#20
Open
ieshishinjin wants to merge 3 commits into
Open
Conversation
Reviewer's Guide添加一个新的“播客订阅器”（podcast reader）插件，包含独立的 Lua 实现、UI 资源和 manifest，其中包括底部持久音频播放器栏、RSS 解析、订阅管理，以及与宿主应用插件框架的集成。 File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience前往你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds a new '播客订阅器' (podcast reader) plugin with its own Lua implementation, UI assets, and manifest, including a bottom persistent audio player bar, RSS parsing, subscription management, and integration with the host app’s plugin framework. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并留下了一些整体性的反馈：
- 在
main.lua中，很多辅助和领域函数（addPodcast、removePodcast、fetchPod、renderUI等）在声明时没有使用local，这会污染全局命名空间；建议将它们标记为local，或者显式挂载到plugin表上，以避免与其他插件发生命名冲突。 - 注入的 JS 里
alignPlayer的逻辑除了通过resize监听器外，还使用setInterval(..., 200)轮询；这种高频轮询在较慢的机器上代价较高，可以考虑改用ResizeObserver，或者只依赖resize/布局事件来更新播放器位置。 podcast-reader.json文件已创建但内容为空；如果不会使用，建议删除，否则应填入预期的配置以免造成困惑。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `main.lua` many helper and domain functions (`addPodcast`, `removePodcast`, `fetchPod`, `renderUI`, etc.) are declared without `local`, which pollutes the global namespace; consider marking them `local` or attaching them explicitly to the `plugin` table to avoid collisions with other plugins.
- The `alignPlayer` logic in the injected JS runs via `setInterval(..., 200)` in addition to a `resize` listener; this frequent polling can be costly on slower machines and could be replaced with a `ResizeObserver` or relying solely on `resize`/layout events to update the player position.
- The `podcast-reader.json` file is created but empty; if it is not used, it should be removed, or otherwise populated with the intended configuration to avoid confusion.
## Individual Comments
### Comment 1
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="190-197" />
<code_context>
+-- ============================================================
+-- 播客管理
+-- ============================================================
+function addPodcast(url)
+ url = trim(url)
+ if not url or #url == 0 then return false, "请输入地址" end
+ if not url:match("^https?://") then return false, "请输入有效地址" end
+ for _, p in ipairs(state.podcasts) do if p.url == url then return false, "已添加过" end end
+ local pod = {id = mkId(url), url = url, title = url, author = "", artwork = "", desc = ""}
+ table.insert(state.podcasts, pod); saveAll()
+ local ok, msg = fetchPod(pod)
+ return true, ok and "已添加" or "添加失败: " .. tostring(msg)
+end
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider reverting the newly inserted podcast if RSS fetching/parsing fails
Because `addPodcast` inserts and saves `pod` before `fetchPod` completes, a failed fetch still returns `true` and leaves a permanently broken entry in `state.podcasts` (and storage). Consider either deferring insertion/saving until after a successful `fetchPod`, or removing/invalidating the newly added `pod` on failure so persisted state and UI only reflect valid podcasts.
</issue_to_address>
### Comment 2
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="106" />
<code_context>
+ else
+ local ce = xml:find(">", ts); if not ce then break end
+ local tc = xml:sub(ts + 1, ce - 1)
+ if tc:match("^%s*$") then i = ce + 1; break end
+ local sc = tc:sub(-1) == "/"
+ if sc then tc = tc:sub(1, -2) end
</code_context>
<issue_to_address>
**issue (bug_risk):** Whitespace-only tag names cause the XML parser to stop parsing early
In `parseXml`, when `tc:match("^%s*$")` is true you currently set `i = ce + 1` and `break` out of the loop. That means a single whitespace-only tag candidate (e.g. from malformed `< >` content) stops parsing and silently drops all remaining XML, which can hide feeds/episodes.
Instead, advance `i` and continue parsing (e.g. restructure the loop or use a `goto` to skip to the next `<`), or simply treat this as a no-op and move on to the next tag, so that one bad segment doesn’t abort the whole parse.
</issue_to_address>
### Comment 3
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="386-395" />
<code_context>
+setInterval(function(){
</code_context>
<issue_to_address>
**suggestion (performance):** Two 200ms polling intervals in the core script may be unnecessarily aggressive
The core script runs two `setInterval` loops at 200ms: one for the seek UI and one repeatedly calling `alignPlayer`. On slower machines or layout-heavy pages, this polling—especially for alignment—can be more frequent than necessary.
Consider keeping 200ms for the seek UI, but lowering the `alignPlayer` interval (e.g., 500–1000ms) or triggering it via `resize` events and a `MutationObserver` on the sidebar instead. This should reduce layout work while keeping the bar in sync.
Suggested implementation:
```
setInterval(function(){
alignPlayer();
},800);
```
` section.
Here are the edits:
<file_operations>
<file_operation operation="edit" file_path="plugins/ieshishinjin/podcast-reader/main.lua">
<<<<<<< SEARCH
setInterval(function(){
alignPlayer();
},200);
=======
setInterval(function(){
alignPlayer();
},800);
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
To fully align with your suggestion of using `resize` events and a `MutationObserver` instead of frequent polling, you will likely want to:
1. Identify the container/sidebar element whose layout changes necessitate realignment (e.g., an element with an id like `pr-sidebar` or similar).
2. Add a `window.addEventListener("resize", function(){ alignPlayer(); });` listener near the `alignPlayer` definition.
3. Instantiate a `MutationObserver` on the sidebar element that calls `alignPlayer()` on relevant DOM mutations (attribute changes, child list changes, subtree changes).
4. Once you are satisfied that `alignPlayer` is kept in sync by these event-based triggers, you can consider removing the `setInterval` entirely or further increasing its interval as a safety net.
These changes require knowledge of the actual DOM structure and naming of elements in the rest of the file, so you will need to adapt the event/observer wiring to match your existing IDs/classes.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的 Review。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
main.luamany helper and domain functions (addPodcast,removePodcast,fetchPod,renderUI, etc.) are declared withoutlocal, which pollutes the global namespace; consider marking themlocalor attaching them explicitly to theplugintable to avoid collisions with other plugins. - The
alignPlayerlogic in the injected JS runs viasetInterval(..., 200)in addition to aresizelistener; this frequent polling can be costly on slower machines and could be replaced with aResizeObserveror relying solely onresize/layout events to update the player position. - The
podcast-reader.jsonfile is created but empty; if it is not used, it should be removed, or otherwise populated with the intended configuration to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `main.lua` many helper and domain functions (`addPodcast`, `removePodcast`, `fetchPod`, `renderUI`, etc.) are declared without `local`, which pollutes the global namespace; consider marking them `local` or attaching them explicitly to the `plugin` table to avoid collisions with other plugins.
- The `alignPlayer` logic in the injected JS runs via `setInterval(..., 200)` in addition to a `resize` listener; this frequent polling can be costly on slower machines and could be replaced with a `ResizeObserver` or relying solely on `resize`/layout events to update the player position.
- The `podcast-reader.json` file is created but empty; if it is not used, it should be removed, or otherwise populated with the intended configuration to avoid confusion.
## Individual Comments
### Comment 1
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="190-197" />
<code_context>
+-- ============================================================
+-- 播客管理
+-- ============================================================
+function addPodcast(url)
+ url = trim(url)
+ if not url or #url == 0 then return false, "请输入地址" end
+ if not url:match("^https?://") then return false, "请输入有效地址" end
+ for _, p in ipairs(state.podcasts) do if p.url == url then return false, "已添加过" end end
+ local pod = {id = mkId(url), url = url, title = url, author = "", artwork = "", desc = ""}
+ table.insert(state.podcasts, pod); saveAll()
+ local ok, msg = fetchPod(pod)
+ return true, ok and "已添加" or "添加失败: " .. tostring(msg)
+end
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider reverting the newly inserted podcast if RSS fetching/parsing fails
Because `addPodcast` inserts and saves `pod` before `fetchPod` completes, a failed fetch still returns `true` and leaves a permanently broken entry in `state.podcasts` (and storage). Consider either deferring insertion/saving until after a successful `fetchPod`, or removing/invalidating the newly added `pod` on failure so persisted state and UI only reflect valid podcasts.
</issue_to_address>
### Comment 2
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="106" />
<code_context>
+ else
+ local ce = xml:find(">", ts); if not ce then break end
+ local tc = xml:sub(ts + 1, ce - 1)
+ if tc:match("^%s*$") then i = ce + 1; break end
+ local sc = tc:sub(-1) == "/"
+ if sc then tc = tc:sub(1, -2) end
</code_context>
<issue_to_address>
**issue (bug_risk):** Whitespace-only tag names cause the XML parser to stop parsing early
In `parseXml`, when `tc:match("^%s*$")` is true you currently set `i = ce + 1` and `break` out of the loop. That means a single whitespace-only tag candidate (e.g. from malformed `< >` content) stops parsing and silently drops all remaining XML, which can hide feeds/episodes.
Instead, advance `i` and continue parsing (e.g. restructure the loop or use a `goto` to skip to the next `<`), or simply treat this as a no-op and move on to the next tag, so that one bad segment doesn’t abort the whole parse.
</issue_to_address>
### Comment 3
<location path="plugins/ieshishinjin/podcast-reader/main.lua" line_range="386-395" />
<code_context>
+setInterval(function(){
</code_context>
<issue_to_address>
**suggestion (performance):** Two 200ms polling intervals in the core script may be unnecessarily aggressive
The core script runs two `setInterval` loops at 200ms: one for the seek UI and one repeatedly calling `alignPlayer`. On slower machines or layout-heavy pages, this polling—especially for alignment—can be more frequent than necessary.
Consider keeping 200ms for the seek UI, but lowering the `alignPlayer` interval (e.g., 500–1000ms) or triggering it via `resize` events and a `MutationObserver` on the sidebar instead. This should reduce layout work while keeping the bar in sync.
Suggested implementation:
```
setInterval(function(){
alignPlayer();
},800);
```
` section.
Here are the edits:
<file_operations>
<file_operation operation="edit" file_path="plugins/ieshishinjin/podcast-reader/main.lua">
<<<<<<< SEARCH
setInterval(function(){
alignPlayer();
},200);
=======
setInterval(function(){
alignPlayer();
},800);
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
To fully align with your suggestion of using `resize` events and a `MutationObserver` instead of frequent polling, you will likely want to:
1. Identify the container/sidebar element whose layout changes necessitate realignment (e.g., an element with an id like `pr-sidebar` or similar).
2. Add a `window.addEventListener("resize", function(){ alignPlayer(); });` listener near the `alignPlayer` definition.
3. Instantiate a `MutationObserver` on the sidebar element that calls `alignPlayer()` on relevant DOM mutations (attribute changes, child list changes, subtree changes).
4. Once you are satisfied that `alignPlayer` is kept in sync by these event-based triggers, you can consider removing the `setInterval` entirely or further increasing its interval as a safety net.
These changes require knowledge of the actual DOM structure and naming of elements in the rest of the file, so you will need to adapt the event/observer wiring to match your existing IDs/classes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+190
to
+197
| function addPodcast(url) | ||
| url = trim(url) | ||
| if not url or #url == 0 then return false, "请输入地址" end | ||
| if not url:match("^https?://") then return false, "请输入有效地址" end | ||
| for _, p in ipairs(state.podcasts) do if p.url == url then return false, "已添加过" end end | ||
| local pod = {id = mkId(url), url = url, title = url, author = "", artwork = "", desc = ""} | ||
| table.insert(state.podcasts, pod); saveAll() | ||
| local ok, msg = fetchPod(pod) |
There was a problem hiding this comment.
issue (bug_risk): 如果 RSS 获取/解析失败，建议回滚新插入的播客
由于 addPodcast 在 fetchPod 完成之前就插入并保存了 pod，当抓取失败时仍然会返回 true，并在 state.podcasts（以及存储）中留下一个永久损坏的条目。建议要么等 fetchPod 成功后再插入/保存，要么在失败时移除/失效刚添加的 pod，从而确保持久化状态和 UI 中只反映有效的播客。
Original comment in English
issue (bug_risk): Consider reverting the newly inserted podcast if RSS fetching/parsing fails
Because addPodcast inserts and saves pod before fetchPod completes, a failed fetch still returns true and leaves a permanently broken entry in state.podcasts (and storage). Consider either deferring insertion/saving until after a successful fetchPod, or removing/invalidating the newly added pod on failure so persisted state and UI only reflect valid podcasts.
| else | ||
| local ce = xml:find(">", ts); if not ce then break end | ||
| local tc = xml:sub(ts + 1, ce - 1) | ||
| if tc:match("^%s*$") then i = ce + 1; break end |
There was a problem hiding this comment.
issue (bug_risk): 仅包含空白的标签名会导致 XML 解析器过早停止解析
在 parseXml 中，当 tc:match("^%s*$") 为真时，你当前会设置 i = ce + 1 并 break 跳出循环。这意味着只要出现一个仅包含空白的标签候选（例如由格式错误的 < > 内容导致），解析就会停止，并静默丢弃后续所有 XML，从而隐藏后面的订阅源/节目。
更好的做法是：前进 i 并继续解析（例如重构循环或使用 goto 跳到下一个 <），或者简单地将其视为 no-op 并移到下一个标签，这样单个坏片段就不会导致整个解析过程被中止。
Original comment in English
issue (bug_risk): Whitespace-only tag names cause the XML parser to stop parsing early
In parseXml, when tc:match("^%s*$") is true you currently set i = ce + 1 and break out of the loop. That means a single whitespace-only tag candidate (e.g. from malformed < > content) stops parsing and silently drops all remaining XML, which can hide feeds/episodes.
Instead, advance i and continue parsing (e.g. restructure the loop or use a goto to skip to the next <), or simply treat this as a no-op and move on to the next tag, so that one bad segment doesn’t abort the whole parse.
Author
|
|
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 Sourcery
添加一个新的播客订阅和播放插件，包含持久化的底部播放器 UI、RSS 解析以及侧边栏集成。
New Features:
Enhancements:
Documentation:
Original summary in English
Summary by Sourcery
Add a new podcast subscription and playback plugin with persistent bottom player UI, RSS parsing, and sidebar integration.
New Features:
Enhancements:
Documentation: