Repository navigation
fix(launcher): refuse a flag or empty value after a value-taking option - #44
Merged
Merged
Conversation
--image, --port and --repo-base-url took "${2:?...}", which rejects only a
missing value. "dmf-init up --repo-base-url --tls" silently stored "--tls"
as the base URL and left TLS off.
Add need_value, which rejects an empty value or one starting with "--" and
names the option. The three options call it before assigning.
The launcher test sources the script, stubs cmd_up and checks that each
option rejects a following flag and an empty value, and that valid values
still parse. The flag cases fail on the old parser.
Refs dmfdeploy/dmfdeploy#160
Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
… terminal The launcher opens the synthesised URL with open or xdg-open when either is on PATH. The harness shimmed docker and curl but not the openers, so a run that reached that step started the real browser. Add no-op open and xdg-open stubs that record their argument next to the fake docker and curl. Two new checks confirm the open step called the stub with the synthesised URL and that both openers resolve to the shim. With the stubs removed and a logging open on PATH, the checks fail and the logging open receives the fixture URLs. Refs dmfdeploy/dmfdeploy#160 Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
lkirc
requested changes
Oct 21, 2026
lkirc
left a comment
lkirc
left a comment
There was a problem hiding this comment.
bin/dmf-init:391 only rejects values beginning with --. The documented short flag -h is still accepted as the value of --image/--port/--repo-base-url (for example, dmf-init up --image -h reaches cmd_up) instead of reporting that the prior option needs a value. Reject any option-looking value (and add a -h regression case).
need_value only rejected values that began with --, so the documented short flag -h was accepted as the value of --image, --port and --repo-base-url. 'dmf-init up --image -h' reached cmd_up instead of reporting that --image needs a value. need_value now rejects any value that starts with a dash. The launcher test gains -h cases next to the -- cases: nonzero exit, the option is named, and cmd_up is not reached. Before the fix the new cases failed (passed=64 failed=9); after it the suite passes (passed=73 failed=0). The harness stubs open and xdg-open. Signed-off-by: znerol2 <6800371+znerol2@users.noreply.github.com>
lkirc
approved these changes
Oct 21, 2026
lkirc
left a comment
lkirc
left a comment
There was a problem hiding this comment.
Static review complete: the prior short-flag concern is resolved; the value guard and focused regression coverage look correct. Required checks are green.
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.
Summary
The launcher's value-taking options (
--image,--port,--repo-base-url) accepted the next argument as their value even when it was another flag, because${2:?...}rejects only a missing or empty value.dmf-init up --repo-base-url --tlstook--tlsas the URL, and--image --port 8080failed with "unknown option '8080'" instead of blaming--image.A small
need_valuehelper now refuses an empty value or one that starts with--, naming the option. Nothing else in the parser changed. This is an install-path bug, inside the v0.1 scope for the installer.Verification
test/launcher/test-dmf-init.shgained an "option values" block that sources the launcher, stubscmd_upand callsmain, so it needs no container runtime. Before the fix: 51 passed, 10 failed (a following flag reachedcmd_upfor each option;--image --port 8080blamed the wrong option). After: 61 passed, 0 failed. Removing the--*case from the helper fails the harness again.shellcheck bin/dmf-init: clean before and after.bin/dmf-init --helpunchanged.Closes dmfdeploy/dmfdeploy#160