Repository navigation
Conversation
- Add --from / --to flags to 'album' command for date range filtering (YYYY-MM-DD format, both boundary dates are inclusive) - Add --media-only global flag: download only audio/video files without saving post content, pictures, or comments - Media-only mode implicitly enables media downloading regardless of --download_media flag - Display filter statistics after download completion - All existing commands (albums, motions, shop, update) unaffected
|
需要讨论的核心问题有下面两个：1. media是否应该抽为单独的子命令 2. update子命令的幂等性 |
|
我认可你的看法，所以你对我提出的将media提升为单独的子命令的建议意下如何？ |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
afdian/album/album.go (1)
62-71: 💤 Low valueConsider validating that
--fromis not after--to.If a user accidentally reverses the dates (e.g.,
--from 2025-01-01 --to 2024-01-01), all posts will be silently filtered out with no warning.Optional validation
if hasFrom || hasTo { + if hasFrom && hasTo && fromTime.After(toTime) { + return fmt.Errorf("--from 日期不能晚于 --to 日期") + } rangeDesc := ""🤖 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 `@afdian/album/album.go` around lines 62 - 71, Validate the date range after parsing: if both hasFrom and hasTo are true and fromTime.After(toTime), emit a clear error (e.g., via slog.Error or return an error) instead of silently proceeding; locate the logic around hasFrom, hasTo, fromTime, toTime and add a check that logs a descriptive message like "invalid date range: --from is after --to" and returns non-zero/propagates the error (or swap the dates if you prefer that behavior) before the existing slog.Info range logging.
🤖 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 `@afdian/album/album.go`:
- Around line 45-60: The date parsing should be timezone-aware: parse
--from/--to with time.ParseInLocation("2006-01-02", ..., time.Local) so the
dates represent local calendar days (adjust fromTime/toTime accordingly, e.g.,
toTime = t.Add(24*time.Hour - time.Second) in that same location), and when
filtering compare post.PublishTime converted to the same location
(post.PublishTime.In(time.Local)) against fromTime/toTime; update the parsing
block that sets fromTime/toTime (variables fromDate, toDate, hasFrom, hasTo) and
the filter check that uses post.PublishTime (around the existing filter at the
comment referring to line ~107) to use these timezone-consistent values.
In `@storage/writer.go`:
- Around line 21-37: The MediaOnly branch (guarded by cfg.MediaOnly) always
calls afdian.GetPostContent and downloadMedia for audio/video and returns
skipped=false, which bypasses the usual existence/skip logic; update this branch
to check per-media existence before downloading (e.g., test audio/video presence
via the same existence gate used elsewhere or a media-specific helper) and only
call downloadMedia for missing items, aggregating whether any new media was
saved, then return skipped=true when no media was downloaded; keep references to
cfg.MediaOnly, afdian.GetPostContent, and downloadMedia when implementing the
existence checks and the final skipped boolean.
---
Nitpick comments:
In `@afdian/album/album.go`:
- Around line 62-71: Validate the date range after parsing: if both hasFrom and
hasTo are true and fromTime.After(toTime), emit a clear error (e.g., via
slog.Error or return an error) instead of silently proceeding; locate the logic
around hasFrom, hasTo, fromTime, toTime and add a check that logs a descriptive
message like "invalid date range: --from is after --to" and returns
non-zero/propagates the error (or swap the dates if you prefer that behavior)
before the existing slog.Info range logging.
🪄 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 Plus
Run ID: 1140b55d-6d64-4c1a-b075-c5d3337c5a20
📒 Files selected for processing (4)
afdian/album/album.goconfig/config.gomain.gostorage/writer.go
| if fromDate != "" { | ||
| t, err := time.Parse("2006-01-02", fromDate) | ||
| if err != nil { | ||
| return fmt.Errorf("--from 日期格式错误，需要 YYYY-MM-DD: %w", err) | ||
| } | ||
| fromTime = t | ||
| hasFrom = true | ||
| } | ||
| if toDate != "" { | ||
| t, err := time.Parse("2006-01-02", toDate) | ||
| if err != nil { | ||
| return fmt.Errorf("--to 日期格式错误，需要 YYYY-MM-DD: %w", err) | ||
| } | ||
| // 将结束日期设置为当天结束 (23:59:59) | ||
| toTime = t.Add(24*time.Hour - time.Second) | ||
| hasTo = true |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What timezone does time.Parse use in Go when format has no timezone?
💡 Result:
In Go, when you use the time.Parse function and the format string does not contain timezone information, the function interprets the time as UTC [1][2]. If you need to parse a time string as being in a specific location (such as the local system timezone) rather than UTC, you should use the time.ParseInLocation function instead [1][2]. This function allows you to specify a *time.Location object to apply when no timezone information is present in the input string [1][2].
Citations:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ls -la
rg -n "fromDate|toDate|fromTime|toTime|hasFrom|hasTo|PublishTime|After\\(toTime\\)|Before\\(toTime\\)" -S afdian/album/album.go
nl -ba afdian/album/album.go | sed -n '1,220p'Repository: PhiFever/AfdianToMarkdown
Length of output: 2467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '35,130p' afdian/album/album.go
rg -n "type .*PublishTime|PublishTime" -S afdian/album afdian | head -n 200
rg -n "PublishTime.*time\\.Parse|PublishTime.*Parse|time\\.Parse\\(" -S afdian | head -n 200Repository: PhiFever/AfdianToMarkdown
Length of output: 4482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' afdian/parser.go
sed -n '1,120p' afdian/model.goRepository: PhiFever/AfdianToMarkdown
Length of output: 9395
🌐 Web query:
In Go, what timezone/location does time.Unix return (UTC or Local)?
💡 Result:
In Go, the time.Unix function returns a time.Time value set to the local timezone [1][2]. While the function takes seconds and nanoseconds relative to UTC [1][2], the resulting time.Time object uses the local timezone configuration of the system it is running on [1][2]. You can convert the resulting time to UTC or another location using the.UTC or.In methods, respectively [3][1]. Note that the method t.Unix (called on a time.Time object) performs the inverse operation: it returns the number of seconds elapsed since January 1, 1970 UTC, and its result is independent of the location associated with the time.Time value [3].
Citations:
- 1: https://pkg.go.dev/time
- 2: https://pkg.go.dev/time@go1.26.3
- 3: https://github.com/golang/go/blob/master/src/time/time.go
Consider timezone-safe date filtering (parse CLI dates in local time)
time.Parse("2006-01-02", ...) treats timezone-less dates as UTC, while --from/--to are typically intended as calendar dates in the user’s local timezone. This can cause posts near midnight to be included/excluded as the “wrong” day. post.PublishTime is derived from a Unix timestamp (time.Unix(..., 0)), so the main mismatch is the boundary dates being in UTC, not PublishTime carrying a different location. The sub-second filtering concern is unlikely here since publish_time is handled as whole seconds.
Suggested improvement
if fromDate != "" {
- t, err := time.Parse("2006-01-02", fromDate)
+ t, err := time.ParseInLocation("2006-01-02", fromDate, time.Local)
if err != nil {
return fmt.Errorf("--from 日期格式错误，需要 YYYY-MM-DD: %w", err)
}
fromTime = t
hasFrom = true
}
if toDate != "" {
- t, err := time.Parse("2006-01-02", toDate)
+ t, err := time.ParseInLocation("2006-01-02", toDate, time.Local)
if err != nil {
return fmt.Errorf("--to 日期格式错误，需要 YYYY-MM-DD: %w", err)
}
- // 将结束日期设置为当天结束 (23:59:59)
- toTime = t.Add(24*time.Hour - time.Second)
+ // 将结束日期设置为次日开始，比较时使用 Before
+ toTime = t.Add(24 * time.Hour)
hasTo = true
}Then update the filter check at line 107:
-if hasTo && post.PublishTime.After(toTime) {
+if hasTo && !post.PublishTime.Before(toTime) {🤖 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 `@afdian/album/album.go` around lines 45 - 60, The date parsing should be
timezone-aware: parse --from/--to with time.ParseInLocation("2006-01-02", ...,
time.Local) so the dates represent local calendar days (adjust fromTime/toTime
accordingly, e.g., toTime = t.Add(24*time.Hour - time.Second) in that same
location), and when filtering compare post.PublishTime converted to the same
location (post.PublishTime.In(time.Local)) against fromTime/toTime; update the
parsing block that sets fromTime/toTime (variables fromDate, toDate, hasFrom,
hasTo) and the filter check that uses post.PublishTime (around the existing
filter at the comment referring to line ~107) to use these timezone-consistent
values.
| if cfg.MediaOnly { | ||
| // 媒体专用模式：只下载音频/视频，不保存帖子内容 | ||
| _, audio, video, err := afdian.GetPostContent(cfg, article.Url, authToken, converter) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| _, err = downloadMedia(filePath, article.Name, audio, "audio", true) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| _, err = downloadMedia(filePath, article.Name, video, "video", true) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| slog.Info("媒体下载完成（媒体专用模式）", "article", article.Name) | ||
| return false, nil | ||
| } |
There was a problem hiding this comment.
Media-only path currently bypasses idempotency and skip accounting.
This branch skips the normal existence gate and always downloads media, then always returns skipped=false. In repeated runs, that can re-download already-present media and distort skipped-existing metrics.
Consider adding per-media existence checks (or a media-specific skip gate) before downloadMedia, and return skipped=true when nothing new is downloaded.
🤖 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 `@storage/writer.go` around lines 21 - 37, The MediaOnly branch (guarded by
cfg.MediaOnly) always calls afdian.GetPostContent and downloadMedia for
audio/video and returns skipped=false, which bypasses the usual existence/skip
logic; update this branch to check per-media existence before downloading (e.g.,
test audio/video presence via the same existence gate used elsewhere or a
media-specific helper) and only call downloadMedia for missing items,
aggregating whether any new media was saved, then return skipped=true when no
media was downloaded; keep references to cfg.MediaOnly, afdian.GetPostContent,
and downloadMedia when implementing the existence checks and the final skipped
boolean.
2 participants
新增功能
1. 专辑时间范围过滤
album子命令新增--from和--to参数，支持只下载指定时间段内的文章：YYYY-MM-DD--from设 00:00:00，--to设 23:59:59）2. 媒体专用模式
新增全局参数
--media_only，仅下载帖子中的音频/视频文件，不保存帖子内容、图片、评论：afdian-dl album -u https://afdian.com/album/xxx --media_only # 支持组合使用 afdian-dl album -u https://afdian.com/album/xxx --from 2024-01-01 --to 2024-12-31 --media_only--download_media）.md文件向后兼容
albums（下载所有作品集）、motions、shop、update命令不受影响改动的文件
config/config.goMediaOnly字段main.go--media-only全局参数afdian/album/album.gostorage/writer.goSavePostIfNotExist中 media-only 分支Summary by CodeRabbit
--fromand--toCLI flags (format: YYYY-MM-DD)--media_onlyflag to download only audio and video files