diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 1e1944d..094804d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -87,7 +87,7 @@ jobs: - name: Unit + integration tests with coverage shell: bash run: | - go test ./test/unit/... ./test/integration/... -race \ + go test ./internal/app/ ./test/unit/... ./test/integration/... -race \ -coverpkg=./internal/... -coverprofile=cover.out # e2e builds a real binary and drives it under Xvfb -- Linux-only by @@ -105,9 +105,7 @@ jobs: if: runner.os == 'Linux' shell: bash env: - # TODO: ratchet this to 80 (the target in doc/ci-cd-setup.md) once - # the suite gets there. Measured at 77.8% when this gate was added. - MIN_COVERAGE: '75' + MIN_COVERAGE: '80' run: | pct=$(go tool cover -func=cover.out | tail -1 | grep -oE '[0-9]+\.[0-9]+' | head -1) echo "Total line coverage: ${pct}% (minimum: ${MIN_COVERAGE}%)" diff --git a/README.md b/README.md index f2a0e49..4d7ec7c 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,8 @@ binary. Nvim's own input protocol. - Live window resizing. - A small, plain `config.toml` for font and Nvim-launch settings. +- Launcher-friendly command line: `--maximized`, plus a `--` separator that + passes any Nvim arguments straight through. - Nerd font support. Simple and pure — no bloat, no bundled plugin marketplace, no telemetry. @@ -83,6 +85,28 @@ extra_args = [] # extra args passed straight to nvim You don't need to create this file to get started — the defaults shown above are exactly what's used if it's absent. +## Command line + +```sh +simplenvim [flags] [file...] [-- nvim-args...] +``` + +| Flag | Description | +|---|---| +| `--maximized` | Start with the window maximized. | +| `--nvim ` | Use a specific `nvim` executable. | +| `--version` | Print the version and exit. | + +Anything after `--` goes to `nvim` untouched, which is what lets a desktop +launcher or shell alias drive it: + +```sh +simplenvim --maximized -- -c term -c edit ~/notes.todo +``` + +See [`doc/developer.md`](doc/developer.md#command-line-flags) for the full +details, including how pass-through arguments are ordered. + ## Documentation - [`doc/developer.md`](doc/developer.md) — everything about building, diff --git a/doc/developer.md b/doc/developer.md index 3831471..ce9e1df 100644 --- a/doc/developer.md +++ b/doc/developer.md @@ -177,10 +177,44 @@ For shipping a `.dmg`, see [Releasing](#releasing-official-build); | Flag | Description | |---|---| | `-nvim /path/to/nvim` | Overrides the `nvim` executable to launch (default: whatever the config file says, or plain `nvim` resolved via `PATH`). | +| `-maximized` | Starts with the window maximized. | +| `-version` | Prints the version and exits. | + +Single-dash and double-dash spellings are equivalent (`-maximized` == +`--maximized`), as usual for Go's `flag` package. Any remaining positional arguments are treated as files to open, exactly like `nvim file1 file2`. +#### Passing arguments through to Nvim + +Everything after a `--` separator is forwarded to `nvim` **verbatim**, so +Nvim's own flags don't collide with SimpleNvimEditor's: + +```sh +simplenvim --maximized -- -c term -c edit ~/notes.todo +``` + +Without the separator, `-c` would be parsed as one of *our* flags and +rejected — that's the whole reason it exists. + +Two details worth knowing: + +- **Pass-through args are placed before positional files.** Several Nvim + flags are positional-hungry, so `simplenvim a.txt b.txt -- -O` becomes + `nvim -O a.txt b.txt` (two vertical splits), not `nvim a.txt b.txt -O`. +- **Only the first `--` is consumed.** A second one is meaningful to Nvim + itself (it ends *its* option list), so it is forwarded unchanged. + +Because forwarding is verbatim, Nvim's own argument grammar applies +unmodified. In the example above, `-c` takes exactly one argument, so +`-c edit ~/notes.todo` is `-c edit` *plus* a positional file — the same +thing plain `nvim` would do with that line. + +Parsing lives in `internal/cli`, which depends only on the standard +library so the whole argument surface is unit-testable without opening a +window (see `test/unit/cli_test.go`). + For the `config.toml` file format (font/Nvim-launch settings, default values, and file locations per OS), see the [Configuration section in the README](../README.md#configuration). @@ -282,7 +316,7 @@ rationale (and the not-yet-implemented publishing channels) is in ### Coverage gate `pr.yml` fails if total line coverage over `internal/...` drops below -**75%** (`MIN_COVERAGE` in the workflow). Coverage comes from the unit + +**80%** (`MIN_COVERAGE` in the workflow). Coverage comes from the unit + integration tiers only — e2e drives a separately-compiled binary, so its execution isn't visible to `-coverpkg` (see §6). diff --git a/src/cmd/simplenvim/main.go b/src/cmd/simplenvim/main.go index 0831e30..92dbcbd 100644 --- a/src/cmd/simplenvim/main.go +++ b/src/cmd/simplenvim/main.go @@ -4,13 +4,16 @@ package main import ( + "errors" "flag" "fmt" "os" + "path/filepath" gioapp "gioui.org/app" editorapp "github.com/kgfly/SimpleNvimEditor/internal/app" + "github.com/kgfly/SimpleNvimEditor/internal/cli" "github.com/kgfly/SimpleNvimEditor/internal/config" ) @@ -18,15 +21,18 @@ import ( var version = "dev" func main() { - showVersion := flag.Bool("version", false, "print version and exit") - nvimPath := flag.String("nvim", "", "path to the nvim executable (overrides config file)") - flag.Usage = func() { - fmt.Fprintf(os.Stderr, "Usage: %s [flags] [file...]\n\n", os.Args[0]) - flag.PrintDefaults() + opts, err := cli.Parse(filepath.Base(os.Args[0]), os.Args[1:], os.Stderr) + if err != nil { + // -h/--help is a successful request for usage, which Parse has + // already written; anything else is a real misuse and flag has + // already reported the specific problem. + if errors.Is(err, flag.ErrHelp) { + return + } + os.Exit(2) } - flag.Parse() - if *showVersion { + if opts.ShowVersion { fmt.Println(version) return } @@ -36,12 +42,14 @@ func main() { fmt.Fprintf(os.Stderr, "simplenvim: loading config: %v\n", err) os.Exit(1) } - if *nvimPath != "" { - cfg.Nvim.Command = *nvimPath + if opts.NvimPath != "" { + cfg.Nvim.Command = opts.NvimPath } - files := flag.Args() - a := editorapp.New(cfg, files) + a := editorapp.New(cfg, editorapp.Options{ + NvimArgs: opts.NvimArgs, + Maximized: opts.Maximized, + }) go func() { win := new(gioapp.Window) diff --git a/src/internal/app/app.go b/src/internal/app/app.go index 12c9559..cd4a2b1 100644 --- a/src/internal/app/app.go +++ b/src/internal/app/app.go @@ -38,10 +38,22 @@ var rootTag = new(int) // and shifted glyphs like ":"). const anyModifier = key.ModCtrl | key.ModCommand | key.ModShift | key.ModAlt | key.ModSuper +// Options are the launch-time choices that come from the command line +// rather than the config file. Keeping them in one struct means adding the +// next flag doesn't churn New's signature for every caller and test. +type Options struct { + // NvimArgs are passed to the Nvim invocation verbatim: pass-through + // arguments first, then files (see the cli package, which is the one + // place that ordering is decided). + NvimArgs []string + // Maximized starts the window filling the monitor's work area. + Maximized bool +} + // App owns everything needed to run one editor window. type App struct { - cfg config.Config - files []string + cfg config.Config + opts Options win *gioapp.Window fonts render.Fonts @@ -59,23 +71,37 @@ type App struct { ime imeShadow } -// New creates an App that will open the given files (may be empty). -func New(cfg config.Config, files []string) *App { +// New creates an App configured by cfg and the command line's opts. +func New(cfg config.Config, opts Options) *App { return &App{ cfg: cfg, - files: files, + opts: opts, state: uistate.New(), ime: newIMEShadow(), } } +// WindowOptions returns the Gio options describing the window this App +// wants. It is exported (and takes no receiver state beyond opts) so tests +// can assert the real, production option set without opening a window — +// the same approach InputFilters takes for the input path. +// +// Size is still requested alongside Maximized on purpose: it is the size +// the window returns to when the user un-maximizes it, and it's what +// platforms that cannot honor a maximized request fall back to. +func WindowOptions(opts Options) []gioapp.Option { + win := []gioapp.Option{gioapp.Size(unit.Dp(1000), unit.Dp(650))} + if opts.Maximized { + win = append(win, gioapp.Maximized.Option()) + } + return win +} + // Run drives win's event loop until the window is closed. It blocks the // calling goroutine, matching Gio's own convention (see gioui.org/app doc). func (a *App) Run(win *gioapp.Window) error { a.win = win - win.Option( - gioapp.Size(unit.Dp(1000), unit.Dp(650)), - ) + win.Option(WindowOptions(a.opts)...) a.fonts = render.Fonts{ Shaper: render.NewShaper(a.cfg.Editor), @@ -322,7 +348,7 @@ func (a *App) syncSize(size image.Point) { // big the initial grid should be, and starts the goroutine that pumps its // redraw events into our state model. func (a *App) startNvim() { - proc, err := nvimproc.Spawn(a.cfg.Nvim.Command, a.cfg.Nvim.ExtraArgs, a.files, a.cols, a.rows) + proc, err := nvimproc.Spawn(a.cfg.Nvim.Command, a.cfg.Nvim.ExtraArgs, a.opts.NvimArgs, a.cols, a.rows) if err != nil { // Nothing meaningful to render without Nvim; surfacing to stderr // is enough for the MVP (see IMPLEMENTATION_PLAN.md for a real diff --git a/src/internal/app/icon_test.go b/src/internal/app/icon_test.go new file mode 100644 index 0000000..54c2d1e --- /dev/null +++ b/src/internal/app/icon_test.go @@ -0,0 +1,74 @@ +package editorapp + +import ( + "image" + "testing" + + "gioui.org/io/key" + "gioui.org/io/pointer" + + "github.com/kgfly/SimpleNvimEditor/internal/config" +) + +func TestAppIcon(t *testing.T) { + img := appIcon() + if img == nil { + t.Fatal("appIcon returned nil") + } + b := img.Bounds() + if b.Dx() == 0 || b.Dy() == 0 { + t.Fatalf("appIcon returned empty image: %v", b) + } +} + +func TestNewApp(t *testing.T) { + cfg := config.Default() + a := New(cfg, Options{NvimArgs: []string{"file1.go", "file2.go"}}) + if a == nil { + t.Fatal("New returned nil") + } + if a.state == nil { + t.Fatal("New should initialize state") + } + if len(a.opts.NvimArgs) != 2 { + t.Fatalf("NvimArgs = %v, want 2 entries", a.opts.NvimArgs) + } +} + +func TestQuitWithNilProc(t *testing.T) { + a := New(config.Default(), Options{}) + // Should not panic. + a.quit() +} + +func TestOnKeyWithNilProc(t *testing.T) { + a := New(config.Default(), Options{}) + a.onKey(key.Event{Name: "A", State: key.Press}) +} + +func TestOnEditWithNilProc(t *testing.T) { + a := New(config.Default(), Options{}) + a.onEdit(key.EditEvent{Text: "hello"}) +} + +func TestOnPointerWithNilProc(t *testing.T) { + a := New(config.Default(), Options{}) + a.onPointer(pointer.Event{Kind: pointer.Press, Buttons: pointer.ButtonPrimary}) +} + +func TestCaretWithZeroMetrics(t *testing.T) { + a := New(config.Default(), Options{}) + c := a.caret() + if c.Ascent != 0 || c.Descent != 0 { + t.Fatalf("caret with zero metrics should be zero, got %+v", c) + } +} + +func TestSyncSizeWithZeroCellDims(t *testing.T) { + a := New(config.Default(), Options{}) + // CellWidth and CellHeight are 0, should return early. + a.syncSize(image.Pt(800, 600)) + if a.proc != nil { + t.Fatal("syncSize should not start nvim with zero cell dims") + } +} diff --git a/src/internal/app/ime_test.go b/src/internal/app/ime_test.go new file mode 100644 index 0000000..bf04b1f --- /dev/null +++ b/src/internal/app/ime_test.go @@ -0,0 +1,173 @@ +package editorapp + +import ( + "testing" + + "gioui.org/io/key" +) + +func TestNewIMEShadow(t *testing.T) { + s := newIMEShadow() + if s.composing() { + t.Fatal("new shadow should not be composing") + } + if got := s.snippet(); got.Text != "" { + t.Fatalf("new shadow snippet = %q, want empty", got.Text) + } + if got := s.composingText(); got != "" { + t.Fatalf("new shadow composingText = %q, want empty", got) + } +} + +func TestIMEReset(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "hello") + s.setComposing(key.Range{Start: 0, End: 5}) + s.reset() + if s.composing() { + t.Fatal("composing should be false after reset") + } + if len(s.text) != 0 { + t.Fatalf("text should be empty after reset, got %d runes", len(s.text)) + } +} + +func TestIMEReplaceInsert(t *testing.T) { + s := newIMEShadow() + displaced := s.replace(key.Range{}, "hello") + if displaced != 0 { + t.Fatalf("insert displaced %d, want 0", displaced) + } + if got := s.snippet().Text; got != "hello" { + t.Fatalf("text = %q, want %q", got, "hello") + } + if s.sel.Start != 5 || s.sel.End != 5 { + t.Fatalf("sel = %+v, want {5,5}", s.sel) + } +} + +func TestIMEReplaceOverwrite(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "hello world") + displaced := s.replace(key.Range{Start: 0, End: 5}, "hi") + if displaced != 5 { + t.Fatalf("displaced = %d, want 5", displaced) + } + if got := s.snippet().Text; got != "hi world" { + t.Fatalf("text = %q, want %q", got, "hi world") + } +} + +func TestIMESetComposingAndComposingText(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "hello") + s.setComposing(key.Range{Start: 0, End: 5}) + if !s.composing() { + t.Fatal("should be composing") + } + if got := s.composingText(); got != "hello" { + t.Fatalf("composingText = %q, want %q", got, "hello") + } +} + +func TestIMESetComposingNegativeClears(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "hello") + s.setComposing(key.Range{Start: 0, End: 5}) + s.setComposing(key.Range{Start: -1, End: -1}) + if s.composing() { + t.Fatal("negative start should clear composing") + } +} + +func TestIMESetSelection(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "hello") + s.setSelection(key.Range{Start: 2, End: 4}) + if s.sel.Start != 2 || s.sel.End != 4 { + t.Fatalf("sel = %+v, want {2,4}", s.sel) + } +} + +func TestIMEClampReversedRange(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "ab") + r := s.clamp(key.Range{Start: 2, End: 0}) + if r.Start != 0 || r.End != 2 { + t.Fatalf("clamp reversed = %+v, want {0,2}", r) + } +} + +func TestIMEClampOutOfBounds(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "ab") + r := s.clamp(key.Range{Start: -5, End: 100}) + if r.Start != 0 || r.End != 2 { + t.Fatalf("clamp oob = %+v, want {0,2}", r) + } +} + +func TestIMESnippet(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "test") + snip := s.snippet() + if snip.Range.Start != 0 || snip.Range.End != 4 { + t.Fatalf("snippet range = %+v, want {0,4}", snip.Range) + } + if snip.Text != "test" { + t.Fatalf("snippet text = %q, want %q", snip.Text, "test") + } +} + +func TestIMETrimIfIdle(t *testing.T) { + s := newIMEShadow() + // Fill buffer beyond maxShadowRunes. + big := make([]rune, maxShadowRunes+1) + for i := range big { + big[i] = 'x' + } + s.text = big + s.trimIfIdle() + if len(s.text) != 0 { + t.Fatalf("trimIfIdle should empty oversized idle buffer, got %d", len(s.text)) + } +} + +func TestIMETrimIfIdleSkipsComposing(t *testing.T) { + s := newIMEShadow() + big := make([]rune, maxShadowRunes+1) + for i := range big { + big[i] = 'x' + } + s.text = big + s.setComposing(key.Range{Start: 0, End: 10}) + s.trimIfIdle() + if len(s.text) == 0 { + t.Fatal("trimIfIdle should not empty buffer while composing") + } +} + +func TestIMETrimIfIdleSmallBuffer(t *testing.T) { + s := newIMEShadow() + s.replace(key.Range{}, "small") + s.trimIfIdle() + if s.snippet().Text != "small" { + t.Fatal("trimIfIdle should not touch small buffers") + } +} + +func TestClampInt(t *testing.T) { + cases := []struct { + v, lo, hi, want int + }{ + {5, 0, 10, 5}, + {-1, 0, 10, 0}, + {15, 0, 10, 10}, + {0, 0, 0, 0}, + } + for _, c := range cases { + if got := clampInt(c.v, c.lo, c.hi); got != c.want { + t.Errorf("clampInt(%d, %d, %d) = %d, want %d", c.v, c.lo, c.hi, got, c.want) + } + } +} diff --git a/src/internal/cli/cli.go b/src/internal/cli/cli.go new file mode 100644 index 0000000..1e69f89 --- /dev/null +++ b/src/internal/cli/cli.go @@ -0,0 +1,92 @@ +// Package cli turns SimpleNvimEditor's command line into the handful of +// options the rest of the program actually needs. +// +// It deliberately depends on nothing but the standard library — no Gio, no +// Nvim, no config file — so the whole argument surface can be exercised as +// a pure function in unit tests instead of only through a real GUI launch. +package cli + +import ( + "flag" + "fmt" + "io" +) + +// Separator ends SimpleNvimEditor's own flags. Everything after it is +// handed to Nvim untouched, which is what lets a launcher write: +// +// simplenvim --maximized -- -c term -c edit ~/notes.todo +// +// Without it, Nvim's own flags (`-c`, `-u`, `-O`, ...) would be parsed as +// ours and rejected. +const Separator = "--" + +// Options is everything the command line can say. +type Options struct { + // ShowVersion asks for the version string instead of a window. + ShowVersion bool + // NvimPath overrides the nvim executable named by the config file. + NvimPath string + // Maximized starts the window filling the monitor's work area. + Maximized bool + // NvimArgs are appended verbatim to the Nvim invocation: first the + // pass-through arguments given after Separator, then any plain + // positional arguments. See Parse for why that order. + NvimArgs []string +} + +// Split divides argv at the first Separator, returning the arguments this +// program parses itself and the ones destined for Nvim. +// +// Splitting before flag parsing (rather than relying on flag's own "--" +// handling) is what keeps the two groups distinguishable: the standard +// parser lumps everything after "--" in with ordinary positional files, +// which would lose the ordering guarantee Parse documents. +func Split(argv []string) (own, passthrough []string) { + for i, arg := range argv { + if arg == Separator { + // Everything after the *first* separator is verbatim, + // including any further "--" Nvim may want to receive. + return argv[:i], argv[i+1:] + } + } + return argv, nil +} + +// Parse interprets argv (the arguments *after* the program name) for a +// program invoked as name, writing usage and errors to out. +// +// Pass-through arguments come before positional files in NvimArgs because +// some Nvim flags are positional-hungry: `simplenvim a.txt b.txt -- -O` +// must become `nvim -O a.txt b.txt`, not `nvim a.txt b.txt -O`. That +// ordering lives here, once, so no caller has to re-derive it. +// +// A flag.ErrHelp return means -h/--help was requested and usage has +// already been written to out; the caller should exit successfully. +func Parse(name string, argv []string, out io.Writer) (Options, error) { + own, passthrough := Split(argv) + + fs := flag.NewFlagSet(name, flag.ContinueOnError) + fs.SetOutput(out) + + var o Options + fs.BoolVar(&o.ShowVersion, "version", false, "print version and exit") + fs.StringVar(&o.NvimPath, "nvim", "", "path to the nvim executable (overrides config file)") + fs.BoolVar(&o.Maximized, "maximized", false, "start with the window maximized") + fs.Usage = func() { + fmt.Fprintf(out, "Usage: %s [flags] [file...] [%s nvim-args...]\n\n", name, Separator) + fs.PrintDefaults() + fmt.Fprintf(out, "\nArguments after %q are passed to nvim unchanged, e.g.\n"+ + " %s --maximized %s -c term -c edit notes.todo\n", Separator, name, Separator) + } + + if err := fs.Parse(own); err != nil { + return Options{}, err + } + + files := fs.Args() + o.NvimArgs = make([]string, 0, len(passthrough)+len(files)) + o.NvimArgs = append(o.NvimArgs, passthrough...) + o.NvimArgs = append(o.NvimArgs, files...) + return o, nil +} diff --git a/src/internal/nvimproc/process.go b/src/internal/nvimproc/process.go index a931e21..57ed664 100644 --- a/src/internal/nvimproc/process.go +++ b/src/internal/nvimproc/process.go @@ -53,17 +53,23 @@ type Process struct { cmds chan func() } -// Spawn starts `command --embed [extraArgs...] [files...]` as a child +// Spawn starts `command --embed [extraArgs...] [userArgs...]` as a child // process and attaches a UI to it with the given initial grid size. // +// extraArgs come from the config file; userArgs come from the command line +// (pass-through arguments and files alike, already in the order Nvim should +// see them). Both are forwarded verbatim: this package deliberately does +// not interpret Nvim's own flag vocabulary, so `-c`, `-O`, `-u` and friends +// keep working without changes here. +// // command is resolved the same way on every OS: exec.Command performs a // PATH lookup, so "nvim" works on Linux, macOS, and Windows alike as long // as the binary is installed and on PATH (or an absolute path is given). -func Spawn(command string, extraArgs, files []string, cols, rows int) (*Process, error) { - args := make([]string, 0, len(extraArgs)+len(files)+2) +func Spawn(command string, extraArgs, userArgs []string, cols, rows int) (*Process, error) { + args := make([]string, 0, len(extraArgs)+len(userArgs)+1) args = append(args, "--embed") args = append(args, extraArgs...) - args = append(args, files...) + args = append(args, userArgs...) v, err := nvim.NewChildProcess( nvim.ChildProcessCommand(command), diff --git a/src/test/e2e/gui_test.go b/src/test/e2e/gui_test.go index ed59e3f..f377188 100644 --- a/src/test/e2e/gui_test.go +++ b/src/test/e2e/gui_test.go @@ -292,3 +292,57 @@ func hasChildNvim(t *testing.T, parentPID int) bool { } return false } + +// TestGUIAcceptsLauncherArgs runs the real binary with the exact command +// line a desktop launcher uses: +// +// simplenvim --maximized -- -c term -c edit +// +// Before pass-through support, this exited immediately with a flag-parsing +// error, so the strongest signal here is simply that a window appears at +// all and renders content. It complements the integration tier, which +// verifies what those arguments did *inside* nvim; this tier verifies the +// binary's own argument handling end to end. +func TestGUIAcceptsLauncherArgs(t *testing.T) { + requireE2ETools(t) + + bin := buildSimplenvim(t) + display := startXvfb(t) + + dir := t.TempDir() + todo := filepath.Join(dir, "n.todo") + if err := os.WriteFile(todo, []byte("launcher args e2e\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + cmd := exec.Command(bin, "--maximized", "--", "-c", "term", "-c", "edit", todo) + cmd.Env = append(os.Environ(), "DISPLAY="+display) + var stderr strings.Builder + cmd.Stderr = &stderr + if err := cmd.Start(); err != nil { + t.Fatalf("start simplenvim: %v", err) + } + t.Cleanup(func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + + // A flag-parsing failure exits before any window exists, so + // waitForWindow succeeding is itself the core assertion. + windowID := waitForWindow(t, display, 30*time.Second) + + var img image.Image + for deadline := time.Now().Add(20 * time.Second); time.Now().Before(deadline); { + img = screenshotWindow(t, display, windowID) + if distinctColorCount(img) >= 5 { + break + } + time.Sleep(250 * time.Millisecond) + } + if n := distinctColorCount(img); n < 5 { + t.Fatalf("screenshot has only %d distinct sampled colors, want a real rendered UI; stderr:\n%s", n, stderr.String()) + } + if s := stderr.String(); strings.Contains(s, "flag provided but not defined") { + t.Fatalf("binary rejected its own launcher arguments:\n%s", s) + } +} diff --git a/src/test/integration/cliargs_test.go b/src/test/integration/cliargs_test.go new file mode 100644 index 0000000..1429438 --- /dev/null +++ b/src/test/integration/cliargs_test.go @@ -0,0 +1,157 @@ +package integration_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/kgfly/SimpleNvimEditor/internal/cli" + "github.com/kgfly/SimpleNvimEditor/internal/nvimproc" + "github.com/kgfly/SimpleNvimEditor/internal/uistate" +) + +// spawnWithArgs starts a real Nvim with userArgs exactly as the App would, +// i.e. whatever cli.Parse produced for the command line. Config-level +// extraArgs stay at "-u NONE -n" so the host's personal init.lua can't +// change the outcome. +func spawnWithArgs(t *testing.T, userArgs []string) *nvimproc.Process { + t.Helper() + nvimPath := requireNvim(t) + + proc, err := nvimproc.Spawn(nvimPath, []string{"-u", "NONE", "-n"}, userArgs, 60, 20) + if err != nil { + t.Fatalf("nvimproc.Spawn(%q): %v", userArgs, err) + } + t.Cleanup(func() { + // Buffers opened by a pass-through "-c edit" may be modified, + // which would leave `confirm qa` waiting on a prompt; drop the + // modified flag first so cleanup can't hang the suite. + _ = proc.Nvim.Command("silent! bufdo set nomodified") + proc.RequestQuit() + select { + case <-proc.Exited: + return + case <-time.After(2 * time.Second): + } + t.Logf("nvim did not exit gracefully after RequestQuit; force-closing") + _ = proc.Nvim.Close() + <-proc.Exited + }) + return proc +} + +// eval returns the trimmed output of `echo expr` from the live Nvim, which +// is ground truth about what the arguments actually did — independent of +// our own redraw-derived mirror. +func eval(t *testing.T, proc *nvimproc.Process, expr string) string { + t.Helper() + out, err := proc.Nvim.CommandOutput("echo " + expr) + if err != nil { + t.Fatalf("echo %s: %v", expr, err) + } + return strings.TrimSpace(out) +} + +// TestLauncherArgsReachNvim is the end-of-the-line check for the launcher +// invocation +// +// simplenvim --maximized -- -c term -c edit +// +// It runs the real argument parser and feeds its output to a real Nvim, so +// it fails if either half of the chain (parsing or forwarding) regresses. +func TestLauncherArgsReachNvim(t *testing.T) { + dir := t.TempDir() + todo := filepath.Join(dir, "n.todo") + if err := os.WriteFile(todo, []byte("write the tests\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + opts, err := cli.Parse("simplenvim", []string{"--maximized", "--", "-c", "term", "-c", "edit", todo}, os.Stderr) + if err != nil { + t.Fatalf("cli.Parse: %v", err) + } + if !opts.Maximized { + t.Fatal("Maximized = false, want true") + } + + proc := spawnWithArgs(t, opts.NvimArgs) + s := uistate.New() + drainUntilFlush(t, proc, s) + + // "-c term" must have produced a terminal buffer... + if got := eval(t, proc, `len(filter(map(range(1, bufnr('$')), 'getbufvar(v:val, "&buftype")'), 'v:val ==# "terminal"'))`); got == "0" { + t.Error("no terminal buffer exists, so \"-c term\" did not reach nvim") + } + + // ...and the file must have been loaded as buffer 1. + // + // Note what nvim actually does with this command line: "-c" takes + // exactly *one* argument, so "-c edit " is "-c edit" plus a + // positional file argument — not "edit " as one command. The + // file therefore becomes buffer 1 (nvim's usual treatment of a file + // argument) while the bare ":edit" merely reloads the current + // buffer, which by then is the terminal "-c term" just opened. So + // the terminal, not the file, is what stays on screen. + // + // This is nvim's own argument grammar, identical to running the same + // line against plain `nvim`, and it's precisely the behaviour a + // pass-through must preserve: our job is to forward arguments + // verbatim, not to second-guess them. + if got := eval(t, proc, "bufname(1)"); !strings.HasSuffix(got, "n.todo") { + t.Errorf("buffer 1 = %q, want the n.todo file argument", got) + } + if got := eval(t, proc, "getbufline(1, 1)[0]"); got != "write the tests" { + t.Errorf("buffer 1 line 1 = %q, want %q", got, "write the tests") + } + if got := eval(t, proc, "&buftype"); got != "terminal" { + t.Errorf("current buftype = %q, want %q (the -c term buffer stays current)", got, "terminal") + } +} + +// TestPassThroughArgsPrecedeFiles pins the ordering cli.Parse promises: +// nvim flags are positional-hungry, so "-O" has to arrive before the files +// it applies to or it opens one window instead of two vertical splits. +func TestPassThroughArgsPrecedeFiles(t *testing.T) { + dir := t.TempDir() + var files []string + for _, name := range []string{"left.txt", "right.txt"} { + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte(name+"\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + files = append(files, path) + } + + argv := append(append([]string{}, files...), "--", "-O") + opts, err := cli.Parse("simplenvim", argv, os.Stderr) + if err != nil { + t.Fatalf("cli.Parse: %v", err) + } + + proc := spawnWithArgs(t, opts.NvimArgs) + s := uistate.New() + drainUntilFlush(t, proc, s) + + if got := eval(t, proc, "winnr('$')"); got != "2" { + t.Errorf("window count = %q, want \"2\" (-O should split both files vertically)", got) + } +} + +// TestNoUserArgsStillStarts guards the plain "open an empty editor" case +// against an off-by-one in argument assembly. +func TestNoUserArgsStillStarts(t *testing.T) { + opts, err := cli.Parse("simplenvim", nil, os.Stderr) + if err != nil { + t.Fatalf("cli.Parse: %v", err) + } + + proc := spawnWithArgs(t, opts.NvimArgs) + s := uistate.New() + drainUntilFlush(t, proc, s) + + if got := eval(t, proc, "expand('%')"); got != "" { + t.Errorf("current buffer name = %q, want empty", got) + } +} diff --git a/src/test/unit/cli_test.go b/src/test/unit/cli_test.go new file mode 100644 index 0000000..57f781c --- /dev/null +++ b/src/test/unit/cli_test.go @@ -0,0 +1,237 @@ +package unit_test + +import ( + "flag" + "io" + "reflect" + "strings" + "testing" + + gioapp "gioui.org/app" + "gioui.org/unit" + + appkg "github.com/kgfly/SimpleNvimEditor/internal/app" + "github.com/kgfly/SimpleNvimEditor/internal/cli" +) + +// parseArgs runs cli.Parse the way main does, discarding usage output. +func parseArgs(t *testing.T, argv ...string) cli.Options { + t.Helper() + opts, err := cli.Parse("simplenvim", argv, io.Discard) + if err != nil { + t.Fatalf("Parse(%q) error = %v, want nil", argv, err) + } + return opts +} + +// TestParseLauncherInvocation is the regression test for the exact command +// line a desktop launcher uses: +// +// simplenvim --maximized -- -c term -c edit /path/to/n.todo +// +// Before pass-through support existed, everything after --maximized was +// parsed as our own flags, so "-c" aborted startup instead of reaching Nvim. +func TestParseLauncherInvocation(t *testing.T) { + const todo = "/Users/k0g0kfq/data1/.nnn/n.todo" + opts := parseArgs(t, "--maximized", "--", "-c", "term", "-c", "edit", todo) + + if !opts.Maximized { + t.Error("Maximized = false, want true") + } + want := []string{"-c", "term", "-c", "edit", todo} + if !reflect.DeepEqual(opts.NvimArgs, want) { + t.Errorf("NvimArgs = %q, want %q", opts.NvimArgs, want) + } +} + +func TestParseArgumentForms(t *testing.T) { + cases := []struct { + desc string + argv []string + maximized bool + nvimPath string + version bool + nvimArgs []string + }{ + { + desc: "no arguments at all opens an empty editor", + argv: nil, + nvimArgs: []string{}, + }, + { + desc: "bare files are passed to nvim", + argv: []string{"a.txt", "b.txt"}, + nvimArgs: []string{"a.txt", "b.txt"}, + }, + { + desc: "single-dash and double-dash spell the same flag", + argv: []string{"-maximized"}, + maximized: true, + nvimArgs: []string{}, + }, + { + desc: "--maximized=false explicitly opts out", + argv: []string{"--maximized=false"}, + maximized: false, + nvimArgs: []string{}, + }, + { + desc: "the -nvim override is ours, not nvim's", + argv: []string{"--nvim", "/opt/bin/nvim", "x.txt"}, + nvimPath: "/opt/bin/nvim", + nvimArgs: []string{"x.txt"}, + }, + { + desc: "--version is recognised", + argv: []string{"--version"}, + version: true, + nvimArgs: []string{}, + }, + { + // Nvim flags are positional-hungry ("nvim -O a b" opens + // splits), so pass-through must precede files. + desc: "pass-through args come before positional files", + argv: []string{"a.txt", "b.txt", "--", "-O"}, + nvimArgs: []string{"-O", "a.txt", "b.txt"}, + }, + { + desc: "a separator with nothing after it is harmless", + argv: []string{"a.txt", "--"}, + nvimArgs: []string{"a.txt"}, + }, + { + // Only the first separator is ours to consume; a second + // one is meaningful to nvim (it ends *its* option list). + desc: "later separators are passed through verbatim", + argv: []string{"--", "-u", "NONE", "--", "-weird-file"}, + nvimArgs: []string{"-u", "NONE", "--", "-weird-file"}, + }, + { + desc: "our flags and pass-through args combine", + argv: []string{"--maximized", "--nvim", "/n", "--", "-c", "term"}, + maximized: true, + nvimPath: "/n", + nvimArgs: []string{"-c", "term"}, + }, + { + // Without the separator this would be a parse error, so + // it doubles as proof the separator is what rescues it. + desc: "nvim flags survive verbatim after the separator", + argv: []string{"--", "-c", "set nonumber", "--cmd", "let g:x=1"}, + nvimArgs: []string{"-c", "set nonumber", "--cmd", "let g:x=1"}, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + opts := parseArgs(t, tc.argv...) + if opts.Maximized != tc.maximized { + t.Errorf("Maximized = %v, want %v", opts.Maximized, tc.maximized) + } + if opts.NvimPath != tc.nvimPath { + t.Errorf("NvimPath = %q, want %q", opts.NvimPath, tc.nvimPath) + } + if opts.ShowVersion != tc.version { + t.Errorf("ShowVersion = %v, want %v", opts.ShowVersion, tc.version) + } + if !reflect.DeepEqual(opts.NvimArgs, tc.nvimArgs) { + t.Errorf("NvimArgs = %q, want %q", opts.NvimArgs, tc.nvimArgs) + } + }) + } +} + +// TestParseUnknownFlagIsAnError guards the other half of the separator's +// value: without it an nvim-style flag really is rejected, so callers +// genuinely need "--" rather than it being cosmetic. +func TestParseUnknownFlagIsAnError(t *testing.T) { + var out strings.Builder + if _, err := cli.Parse("simplenvim", []string{"-c", "term"}, &out); err == nil { + t.Fatal("Parse(-c term) returned nil error, want a parse failure") + } + if out.Len() == 0 { + t.Error("a parse failure should explain itself on the provided writer") + } +} + +// TestParseHelpIsNotAFailure pins the distinction main relies on: "user +// asked for help" (exit 0) versus "user got it wrong" (exit 2). +func TestParseHelpIsNotAFailure(t *testing.T) { + var out strings.Builder + _, err := cli.Parse("simplenvim", []string{"-h"}, &out) + if err != flag.ErrHelp { + t.Fatalf("Parse(-h) error = %v, want flag.ErrHelp", err) + } + // The separator is the non-obvious part of the interface, so usage + // has to actually document it. + usage := out.String() + for _, want := range []string{"Usage:", cli.Separator, "maximized"} { + if !strings.Contains(usage, want) { + t.Errorf("usage text does not mention %q:\n%s", want, usage) + } + } +} + +func TestSplitFindsTheFirstSeparator(t *testing.T) { + cases := []struct { + desc string + argv []string + own, passthrough []string + }{ + {"no separator", []string{"a", "-maximized"}, []string{"a", "-maximized"}, nil}, + {"separator first", []string{"--", "-c", "q"}, []string{}, []string{"-c", "q"}}, + {"separator last", []string{"a", "--"}, []string{"a"}, []string{}}, + {"repeated separators", []string{"--", "--", "x"}, []string{}, []string{"--", "x"}}, + {"empty argv", nil, nil, nil}, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + own, pass := cli.Split(tc.argv) + if !reflect.DeepEqual(own, tc.own) { + t.Errorf("own = %q, want %q", own, tc.own) + } + if !reflect.DeepEqual(pass, tc.passthrough) { + t.Errorf("passthrough = %q, want %q", pass, tc.passthrough) + } + }) + } +} + +// applyWindowOptions applies the App's real option set to a Gio Config the +// same way gioui.org/app does internally, so these tests exercise the +// production options rather than a restatement of them. +func applyWindowOptions(t *testing.T, opts appkg.Options) gioapp.Config { + t.Helper() + var cfg gioapp.Config + metric := unit.Metric{PxPerDp: 1, PxPerSp: 1} + for _, o := range appkg.WindowOptions(opts) { + o(metric, &cfg) + } + return cfg +} + +// TestWindowOptionsMaximized checks that --maximized reaches Gio as a real +// window mode, rather than being parsed and then quietly ignored. +func TestWindowOptionsMaximized(t *testing.T) { + cfg := applyWindowOptions(t, appkg.Options{Maximized: true}) + if cfg.Mode != gioapp.Maximized { + t.Errorf("Mode = %v, want %v", cfg.Mode, gioapp.Maximized) + } + // A restore size must still be requested: it is what the window + // returns to when un-maximized, and the fallback on platforms that + // cannot honor a maximize request at all. + if cfg.Size.X <= 0 || cfg.Size.Y <= 0 { + t.Errorf("Size = %v, want a positive restore size alongside Maximized", cfg.Size) + } +} + +func TestWindowOptionsDefaultsToWindowed(t *testing.T) { + cfg := applyWindowOptions(t, appkg.Options{}) + if cfg.Mode != gioapp.Windowed { + t.Errorf("Mode = %v, want %v", cfg.Mode, gioapp.Windowed) + } + if cfg.Size.X <= 0 || cfg.Size.Y <= 0 { + t.Errorf("Size = %v, want a positive default window size", cfg.Size) + } +} diff --git a/src/test/unit/config_test.go b/src/test/unit/config_test.go index 704c3f4..93f8509 100644 --- a/src/test/unit/config_test.go +++ b/src/test/unit/config_test.go @@ -189,3 +189,113 @@ func TestDefaultFontFamilyForOSNeverEmpty(t *testing.T) { t.Fatalf("DefaultFontFamilyForOS() returned empty string") } } + +func TestLoadExplicitZeroFontSizeAndEmptyCommand(t *testing.T) { + withIsolatedConfigDir(t) + + path, err := config.FilePath() + if err != nil { + t.Fatalf("FilePath() error = %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + toml := ` +[editor] +font_size = 0 + +[nvim] +command = "" +` + if err := os.WriteFile(path, []byte(toml), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + cfg, err := config.Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Editor.FontSize != config.Default().Editor.FontSize { + t.Errorf("zero FontSize should fall back to default, got %v", cfg.Editor.FontSize) + } + if cfg.Nvim.Command != config.Default().Nvim.Command { + t.Errorf("empty Command should fall back to default, got %q", cfg.Nvim.Command) + } +} + +func TestLoadNegativeFontSize(t *testing.T) { + withIsolatedConfigDir(t) + + path, err := config.FilePath() + if err != nil { + t.Fatalf("FilePath() error = %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(path, []byte("[editor]\nfont_size = -5\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + cfg, err := config.Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Editor.FontSize != config.Default().Editor.FontSize { + t.Errorf("negative FontSize should fall back to default, got %v", cfg.Editor.FontSize) + } +} + +func TestLoadUnreadableFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("chmod does not restrict reads on Windows") + } + withIsolatedConfigDir(t) + + path, err := config.FilePath() + if err != nil { + t.Fatalf("FilePath() error = %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(path, []byte("[editor]\n"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if err := os.Chmod(path, 0o000); err != nil { + t.Skipf("cannot chmod on this platform: %v", err) + } + t.Cleanup(func() { os.Chmod(path, 0o644) }) + + _, err = config.Load() + if err == nil { + t.Fatal("Load() with unreadable file should return an error") + } +} + +func TestLoadSameFontFamilyAsDefault(t *testing.T) { + withIsolatedConfigDir(t) + + path, err := config.FilePath() + if err != nil { + t.Fatalf("FilePath() error = %v", err) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // Set font_family to the same as default — UseSystemFonts should NOT be set. + toml := `[editor] +font_family = "monospace" +` + if err := os.WriteFile(path, []byte(toml), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + cfg, err := config.Load() + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Editor.UseSystemFonts { + t.Error("UseSystemFonts should not be set when font_family == default") + } +} diff --git a/src/test/unit/coverage_test.go b/src/test/unit/coverage_test.go new file mode 100644 index 0000000..ed86d7f --- /dev/null +++ b/src/test/unit/coverage_test.go @@ -0,0 +1,602 @@ +package unit_test + +import ( + "image/color" + "testing" + + "github.com/kgfly/SimpleNvimEditor/internal/config" + "github.com/kgfly/SimpleNvimEditor/internal/uistate" +) + +func nrgba(r, g, b uint8) color.NRGBA { + return color.NRGBA{R: r, G: g, B: b, A: 0xff} +} + +// Exercise the len(t)