From 3eb6a8182e947dba0ffd671957972312112c9b70 Mon Sep 17 00:00:00 2001 From: Mark Pitman Date: Tue, 13 Oct 2026 21:24:53 -0700 Subject: [PATCH] Fix CLI argument handling and exit codes Flags after the filename were silently ignored because flag.Parse stops at the first positional argument. Parse repeatedly so flags work on either side of the filename, and honor -- as the end of flags. Reject more than one filename instead of ignoring the extras. Explicit -h and -help now print usage to stdout and exit 0. A missing filename still prints usage to stderr and exits 1. Reading from - now fails with a message when standard input is a terminal, instead of blocking. Closes #67 --- CLAUDE.md | 2 +- main.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++----- main_test.go | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ffe104c..6a15a87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,7 +31,7 @@ Requires Go (see `go.mod`/`mise.toml` for version) and, for packaging, `just`, ` Everything happens in `main()` in `main.go`, in a straight-line pipeline: -1. **Flag parsing** — `-o`, `-v/-version`, `-h/-help`, `-b/-bare`. First positional arg is the input markdown file; a single `-` reads the markdown from stdin instead, in which case `baseDir` for image resolution is the working directory rather than the input file's directory. +1. **Flag parsing** (`parseFlags`) — `-o`, `-v/-version`, `-h/-help` (prints usage to stdout, exits 0), `-b/-bare`. Flags work before or after the filename (`--` ends flag parsing). Exactly one positional arg is accepted: the input markdown file. A single `-` reads the markdown from stdin instead (and fails if stdin is a terminal), in which case `baseDir` for image resolution is the working directory rather than the input file's directory. 2. **Image inlining** (`newMarkdown` → `imageTransformer` + `htmlImageRenderer` → `embedImage`/`imageToDataURI`) — rewrites relative image references into base64 `data:` URIs so the output HTML is fully self-contained/offline-viewable. Markdown images (inline and reference-style) are rewritten by an AST transformer, so images inside code spans and code blocks are left alone. Raw `` tags (`src` and `srcset`) are rewritten by a custom renderer for `HTMLBlock`/`RawHTML` nodes. `embedImage` strips query/fragment and decodes percent-encoding. `imageToDataURI` only reads files with an allowlisted image extension (`getMimeType`), resolves symlinks before checking the path (at most 3 `..` levels above the base directory), accepts only regular files, and caps images at 10MB. 3. **Markdown parsing** via Goldmark (built by `newMarkdown`), configured with: - `extension.GFM` (tables, task lists, strikethrough, etc.) diff --git a/main.go b/main.go index f089a79..3b48418 100644 --- a/main.go +++ b/main.go @@ -65,28 +65,46 @@ func main() { flag.BoolVar(helpPtr, "h", false, "Prints mdview help message.") flag.BoolVar(barePtr, "b", false, "Bare HTML with no style applied.") - flag.Parse() - inputFilename := flag.Arg(0) + flag.Usage = func() { printUsage(os.Stderr) } + args, err := parseFlags(flag.CommandLine, os.Args[1:]) + if err != nil { + os.Exit(2) + } if *versionPtr { fmt.Println(appVersion) os.Exit(0) } - if inputFilename == "" || *helpPtr { - os.Stderr.WriteString("Usage:\nmdview [options] \nFormats markdown and launches it in a browser.\nUse - as the filename to read markdown from standard input.\nIf the environment variable MDVIEW_DIR is set, the temporary file will be written there.\n") - flag.PrintDefaults() + if *helpPtr { + printUsage(os.Stdout) + os.Exit(0) + } + + if len(args) == 0 { + printUsage(os.Stderr) os.Exit(1) } + if len(args) > 1 { + fmt.Fprintf(os.Stderr, "mdview accepts one filename, but received %d: %s\n\n", len(args), strings.Join(args, " ")) + printUsage(os.Stderr) + os.Exit(1) + } + + inputFilename := args[0] + // A filename of - means read the markdown from standard input. Relative image // paths are then resolved against the working directory, as there is no input // file to anchor them to. var dat []byte - var err error baseDir := "." if inputFilename == "-" { + if stdinIsTerminal() { + fmt.Fprintln(os.Stderr, "mdview: no markdown on standard input. Pipe markdown into mdview or pass a filename.") + os.Exit(1) + } dat, err = io.ReadAll(os.Stdin) check(err) } else { @@ -137,6 +155,40 @@ func main() { check(err) } +// printUsage writes the usage text and the option defaults to w +func printUsage(w io.Writer) { + fmt.Fprint(w, "Usage:\nmdview [options] \nFormats markdown and launches it in a browser.\nUse - as the filename to read markdown from standard input.\nIf the environment variable MDVIEW_DIR is set, the temporary file will be written there.\n") + flag.CommandLine.SetOutput(w) + flag.PrintDefaults() +} + +// parseFlags parses args and returns the positional arguments. Unlike FlagSet.Parse +// alone, it accepts flags before, between and after the positional arguments. +// Everything after a -- argument is positional. +func parseFlags(fs *flag.FlagSet, args []string) ([]string, error) { + var positional []string + for { + if err := fs.Parse(args); err != nil { + return nil, err + } + consumed := len(args) - fs.NArg() + if consumed > 0 && args[consumed-1] == "--" { + return append(positional, fs.Args()...), nil + } + if fs.NArg() == 0 { + return positional, nil + } + positional = append(positional, fs.Arg(0)) + args = fs.Args()[1:] + } +} + +// stdinIsTerminal reports whether standard input is an interactive terminal +func stdinIsTerminal() bool { + info, err := os.Stdin.Stat() + return err == nil && info.Mode()&os.ModeCharDevice != 0 +} + func tempFileName(prefix, suffix string) string { randBytes := make([]byte, 16) rand.Read(randBytes) diff --git a/main_test.go b/main_test.go index 4228e7c..4d4c620 100644 --- a/main_test.go +++ b/main_test.go @@ -2,10 +2,12 @@ package main import ( "bytes" + "flag" "fmt" stdhtml "html" "os" "path/filepath" + "reflect" "strings" "testing" @@ -234,3 +236,41 @@ func TestTitleIsEscaped(t *testing.T) { }) } } + +func TestParseFlags(t *testing.T) { + tests := []struct { + name string + args []string + wantPos []string + wantOut string + wantBare bool + }{ + {"no arguments", nil, nil, "", false}, + {"flags first", []string{"-o", "x.html", "a.md"}, []string{"a.md"}, "x.html", false}, + {"flags after filename", []string{"a.md", "-o", "x.html", "-b"}, []string{"a.md"}, "x.html", true}, + {"flags on both sides", []string{"-b", "a.md", "-o", "x.html"}, []string{"a.md"}, "x.html", true}, + {"stdin marker", []string{"-", "-o", "x.html"}, []string{"-"}, "x.html", false}, + {"double dash", []string{"-b", "--", "-weird.md"}, []string{"-weird.md"}, "", true}, + {"flags after double dash are positional", []string{"--", "a.md", "-b"}, []string{"a.md", "-b"}, "", false}, + {"several files", []string{"a.md", "b.md"}, []string{"a.md", "b.md"}, "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := flag.NewFlagSet("mdview", flag.ContinueOnError) + out := fs.String("o", "", "") + bare := fs.Bool("b", false, "") + + got, err := parseFlags(fs, tt.args) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, tt.wantPos) { + t.Errorf("positional = %q, want %q", got, tt.wantPos) + } + if *out != tt.wantOut || *bare != tt.wantBare { + t.Errorf("o = %q, b = %v; want o = %q, b = %v", *out, *bare, tt.wantOut, tt.wantBare) + } + }) + } +}