Repository navigation
Accept images and image folders in the CLI - #26
Merged
Merged
Conversation
vcs only spoke video, while the library had grown an image pipeline. It now takes folders and image files too, detected from the input itself rather than through a new verb or a mode flag: vcs holiday/ -> holiday.png vcs holiday/ --fit cover -c 5 -r 4 vcs holiday/*.jpg -o summer.jpg -f jpg vcs clip.mp4 holiday/ -> two sheets Detection is Directory.Exists, then a known image extension, then video, so anything that resolved to video before still does - unusual containers included. The awkward part is that the positional argument means "one input, one sheet" for video, but N images make one sheet. InputResolver owns that asymmetry: a video and a directory each get their own sheet, while loose image files coalesce into a single group that takes the position of the first one. It also expands wildcards itself, since cmd and PowerShell hand them through verbatim, so vcs holiday/*.jpg behaves the same on every platform. --recursive, --fit and --all mirror the library knobs, with a matching [image] section in the TOML config. Everything else already applied to both media; --timestamp is described as the caption overlay now, since for images it carries the file name. Options that cannot apply to the given inputs are rejected rather than silently doing nothing, which is the difference between a typo and a sheet that quietly ignored what you asked for. A run that mixes both kinds is exempt: there, each option applies to the inputs it belongs to, so vcs clip.mp4 holiday/ -i 2m is fine. Verified against the real binary: folder, glob, mixed input with default output names, both misapplied-option errors with their exit codes, video unchanged, --all vs sampling, --fit cover, and the [image] config section round-tripping through Tomlyn. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BuildContactSheet_ReportsProgressOncePerImage failed on a loaded Ubuntu runner with 3 of 4 reports. The test used Progress<T>, which posts to the synchronization context rather than invoking inline, so the reports could still be in flight when the build returned; it then polled for them with a five second deadline, which is a race rather than a wait. That asynchrony belongs to Progress<T>, not to the library, so the test now passes a synchronous IProgress recorder. Every report has landed by the time the awaited call returns, and the assertion can be the exact sequence - 0.25, 0.5, 0.75, 1.0 - instead of a count and the last value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
vcsonly spoke video while the library had grown an image pipeline. It now takes folders andimage files too, detected from the input itself rather than through a new verb or a mode flag.
Detection order is
Directory.Exists, then a known image extension, then video — so anythingthat resolved to video before still does, unusual containers included.
The awkward part
The positional argument means "one input → one sheet" for video, but N images make one sheet.
InputResolverowns that asymmetry: a video and a directory each get their own sheet, while looseimage files coalesce into a single group that takes the position of the first one.
It also expands wildcards itself, since cmd and PowerShell hand them through verbatim unlike a
POSIX shell — so
vcs holiday/*.jpgbehaves identically on every platform.New options
--recursive,--fit <contain|cover|stretch>and--all, mirroring the library knobs, plus amatching
[image]section in the TOML config. Everything else already applied to both media.--timestampis now described as the caption overlay, since for images it carries the file name.No short alias for
--recursive:-ris already--rows, and-r/-Rdiffering only in caseis a footgun on a tool where both are plausible.
Misapplied options are errors
The difference between a typo and a sheet that quietly ignored what you asked for. Mixed runs are
exempt, since there each option legitimately applies to part of the input.
Structure
InputResolver(detection + grouping + globbing),OptionScopeValidator(scope rules),ImageOptionsFactory(config→CLI layering + collection construction), andSheetRunnerswitchingon the kind.
VideoandImageCollectionalready expose the sameSaveContactSheetAsyncshape,so the runner needed a switch rather than an abstraction over two cases.
Verification
36 new tests (112 total, all passing), clean under
-warnaserror. Beyond the unit tests, runagainst the real binary with a 16-photo folder:
vcs <folder>vcs '03*.jpg'from cmd-style invocationvcs sample.mp4 photos -i 5sample.png+photos.png, interval applied to the video onlyvcs <folder> --interval 3m30vcs sample.mp4 --fit covervcs sample.mp4--allvs sampling at-c 4 -r 2--fit cover[image]config via--configSupersedes the "CLI covers video only for now" line from #25.
🤖 Generated with Claude Code