Merged
Conversation
Contributor
WalkthroughAdds a Rich log renderer variant that disables table expansion when Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
doc/asciicasts/freeze.cast (1)
37-37: Normalize recording environment to avoid personal/localized metadata in docs casts.This cast now embeds host-specific values (
/home/ben/...,ben ben,22 mrt), which makes docs diffs noisy and leaks recorder identity. Please regenerate casts with a fixed env (e.g., stableHOME/USER,LC_ALL=C,TZ=UTC) so outputs stay deterministic.Also applies to: 64-64, 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/asciicasts/freeze.cast` at line 37, This cast embeds host-specific metadata (e.g., "/home/ben/Programming/dfetch/doc/generate-casts/freeze", "ben ben", "22 mrt") making diffs noisy; regenerate the cast with deterministic environment variables (set LC_ALL=C, TZ=UTC and fixed HOME/USER values such as HOME=/home/docs_user and USER=docs_user) and re-record so those strings are replaced with stable values; update freeze.cast (and the other occurrences noted around the same cast) to reflect the regenerated output so the personal/localized values are removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/asciicasts/check-ci.cast`:
- Line 98: The unfetched-project diagnostic description string in reporter.py
has a missing closing single quote in the command example; locate the
description template (the unfetched-project/long_description template variable
used to build the diagnostic message) and change "fetch it with 'dfetch update
{project.name}. " to "fetch it with 'dfetch update {project.name}'." so the
generated JSON contains the properly quoted command example and the output is
valid.
In `@doc/asciicasts/diff.cast`:
- Line 14: The asciicast frames currently contain maintainer-local usernames and
paths (e.g., the "ben" owner in lines showing "drwxr-xr-x 3 ben ben ..." and
embedded home/prompt traces), so re-record the affected casts (including the
frames around the shown diff and the ones at the other reported locations) using
a neutral prompt/home (no personal username or full home paths) and replace any
remaining recorder-local identifiers with generic placeholders; ensure filenames
like "cpputest", "dfetch.yaml", "jsmn" remain but owner and path fields are
scrubbed or anonymized before committing.
In `@doc/asciicasts/environment.cast`:
- Around line 21-22: The cast's http.client section contains placeholder lines
("<intro stuff goes here>", "<other stuff, too>") and an entire HTTPConnection
state-machine dump that overwhelms the dfetch environment demo; remove the
placeholder/debug lines and replace the large state-machine dump with a short
one-line summary or a compact excerpt, or move the full diagram into a separate
detailed doc, so the example output for "http.client" remains concise and
focused while keeping the demo readable (search for the "http.client :"
header and the placeholder strings to find where to edit).
In `@doc/asciicasts/sbom.cast`:
- Around line 1-10: The asciicast recording ends prematurely right after the
typed input "ls -l" and is missing the final newline, the command output, and
the "dfetch sbom" invocation; fix it by replacing or extending the JSON event
stream in sbom.cast (the {"version":2,...} header/event list) so that there is
an event for the carriage return after "ls -l", events emitting the actual ls
output, and subsequent events showing the "dfetch sbom" command and its output
(including exits), ensuring the timeline/timestamps are consistent and the
terminal control sequences match existing entries.
---
Nitpick comments:
In `@doc/asciicasts/freeze.cast`:
- Line 37: This cast embeds host-specific metadata (e.g.,
"/home/ben/Programming/dfetch/doc/generate-casts/freeze", "ben ben", "22 mrt")
making diffs noisy; regenerate the cast with deterministic environment variables
(set LC_ALL=C, TZ=UTC and fixed HOME/USER values such as HOME=/home/docs_user
and USER=docs_user) and re-record so those strings are replaced with stable
values; update freeze.cast (and the other occurrences noted around the same
cast) to reflect the regenerated output so the personal/localized values are
removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 26904bc1-f7d8-4ba6-adcc-801b883e28b6
📒 Files selected for processing (16)
dfetch/log.pydoc/asciicasts/basic.castdoc/asciicasts/check-ci.castdoc/asciicasts/check.castdoc/asciicasts/diff.castdoc/asciicasts/environment.castdoc/asciicasts/format-patch.castdoc/asciicasts/freeze.castdoc/asciicasts/import.castdoc/asciicasts/init.castdoc/asciicasts/report.castdoc/asciicasts/sbom.castdoc/asciicasts/update-patch.castdoc/asciicasts/update.castdoc/asciicasts/validate.castdoc/conf.py
spoorcc
force-pushed
the
fix-newline-asciicast
branch
from
f23d9fe to
40d0391
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
doc/asciicasts/report.cast (1)
1-1: Consider normalizing volatile timestamps in generated cast fixtures.
timestampand embedded date/time fields will keep creating unrelated doc diffs on each regeneration. If you want cleaner PRs, consider post-processing casts to scrub or freeze time-dependent values.Also applies to: 13-13, 33-33, 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/asciicasts/report.cast` at line 1, The generated cast fixture contains volatile time fields (the top-level "timestamp" and other embedded date/time values) that cause unrelated diffs on regen; update the post-generation step or the fixture itself to normalize or scrub those values (e.g., replace "timestamp" and any date/time fields in the JSON with a fixed sentinel or remove them) so the file doc/asciicasts/report.cast yields stable diffs across runs—locate the "timestamp" key and any nested date/time entries and apply the normalization/freeze there.doc/asciicasts/sbom.cast (1)
50-50: Trailing path output may be an unintended artifact.Line 50 outputs
/workspaces/dfetch/doc/generate-castswithout a shell prompt, which appears to be an internal development path (possibly from$PWDin the prompt or echoed by a script). If unintentional, this could be trimmed or the recording stopped earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/asciicasts/sbom.cast` at line 50, The asciicast contains a stray output event printing "/workspaces/dfetch/doc/generate-casts\r\n" (the entry [12.842484, "o", "/workspaces/dfetch/doc/generate-casts\r\n"]); remove that event from the sbom.cast recording or update the recording script/command (e.g., stop echoing $PWD or suppress the prompt output) so the trailing path is not emitted in future casts, ensuring the asciicast no longer contains the unwanted path string.doc/asciicasts/format-patch.cast (1)
1-1: Consider sanitizing volatile timestamps to reduce doc artifact churn.The cast still embeds run-specific time data (header timestamp,
ls -lmtimes, patchDate:), which will keep producing noisy diffs on re-record. If reproducibility is a goal, prefer fixed-time fixtures/sanitization in cast generation.Also applies to: 13-13, 120-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/asciicasts/format-patch.cast` at line 1, The cast file embeds run-specific timestamps (the JSON "timestamp" field, `ls -l` modification times, and the patch "Date:" header) which cause churn; update the cast generation or a post-processing sanitization step to replace those with a fixed canonical value (e.g., 0 or a constant ISO date) so the header "timestamp", any `ls -l` mtime strings, and the patch "Date:" lines are normalized; locate these tokens in format-patch.cast (look for the JSON "timestamp" key, occurrences of `ls -l` output, and the patch "Date:" header) and rewrite them to the chosen fixed value before committing the cast.doc/asciicasts/basic.cast (1)
13-14: Avoidls -lin the recorded demo if you want stable casts.These snapshots still bake in environment-specific metadata (
dev dev,Mar 23 06:24, permissions/ACL markers), so future re-records will keep churning even if the newline/layout issue is fixed. If the goal is reproducible docs, prefer a metadata-free listing (ls -1,find, etc.) or normalize ownership/mtime before recording.Also applies to: 159-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@doc/asciicasts/basic.cast` around lines 13 - 14, The recorded cast contains an `ls -l` style directory listing (the lines showing "total 4" and "-rw-rw-rw- 1 dev dev 733 Mar 23 06:24 dfetch.yaml") which bakes in user/mtime/permission metadata; replace that output with a metadata-free listing to make the cast stable — e.g., re-record using `ls -1` or `find . -maxdepth 1 -type f -printf "%f\n"` (or normalize ownership/mtime before recording with `chown`/`touch`) wherever the long listing appears so the cast no longer contains environment-specific strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/asciicasts/basic.cast`:
- Around line 161-167: The cast ends with stray terminal output entries (the
empty styled command sequences and the trailing
"/workspaces/dfetch/doc/generate-casts" output) that should be removed or
explicitly recorded; open doc/asciicasts/basic.cast and remove the final output
events corresponding to the empty ANSI sequences (e.g., the entries around
timestamps 30.213341–30.576462 and the final line showing
"/workspaces/dfetch/doc/generate-casts"), or alternatively record the actual
command that produced that path so the output is intentional—ensure the file no
longer contains the stray styled command event followed by the unreferenced
working-directory path.
In `@doc/asciicasts/format-patch.cast`:
- Around line 121-127: The cast tail contains only ANSI escape fragments
(entries like [24.178781, "o", "\u001b["], [24.360898, "o", "1m"], [24.451026,
"o", "\u001b["], [24.541173, "o", "0m"]) immediately before the final output
line "/workspaces/dfetch/doc/generate-casts" and CRLF, so regenerate or edit the
cast so the echoed command text is captured instead of just style codes; re-run
the recording of the final command (or insert the missing printed command
string) so the sequence includes the actual command echo (e.g., the command line
that produced "/workspaces/dfetch/doc/generate-casts") followed by the escape
sequences and the CRLF, ensuring the entries around the timestamps that
currently contain only "\u001b[" and "1m"/"0m" are replaced with the real echoed
command text.
---
Nitpick comments:
In `@doc/asciicasts/basic.cast`:
- Around line 13-14: The recorded cast contains an `ls -l` style directory
listing (the lines showing "total 4" and "-rw-rw-rw- 1 dev dev 733 Mar 23 06:24
dfetch.yaml") which bakes in user/mtime/permission metadata; replace that output
with a metadata-free listing to make the cast stable — e.g., re-record using `ls
-1` or `find . -maxdepth 1 -type f -printf "%f\n"` (or normalize ownership/mtime
before recording with `chown`/`touch`) wherever the long listing appears so the
cast no longer contains environment-specific strings.
In `@doc/asciicasts/format-patch.cast`:
- Line 1: The cast file embeds run-specific timestamps (the JSON "timestamp"
field, `ls -l` modification times, and the patch "Date:" header) which cause
churn; update the cast generation or a post-processing sanitization step to
replace those with a fixed canonical value (e.g., 0 or a constant ISO date) so
the header "timestamp", any `ls -l` mtime strings, and the patch "Date:" lines
are normalized; locate these tokens in format-patch.cast (look for the JSON
"timestamp" key, occurrences of `ls -l` output, and the patch "Date:" header)
and rewrite them to the chosen fixed value before committing the cast.
In `@doc/asciicasts/report.cast`:
- Line 1: The generated cast fixture contains volatile time fields (the
top-level "timestamp" and other embedded date/time values) that cause unrelated
diffs on regen; update the post-generation step or the fixture itself to
normalize or scrub those values (e.g., replace "timestamp" and any date/time
fields in the JSON with a fixed sentinel or remove them) so the file
doc/asciicasts/report.cast yields stable diffs across runs—locate the
"timestamp" key and any nested date/time entries and apply the
normalization/freeze there.
In `@doc/asciicasts/sbom.cast`:
- Line 50: The asciicast contains a stray output event printing
"/workspaces/dfetch/doc/generate-casts\r\n" (the entry [12.842484, "o",
"/workspaces/dfetch/doc/generate-casts\r\n"]); remove that event from the
sbom.cast recording or update the recording script/command (e.g., stop echoing
$PWD or suppress the prompt output) so the trailing path is not emitted in
future casts, ensuring the asciicast no longer contains the unwanted path
string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a06a1cc-f5ab-42f1-bf07-4b967dbb4665
📒 Files selected for processing (20)
CHANGELOG.rstdfetch/project/archivesubproject.pydfetch/reporting/check/reporter.pydoc/asciicasts/basic.castdoc/asciicasts/check-ci.castdoc/asciicasts/check.castdoc/asciicasts/diff.castdoc/asciicasts/environment.castdoc/asciicasts/format-patch.castdoc/asciicasts/freeze.castdoc/asciicasts/import.castdoc/asciicasts/init.castdoc/asciicasts/report.castdoc/asciicasts/sbom.castdoc/asciicasts/update-patch.castdoc/asciicasts/update.castdoc/asciicasts/validate.castdoc/generate-casts/generate-casts.shfeatures/check-report-code-climate.featurefeatures/check-report-jenkins.feature
✅ Files skipped from review due to trivial changes (9)
- dfetch/reporting/check/reporter.py
- dfetch/project/archivesubproject.py
- CHANGELOG.rst
- doc/generate-casts/generate-casts.sh
- features/check-report-jenkins.feature
- doc/asciicasts/validate.cast
- features/check-report-code-climate.feature
- doc/asciicasts/import.cast
- doc/asciicasts/init.cast
🚧 Files skipped from review as they are similar to previous changes (6)
- doc/asciicasts/check.cast
- doc/asciicasts/update-patch.cast
- doc/asciicasts/update.cast
- doc/asciicasts/environment.cast
- doc/asciicasts/diff.cast
- doc/asciicasts/check-ci.cast
Comment on lines
+161
to
+167
| [30.213341, "o", "$ "] | ||
| [30.215274, "o", "\u001b["] | ||
| [30.395592, "o", "1m"] | ||
| [30.485702, "o", "\u001b["] | ||
| [30.575831, "o", "0m"] | ||
| [30.576462, "o", "\r\n"] | ||
| [30.579282, "o", "/workspaces/dfetch/doc/generate-casts\r\n"] |
Contributor
There was a problem hiding this comment.
Trim the stray working-directory output from the end of the cast.
The recording ends with an empty styled command and then /workspaces/dfetch/doc/generate-casts, but there is no visible input that produces that path. This looks like cleanup output leaking into the demo and will render as unrelated noise.
✂️ Proposed trim
[30.213341, "o", "$ "]
-[30.215274, "o", "\u001b["]
-[30.395592, "o", "1m"]
-[30.485702, "o", "\u001b["]
-[30.575831, "o", "0m"]
-[30.576462, "o", "\r\n"]
-[30.579282, "o", "/workspaces/dfetch/doc/generate-casts\r\n"]If that path is meant to be shown, the corresponding command should be recorded explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/asciicasts/basic.cast` around lines 161 - 167, The cast ends with stray
terminal output entries (the empty styled command sequences and the trailing
"/workspaces/dfetch/doc/generate-casts" output) that should be removed or
explicitly recorded; open doc/asciicasts/basic.cast and remove the final output
events corresponding to the empty ANSI sequences (e.g., the entries around
timestamps 30.213341–30.576462 and the final line showing
"/workspaces/dfetch/doc/generate-casts"), or alternatively record the actual
command that produced that path so the output is intentional—ensure the file no
longer contains the stray styled command event followed by the unreferenced
working-directory path.
Comment on lines
+121
to
+127
| [24.178781, "o", "$ "] | ||
| [24.180615, "o", "\u001b["] | ||
| [24.360898, "o", "1m"] | ||
| [24.451026, "o", "\u001b["] | ||
| [24.541173, "o", "0m"] | ||
| [24.541658, "o", "\r\n"] | ||
| [24.54463, "o", "/workspaces/dfetch/doc/generate-casts\r\n"] |
Contributor
There was a problem hiding this comment.
Regenerate the tail of the cast; output appears without an echoed command.
Line 127 prints /workspaces/dfetch/doc/generate-casts, but Lines 121–126 contain only style escape sequences and no command text. This looks like an incomplete capture at the end and makes the recording ambiguous.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@doc/asciicasts/format-patch.cast` around lines 121 - 127, The cast tail
contains only ANSI escape fragments (entries like [24.178781, "o", "\u001b["],
[24.360898, "o", "1m"], [24.451026, "o", "\u001b["], [24.541173, "o", "0m"])
immediately before the final output line "/workspaces/dfetch/doc/generate-casts"
and CRLF, so regenerate or edit the cast so the echoed command text is captured
instead of just style codes; re-run the recording of the final command (or
insert the missing printed command string) so the sequence includes the actual
command echo (e.g., the command line that produced
"/workspaces/dfetch/doc/generate-casts") followed by the escape sequences and
the CRLF, ensuring the entries around the timestamps that currently contain only
"\u001b[" and "1m"/"0m" are replaced with the real echoed command text.
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
Bug Fixes
Documentation