feat(cli): render multiple source files in one session#181
Closed
airvzxf wants to merge 2 commits into
Closed
Conversation
mdfried currently only accepts a single positional [SOURCE]. When a
shell glob like `mdfried **/PRO*.md` expands to multiple paths, clap
rejects the extras as `unexpected argument ...`. This commit makes
mdfried accept one or more file paths, render each in turn, and insert
a single thin line `── File: <name> ──…` between files so the boundary
is visible. The path is the user-supplied path verbatim, preserving
relative-path context from shell globs. Content inside each file is
not modified.
v1 is file paths only: URLs, github repos, and stdin are still single
source (mixed stdin + files is rejected with a friendly usage error).
`--watch` rejects more than one source, since watching multiple files
is out of scope for v1.
Implementation notes:
- New `DocumentSource::MultiFile { entries: Vec<MultiFileEntry> }`
variant carries the pre-loaded text per file, so the worker can
parse each in order without re-reading the filesystem.
- New `SectionContent::FileSeparator { filename }` variant is what the
view layer renders as a single muted line. It uses the same
`\u2500` character bat uses.
- New `Cmd::ParseMulti` command asks the worker to parse a list of
(text, basepath) pairs. The worker extracts `parse_one_file` and
`drain_image_cache` helpers out of the existing `Cmd::Parse` flow
so both arms share the parsing and image-cache code.
- Each section produced by `parse_one_file` in multi-file mode
carries the current file's basepath through `SectionEvent::Image`
to `image_section`, so relative `` references resolve
against the correct file. Single-file behaviour is unchanged because
`image_section` still falls back to `document_source.read()` when
no basepath override is given.
- `Model::parse_multi` sends `Cmd::ParseMulti` to the worker.
- The CLI validates the source list before any I/O: a friendly usage
error is returned for `--watch` + multiple sources, multiple
`-`, mixed stdin + files, and URLs alongside file paths.
A new unit test (`multi_file_renders_with_file_separator`) builds a
Model with a `MultiFile` source, calls `parse_multi` for two
trivial markdown files, and asserts the resulting sections include a
`FileSeparator` for each file with the user-supplied path verbatim,
and that the rendered buffer contains the bat-style rule character
around each filename.
There was a problem hiding this comment.
Pull request overview
Adds first-class multi-source rendering to mdfried, allowing multiple file paths to be rendered sequentially in one session with a bat-style separator line, while preserving per-file relative image resolution.
Changes:
- Extend the source/model/worker pipeline to support
DocumentSource::MultiFileandCmd::ParseMulti, parsing multiple in-memory file entries in order. - Introduce
SectionContent::FileSeparatorand render it as a muted bat-style rule line in the view layer. - Thread per-file basepaths through image post-processing so relative image links resolve against the correct file in multi-file mode.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | CLI now accepts multiple positional sources and performs upfront validation before dispatching multi-file parsing. |
| src/sources.rs | Adds MultiFileEntry and DocumentSource::MultiFile to carry per-file preloaded text/basepaths. |
| src/model.rs | Adds Model::parse_multi and blocks link navigation in multi-file mode. |
| src/worker.rs | Adds Cmd::ParseMulti, refactors shared parse logic (parse_one_file, drain_image_cache), and emits FileSeparator sections. |
| src/worker/sections.rs | Adds iterator support for non-zero starting section IDs to keep section IDs unique across files. |
| src/view.rs | Renders SectionContent::FileSeparator as a single muted rule line containing the filename. |
| src/document.rs | Adds SectionContent::FileSeparator and adds a basepath override to image_section to support per-file relative images. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Four Copilot review comments applied: 1. Gate the URL/github validation on `sources.len() > 1` (and on `is_multi_file`). Single-source URL and `github:owner/repo` invocations must keep flowing through `open_source(...)`; the v1 restriction is only on mixing them with file paths. 2. Move the mixed `-` + file path rejection before any I/O. The previous code printed a friendly error inside the multi-file read loop, which meant a partial file read had already happened by the time the user saw the error. The check now lives next to the other pre-flight validations. 3. Use display width (via `unicode_width::UnicodeWidthStr`) instead of `String::len()` / `String::truncate` in the FileSeparator renderer. `inner_w` is a terminal cell width, so the previous code could panic with 'byte index N is not a char boundary' when a non-ASCII filename (or the `\u2500` rule char) caused the truncation point to land mid-UTF-8 sequence. 4. Drop the `all_text` accumulator in `Cmd::ParseMulti`. The text payload of `Event::ParseDone` is only consumed by `DocumentSource::Stdin::return_text` (which is not used in multi-file mode), so concatenating every input file in memory was wasted work. The empty string is now passed instead.
Owner
|
Not good, and too complex for little benefit even if it wasn't AI generated. It would be a lot better if you would discuss changes before opening these big PRs. |
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
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.
Closes #180
What
mdfriednow accepts more than one positional[SOURCE]andrenders them in sequence, separated by a thin bat-style rule line
that includes the next file's name.
mdfried a.md b.md c.mdworksend-to-end.
renders
a.md, then a single muted separator line (\u2500\u2500\u2500\u2500\u2500 File: b.md \u2500\u2500...), thenb.md. Markdown content inside eachfile is not modified.
Why
A shell glob like
mdfried **/PRO*.mdexpands to multiple paths andcurrently fails with clap's generic
unexpected argument ... found.The user has to either quote the glob or loop over
mdfriedmanually. First-class multi-source support makes the common case
("show me all PROPOSAL.md files") one command, matching
bat'sergonomics.
How
DocumentSource::MultiFile { entries: Vec<MultiFileEntry> }variant carries the pre-loaded text per file, so the worker can
parse each in order without re-reading the filesystem.
SectionContent::FileSeparator { filename }variant is whatthe view layer renders as a single muted line, using the same
\u2500characterbatuses.Cmd::ParseMulticommand asks the worker to parse a list of(text, basepath) pairs.
parse_one_fileanddrain_image_cacheare extracted from the existing
Cmd::Parseflow so both armsshare the parsing and image-cache code.
parse_one_filein multi-file modecarries the current file's basepath through
SectionEvent::Imageto
image_section, so relativereferences resolveagainst the correct file. Single-file behaviour is unchanged because
image_sectionstill falls back todocument_source.read()whenno basepath override is given.
Model::parse_multisendsCmd::ParseMultito the worker.error is returned for
--watch+ multiple sources, multiple-,mixed stdin + files, and URLs alongside file paths.
v1 scope (intentional)
github:owner/repo, and-(stdin)remain single-source for v1; mixing them with file paths fails
with a friendly usage error pointing at the limitation.
matching
bat.--watchrejects more than one source.A new unit test
multi_file_renders_with_file_separatorbuilds aModel with a
MultiFilesource, callsparse_multifor twotrivial markdown files, and asserts the resulting sections include a
FileSeparatorfor each file with the user-supplied path verbatim,and that the rendered buffer contains the bat-style rule character
around each filename. The full test suite (
cargo test --bin mdfried)passes (34/34).
cargo clippy --all-targets -- -D warningsandcargo doc --no-depsare clean.cargo build --releaseworks.Test plan for the reviewer
Out of scope (v2)
--watch.:backhistory.deep_fryoverrides.Independent of #178 / #179
This PR does not interact with #178 or #179. When this lands, the
friendly error in #179 becomes unnecessary because the rejection no
longer happens for the file-path case.