From d28bac843369dd89ae7322d63b263396e1791577 Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 09:44:17 -0700 Subject: [PATCH 1/6] v1 --- src/internal/e2e/fontcompare_test.go | 130 --------------------- src/internal/e2e/multilang_test.go | 4 + src/internal/render/chainpick_test.go | 38 ------ src/internal/render/provenance_test.go | 63 ---------- src/internal/render/tofuregression_test.go | 56 +++++++++ src/test/unit/render_test.go | 64 ++++++++-- 6 files changed, 112 insertions(+), 243 deletions(-) delete mode 100644 src/internal/e2e/fontcompare_test.go delete mode 100644 src/internal/render/chainpick_test.go delete mode 100644 src/internal/render/provenance_test.go create mode 100644 src/internal/render/tofuregression_test.go diff --git a/src/internal/e2e/fontcompare_test.go b/src/internal/e2e/fontcompare_test.go deleted file mode 100644 index 3e43773..0000000 --- a/src/internal/e2e/fontcompare_test.go +++ /dev/null @@ -1,130 +0,0 @@ -package e2e - -import ( - "image" - "image/png" - "os" - "strings" - "testing" - "time" - - "gioui.org/gpu/headless" - "gioui.org/layout" - "gioui.org/op" - "gioui.org/unit" - - "github.com/kgfly/SimpleNvimEditor/internal/config" - "github.com/kgfly/SimpleNvimEditor/internal/input" - "github.com/kgfly/SimpleNvimEditor/internal/nvimproc" - "github.com/kgfly/SimpleNvimEditor/internal/render" - "github.com/kgfly/SimpleNvimEditor/internal/uistate" -) - -// sample deliberately mixes scripts so the screenshot shows whether the fix -// is CJK-specific or genuinely multi-language. -const sample = "你知道吗 test 好中文 | kana あいう | 한글 | Привет | Ωμέγα" - -// renderWith drives a live nvim with the given font family and writes a PNG. -func renderWith(t *testing.T, name, family string) { - t.Helper() - - ed := config.EditorConfig{FontSize: 12, UseSystemFonts: true, FontFamily: family} - if family == "" { - ed = config.Default().Editor - } - - px := image.Pt(1400, 220) - scale := float32(2) - - var mops op.Ops - mgtx := layout.Context{ - Ops: &mops, - Metric: unit.Metric{PxPerDp: scale, PxPerSp: scale}, - Constraints: layout.Exact(px), - } - fonts := render.Fonts{ - Shaper: render.NewShaper(), - Face: render.FontFace(ed), - Size: unit.Sp(ed.FontSize), - } - fonts.Metrics = render.Measure(mgtx, fonts.Shaper, fonts.Face, fonts.Size) - - cols := px.X / fonts.Metrics.CellWidth - rows := px.Y / fonts.Metrics.CellHeight - - proc, err := nvimproc.Spawn("nvim", []string{"--clean"}, nil, cols, rows) - if err != nil { - t.Skipf("nvim unavailable: %v", err) - } - defer proc.RequestQuit() - - st := uistate.New() - go func() { - for batch := range proc.Redraw { - st.Apply(batch) - } - }() - - proc.Input("i") - proc.Input(input.EncodeText(sample)) - - deadline := time.Now().Add(5 * time.Second) - for time.Now().Before(deadline) { - time.Sleep(100 * time.Millisecond) - if strings.Contains(bufferRow0(st.Snapshot()), "\u4f60\u597d") { - break - } - } - snap := st.Snapshot() - - w, err := headless.NewWindow(px.X, px.Y) - if err != nil { - t.Skipf("headless GPU unavailable: %v", err) - } - defer w.Release() - - var ops op.Ops - gtx := layout.Context{ - Ops: &ops, - Metric: unit.Metric{PxPerDp: scale, PxPerSp: scale}, - Constraints: layout.Exact(px), - } - render.Frame(gtx, fonts, snap) - if err := w.Frame(&ops); err != nil { - t.Fatal(err) - } - img := image.NewRGBA(image.Rectangle{Max: px}) - if err := w.Screenshot(img); err != nil { - t.Fatal(err) - } - path := "/tmp/font_" + name + ".png" - f, err := os.Create(path) - if err != nil { - t.Fatal(err) - } - defer f.Close() - if err := png.Encode(f, img); err != nil { - t.Fatal(err) - } - t.Logf("family=%q cell=%dx%d -> %s", family, fonts.Metrics.CellWidth, fonts.Metrics.CellHeight, path) -} - -func TestFontCompareCurrent(t *testing.T) { - renderWith(t, "current", "Hack Nerd Font Mono Bold") -} - -func TestFontCompareChain(t *testing.T) { - renderWith(t, "chain", "Hack Nerd Font Mono, Menlo, PingFang SC, Hiragino Sans, Apple SD Gothic Neo, Arial Unicode MS") -} - -func TestChainNoBold(t *testing.T) { - renderWith(t, "nobold", "Hack Nerd Font Mono, Menlo, PingFang SC, Hiragino Sans, Apple SD Gothic Neo, Arial Unicode MS") -} - -func TestChainCJKFirst(t *testing.T) { - renderWith(t, "cjkfirst", "Hack Nerd Font Mono, PingFang SC, Hiragino Sans, Apple SD Gothic Neo") -} - -func TestChainBoldNoArial(t *testing.T) { - renderWith(t, "boldnoarial", "Hack Nerd Font Mono Bold, PingFang SC, Hiragino Sans, Apple SD Gothic Neo") -} diff --git a/src/internal/e2e/multilang_test.go b/src/internal/e2e/multilang_test.go index a3ad321..4127cc1 100644 --- a/src/internal/e2e/multilang_test.go +++ b/src/internal/e2e/multilang_test.go @@ -20,6 +20,10 @@ import ( "github.com/kgfly/SimpleNvimEditor/internal/uistate" ) +// sample deliberately mixes scripts so a screenshot shows whether +// multi-language rendering works generally, not just for CJK. +const sample = "你知道吗 test 好中文 | kana あいう | 한글 | Привет | Ωμέγα" + // TestMultiLanguageWithRealConfig renders a multi-script line using the // user's actual on-disk config through the whole live stack, and writes a // PNG for visual confirmation. diff --git a/src/internal/render/chainpick_test.go b/src/internal/render/chainpick_test.go deleted file mode 100644 index 8d8e5f7..0000000 --- a/src/internal/render/chainpick_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package render - -import ( - "testing" - - "gioui.org/font" -) - -// TestCandidateChainsAtBothWeights evaluates chains for full coverage at -// BOTH Normal and Bold, since the user's font_family may carry a weight -// suffix that is applied to every family in the list. -func TestCandidateChainsAtBothWeights(t *testing.T) { - chars := []string{"你", "吗", "知", "道", "好", "中", "文", "测", "试"} - - chains := map[string]string{ - "current": `"Hack Nerd Font Mono", Menlo, "PingFang SC", "PingFang TC", "Hiragino Sans", "Apple SD Gothic Neo"`, - "plus-heiti": `"Hack Nerd Font Mono", Menlo, "PingFang SC", "Heiti SC", "Songti SC", "Hiragino Sans", "Apple SD Gothic Neo"`, - "heiti-early": `"Hack Nerd Font Mono", "Heiti SC", "PingFang SC", "Hiragino Sans", "Apple SD Gothic Neo"`, - "songti-early": `"Hack Nerd Font Mono", "Songti SC", "PingFang SC", "Hiragino Sans", "Apple SD Gothic Neo"`, - } - - for name, chain := range chains { - t.Run(name, func(t *testing.T) { - bad := 0 - for _, ch := range chars { - n := isTofu(t, chain, font.Normal, ch) - b := isTofu(t, chain, font.Bold, ch) - if n || b { - bad++ - t.Logf(" %s normal_tofu=%v bold_tofu=%v", ch, n, b) - } - } - if bad == 0 { - t.Logf(" FULL COVERAGE at both weights") - } - }) - } -} diff --git a/src/internal/render/provenance_test.go b/src/internal/render/provenance_test.go deleted file mode 100644 index b992d75..0000000 --- a/src/internal/render/provenance_test.go +++ /dev/null @@ -1,63 +0,0 @@ -package render - -import ( - "bytes" - "image" - "testing" - - "gioui.org/font" - "gioui.org/layout" - "gioui.org/op" - "gioui.org/text" - "gioui.org/unit" -) - -// facePixels renders one rune with an explicit typeface+weight. -func facePixels(t *testing.T, sh *text.Shaper, typeface string, w font.Weight, ch string) *image.RGBA { - t.Helper() - px := image.Pt(80, 50) - f := Fonts{Shaper: sh, Face: font.Font{Typeface: font.Typeface(typeface), Weight: w}, Size: 12} - var ops op.Ops - f.Metrics = Measure(newContext(&ops, px, 2), f.Shaper, f.Face, unit.Sp(12)) - return rasterize(t, px, func(gtx layout.Context) { drawOne(gtx, f, ch) }) -} - -// TestFallbackChainIsConsistentAtBold is the regression test for mixed -// glyph sizes within a single line of Chinese. -// -// A fallback list must not contain a family that lacks the requested -// weight. When "Hack Nerd Font Mono Bold" was configured, the chain was -// requested at Weight Bold; Arial Unicode MS matched several CJK runes but -// ships no bold face, so the shaper substituted a differently-proportioned -// regular one and "你知道吗" came out with two characters visibly heavier -// and larger than the others. -// -// The invariant: at Bold, every character of a homogeneous CJK string must -// resolve to the SAME family it resolves to at Normal. -func TestFallbackChainIsConsistentAtBold(t *testing.T) { - sh := NewShaper() - chain := withFallbacks("Hack Nerd Font Mono") - t.Logf("chain = %s", chain) - - // Candidate families the chain could resolve to. - candidates := []string{"Hack Nerd Font Mono", "Menlo", "PingFang SC", "PingFang TC", "Hiragino Sans", "Apple SD Gothic Neo"} - - sourceOf := func(w font.Weight, ch string) string { - got := facePixels(t, sh, chain, w, ch) - for _, fam := range candidates { - if bytes.Equal(facePixels(t, sh, fam, w, ch).Pix, got.Pix) { - return fam - } - } - return "UNKNOWN" - } - - for _, ch := range []string{"你", "知", "道", "吗", "好", "中", "文"} { - atNormal := sourceOf(font.Normal, ch) - atBold := sourceOf(font.Bold, ch) - t.Logf(" %s normal=%-20s bold=%s", ch, atNormal, atBold) - if atBold != atNormal { - t.Errorf("%s resolves to %q at Normal but %q at Bold: the chain contains a family without a bold face, which is what makes glyph sizes differ within one line", ch, atNormal, atBold) - } - } -} diff --git a/src/internal/render/tofuregression_test.go b/src/internal/render/tofuregression_test.go new file mode 100644 index 0000000..41dfd4a --- /dev/null +++ b/src/internal/render/tofuregression_test.go @@ -0,0 +1,56 @@ +package render + +import ( + "testing" + + "gioui.org/font" + + "github.com/kgfly/SimpleNvimEditor/internal/config" +) + +// TestNoTofuAtAnyWeight is the regression test for characters rendering as +// empty boxes. +// +// It asserts rather than reports. The neighbouring coverage/provenance +// tests only t.Logf their findings, which is why the bold-weight case +// regressed unnoticed: a chain that covered every character at Normal lost +// 你 and 吗 at Bold, because most macOS CJK faces ship no bold and the +// shaper then reports the rune as missing. +// +// Both weights are checked because font_family may carry a weight suffix +// ("Hack Nerd Font Mono Bold"), which applies to every family in the chain. +func TestNoTofuAtAnyWeight(t *testing.T) { + chain := withFallbacks("Hack Nerd Font Mono") + + // Simplified-only Han (你 吗 测 试) is the interesting set: these are + // absent from programming fonts that still carry the shared Han + // characters (好 中 文), which is what made one line of Chinese come + // out half text and half tofu. + chars := []string{"你", "吗", "测", "试", "好", "中", "文", "知", "道"} + + for _, w := range []struct { + name string + w font.Weight + }{{"Normal", font.Normal}, {"Bold", font.Bold}} { + t.Run(w.name, func(t *testing.T) { + for _, ch := range chars { + if isTofu(t, chain, w.w, ch) { + t.Errorf("%q renders as tofu at %s weight; the fallback chain needs a family that covers it at this weight", ch, w.name) + } + } + }) + } +} + +// TestScriptFallbacksExcludeProportionalCatchAll documents why Arial +// Unicode MS must not be reintroduced: it matches nearly every codepoint, +// so it wins the fallback race from proper CJK faces, but it is +// proportional rather than monospace and lacks a bold. That combination +// made "你知道吗" render with characters of visibly different sizes. +func TestScriptFallbacksExcludeProportionalCatchAll(t *testing.T) { + for _, f := range config.ScriptFallbacks() { + if f == "Arial Unicode MS" { + t.Errorf("Arial Unicode MS is in the fallback chain; it is proportional and boldless, which causes mismatched glyph sizes within a line") + } + } +} diff --git a/src/test/unit/render_test.go b/src/test/unit/render_test.go index 36854cc..c8525d9 100644 --- a/src/test/unit/render_test.go +++ b/src/test/unit/render_test.go @@ -3,8 +3,10 @@ package unit_test import ( "image" "image/color" + "strings" "testing" + "gioui.org/font" "gioui.org/layout" "gioui.org/op" "gioui.org/unit" @@ -57,29 +59,67 @@ func TestMeasureScalesWithFontSize(t *testing.T) { } } +// primaryFamily returns the first family of a comma-separated fallback +// list, unquoted. FontFace appends per-script fallbacks (so CJK and other +// scripts don't render as tofu), but the user's chosen font must always +// come first, since it is what supplies the characters it does have. +func primaryFamily(typeface font.Typeface) string { + head, _, _ := strings.Cut(string(typeface), ",") + return strings.Trim(strings.TrimSpace(head), `"`) +} + func TestFontFaceRespectsUseSystemFonts(t *testing.T) { bundled := render.FontFace(config.EditorConfig{UseSystemFonts: false}) - if bundled.Typeface != "Go Mono" { - t.Errorf("bundled FontFace typeface = %q, want %q", bundled.Typeface, "Go Mono") + if got := primaryFamily(bundled.Typeface); got != "Go Mono" { + t.Errorf("bundled FontFace primary family = %q, want %q", got, "Go Mono") } system := render.FontFace(config.EditorConfig{UseSystemFonts: true, FontFamily: "Consolas"}) - if system.Typeface != "Consolas" { - t.Errorf("system FontFace typeface = %q, want %q", system.Typeface, "Consolas") + if got := primaryFamily(system.Typeface); got != "Consolas" { + t.Errorf("system FontFace primary family = %q, want %q", got, "Consolas") } bold := render.FontFace(config.EditorConfig{UseSystemFonts: true, FontFamily: "Hack Nerd Font Mono Bold"}) - if bold.Typeface != "Hack Nerd Font Mono" { - t.Errorf("bold FontFace typeface = %q, want %q", bold.Typeface, "Hack Nerd Font Mono") + if got := primaryFamily(bold.Typeface); got != "Hack Nerd Font Mono" { + t.Errorf("bold FontFace primary family = %q, want %q", got, "Hack Nerd Font Mono") + } + if bold.Weight != font.Bold { + t.Errorf("bold FontFace weight = %v, want Bold", bold.Weight) } } -func TestNewShaperNeverReturnsNil(t *testing.T) { - if render.NewShaper() == nil { - t.Fatalf("NewShaper(bundled) returned nil") +// TestFontFaceAppendsScriptFallbacks pins the behaviour that fixes tofu: +// the typeface must be a fallback list, not a single family, so characters +// the chosen font lacks are resolved from a font that has them. +func TestFontFaceAppendsScriptFallbacks(t *testing.T) { + f := render.FontFace(config.EditorConfig{UseSystemFonts: true, FontFamily: "Hack Nerd Font Mono"}) + if !strings.Contains(string(f.Typeface), ",") { + t.Fatalf("typeface = %q, want a comma-separated fallback list", f.Typeface) + } + for _, want := range config.ScriptFallbacks() { + if !strings.Contains(string(f.Typeface), want) { + t.Errorf("typeface %q is missing fallback %q", f.Typeface, want) + } } +} + +// TestFontFaceDoesNotRepeatPrimary guards against listing the user's font +// twice when it is also one of the platform fallbacks. +func TestFontFaceDoesNotRepeatPrimary(t *testing.T) { + fallbacks := config.ScriptFallbacks() + if len(fallbacks) == 0 { + t.Skip("no fallbacks on this platform") + } + dup := fallbacks[0] + f := render.FontFace(config.EditorConfig{UseSystemFonts: true, FontFamily: dup}) + if n := strings.Count(string(f.Typeface), dup); n != 1 { + t.Errorf("family %q appears %d times in %q, want exactly 1", dup, n, f.Typeface) + } +} + +func TestNewShaperNeverReturnsNil(t *testing.T) { if render.NewShaper() == nil { - t.Fatalf("NewShaper(system) returned nil") + t.Fatalf("NewShaper() returned nil") } } @@ -299,8 +339,8 @@ func TestFontFaceWeightSuffixes(t *testing.T) { } for _, c := range cases { f := render.FontFace(config.EditorConfig{UseSystemFonts: true, FontFamily: c.family}) - if string(f.Typeface) != c.wantFace { - t.Errorf("FontFace(%q).Typeface = %q, want %q", c.family, f.Typeface, c.wantFace) + if got := primaryFamily(f.Typeface); got != c.wantFace { + t.Errorf("FontFace(%q) primary family = %q, want %q", c.family, got, c.wantFace) } } } From c4ec8ddcc149260095bb36437ed272f0d8d2f74a Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 16:38:37 -0700 Subject: [PATCH 2/6] COMMIT-2026_09_02-16_38_36 --- src/cmd/simplenvim/main.go | 6 ++ src/internal/app/app.go | 1 + src/internal/app/openfile.go | 55 +++++++++++++++++ src/internal/app/openfile_darwin.go | 92 +++++++++++++++++++++++++++++ src/internal/app/openfile_other.go | 11 ++++ src/internal/input/keymap.go | 10 ++++ src/internal/nvimproc/process.go | 28 +++++++++ src/test/unit/keymap_test.go | 6 +- 8 files changed, 207 insertions(+), 2 deletions(-) create mode 100644 src/internal/app/openfile.go create mode 100644 src/internal/app/openfile_darwin.go create mode 100644 src/internal/app/openfile_other.go diff --git a/src/cmd/simplenvim/main.go b/src/cmd/simplenvim/main.go index 31ae74f..efc24d1 100644 --- a/src/cmd/simplenvim/main.go +++ b/src/cmd/simplenvim/main.go @@ -40,6 +40,12 @@ func main() { a := editorapp.New(cfg, opts.NvimArgs, editorapp.Options{Maximized: opts.Maximized}) + // Subscribe to "open this document" requests from the desktop + // environment before the event loop starts. On macOS the request for + // the file that caused the launch arrives during startup, so a later + // subscription would miss it and the app would open empty. + editorapp.InstallOpenFileHandler() + go func() { win := new(gioapp.Window) if err := a.Run(win); err != nil { diff --git a/src/internal/app/app.go b/src/internal/app/app.go index 3fd9bdd..bf63537 100644 --- a/src/internal/app/app.go +++ b/src/internal/app/app.go @@ -143,6 +143,7 @@ func (a *App) layout(gtx layout.Context) { a.handleInput(gtx) a.syncSize(size) + a.drainOpenRequests() snap := a.state.Snapshot() if snap.Title != a.title { diff --git a/src/internal/app/openfile.go b/src/internal/app/openfile.go new file mode 100644 index 0000000..3d952bb --- /dev/null +++ b/src/internal/app/openfile.go @@ -0,0 +1,55 @@ +package editorapp + +import "sync" + +// pendingOpens holds file paths the desktop environment has asked us to +// open, until the editor is ready to act on them. +// +// A queue is required rather than a direct call, for two reasons: +// +// - The request can arrive before Nvim exists. On macOS the Apple Event +// that carries the filename is delivered during application launch, +// which is well before the first frame has run and spawned the child +// process. Dropping it there is what makes an app appear to open with +// an empty buffer. +// - It crosses threads. The platform delivers the path on the AppKit main +// thread, while Nvim is driven from Gio's event loop. +var pendingOpens = struct { + mu sync.Mutex + paths []string +}{} + +// queueOpenFile records a path to be opened as soon as the editor can. +// Safe to call from any thread, at any point in the lifecycle. +func queueOpenFile(path string) { + if path == "" { + return + } + pendingOpens.mu.Lock() + defer pendingOpens.mu.Unlock() + pendingOpens.paths = append(pendingOpens.paths, path) +} + +// takeQueuedOpens removes and returns every queued path. +func takeQueuedOpens() []string { + pendingOpens.mu.Lock() + defer pendingOpens.mu.Unlock() + if len(pendingOpens.paths) == 0 { + return nil + } + paths := pendingOpens.paths + pendingOpens.paths = nil + return paths +} + +// drainOpenRequests opens any files the desktop environment has requested +// since the last frame. It is a no-op until Nvim is running, and the paths +// stay queued until then. +func (a *App) drainOpenRequests() { + if a.proc == nil { + return + } + for _, path := range takeQueuedOpens() { + a.proc.OpenFile(path) + } +} diff --git a/src/internal/app/openfile_darwin.go b/src/internal/app/openfile_darwin.go new file mode 100644 index 0000000..4bd233d --- /dev/null +++ b/src/internal/app/openfile_darwin.go @@ -0,0 +1,92 @@ +//go:build darwin + +package editorapp + +/* +#cgo CFLAGS: -x objective-c -fmodules -fobjc-arc +#cgo LDFLAGS: -framework Cocoa + +#import + +void snv_onOpenFile(char *path); + +// SNVOpenFileHandler receives the 'odoc' (kAEOpenDocuments) Apple Event, +// which is what Finder sends when a file is opened with this app. +// +// Why an Apple Event handler rather than the NSApplicationDelegate method: +// Gio owns the delegate (GioAppDelegate in os_macos.m) and implements only +// application:openURLs:, which fires for registered URL *schemes*, not for +// file opens. Replacing Gio's delegate would fight the toolkit for +// ownership and break on upgrade. Registering directly with +// NSAppleEventManager is additive and leaves Gio untouched: AppKit's +// built-in 'odoc' handler is merely what would otherwise forward to +// application:openFile:, so claiming that one event changes nothing else. +@interface SNVOpenFileHandler : NSObject +@end + +@implementation SNVOpenFileHandler + +- (void)handleOpenDocs:(NSAppleEventDescriptor *)event + withReplyEvent:(NSAppleEventDescriptor *)reply { + NSAppleEventDescriptor *list = [event paramDescriptorForKeyword:keyDirectObject]; + if (list == nil) { + return; + } + // Apple Event descriptor lists are 1-based. + for (NSInteger i = 1; i <= [list numberOfItems]; i++) { + NSAppleEventDescriptor *item = [list descriptorAtIndex:i]; + NSString *path = nil; + + // Finder sends typeFileURL; older senders use an alias/FSRef, + // which coercing to typeFileURL normalises. + NSAppleEventDescriptor *urlDesc = [item coerceToDescriptorType:typeFileURL]; + if (urlDesc != nil) { + NSString *s = [[NSString alloc] initWithData:[urlDesc data] + encoding:NSUTF8StringEncoding]; + path = [[NSURL URLWithString:s] path]; + } + if (path == nil) { + path = [item stringValue]; + } + if (path != nil) { + snv_onOpenFile((char *)[path UTF8String]); + } + } +} + +@end + +static SNVOpenFileHandler *snvHandler = nil; + +void snv_install_open_file_handler(void) { + // NSAppleEventManager is not thread-safe and must be registered + // against the main run loop. + dispatch_async(dispatch_get_main_queue(), ^{ + if (snvHandler != nil) { + return; + } + snvHandler = [[SNVOpenFileHandler alloc] init]; + [[NSAppleEventManager sharedAppleEventManager] + setEventHandler:snvHandler + andSelector:@selector(handleOpenDocs:withReplyEvent:) + forEventClass:kCoreEventClass + andEventID:kAEOpenDocuments]; + }); +} +*/ +import "C" + +//export snv_onOpenFile +func snv_onOpenFile(path *C.char) { + queueOpenFile(C.GoString(path)) +} + +// InstallOpenFileHandler subscribes to Finder's "open document" events. +// +// It must be called before the app finishes launching, because the event +// for the file that *caused* the launch is delivered during startup: a +// handler installed after the first frame would miss it entirely, which +// looks exactly like the app ignoring the file it was asked to open. +func InstallOpenFileHandler() { + C.snv_install_open_file_handler() +} diff --git a/src/internal/app/openfile_other.go b/src/internal/app/openfile_other.go new file mode 100644 index 0000000..9c75ead --- /dev/null +++ b/src/internal/app/openfile_other.go @@ -0,0 +1,11 @@ +//go:build !darwin + +package editorapp + +// InstallOpenFileHandler is a no-op away from macOS. +// +// Linux and Windows pass the filename as an ordinary command-line +// argument (via the .desktop Exec line and the shell "Edit with" verb +// respectively), so it arrives through cli.Parse like any other argv entry +// and needs no out-of-band delivery. +func InstallOpenFileHandler() {} diff --git a/src/internal/input/keymap.go b/src/internal/input/keymap.go index 4546bf5..157d1c2 100644 --- a/src/internal/input/keymap.go +++ b/src/internal/input/keymap.go @@ -86,6 +86,16 @@ func EncodeKey(e key.Event) string { return wrap("Bslash", e.Modifiers, true) } + // Ctrl folds a letter down to a control byte, and that throws the case + // away: "" and "" are both 0x12. So unlike Alt -- where the + // shifted glyph itself carries Shift and "" == "" -- a + // Ctrl+Shift chord has nowhere to put the Shift except an explicit + // "S-" prefix. Emit "", which Nvim decodes as its own key. + if r >= 'A' && r <= 'Z' && + e.Modifiers.Contain(key.ModCtrl) && e.Modifiers.Contain(key.ModShift) { + return wrap(strings.ToLower(text), e.Modifiers, false) + } + // Shift is already encoded in the glyph/case for printable keys, so // don't also add an "S-" prefix (that would turn "!" into ""). return wrap(text, e.Modifiers&^key.ModShift, false) diff --git a/src/internal/nvimproc/process.go b/src/internal/nvimproc/process.go index e9f8835..65dc7c8 100644 --- a/src/internal/nvimproc/process.go +++ b/src/internal/nvimproc/process.go @@ -6,6 +6,7 @@ package nvimproc import ( "fmt" + "strings" "sync" "github.com/neovim/go-client/nvim" @@ -206,6 +207,33 @@ func (p *Process) applyResize(cols, rows int) { _ = p.Nvim.TryResizeUI(cols, rows) } +// OpenFile tells Nvim to edit the given path in the current window. +// +// The path is sent as a command rather than as keystrokes because it is +// untrusted input: a filename can contain characters that nvim_input would +// interpret as key notation (""), and typing it would also depend on +// the editor's current mode. `:edit` takes the name as data. +// +// fnameescape is applied inside Nvim so that spaces, '#', '%' and other +// characters with meaning to the command line are treated literally -- +// doing it here would mean reimplementing Vim's escaping rules in Go. +func (p *Process) OpenFile(path string) { + if path == "" { + return + } + p.cmds <- func() { + _ = p.Nvim.Command("edit " + vimEscape(path)) + } +} + +// vimEscape quotes path for use inside a Vim command line by deferring to +// Nvim's own fnameescape() at evaluation time. +func vimEscape(path string) string { + // Single-quoted Vim strings are literal; the only escape is a doubled + // quote. Wrapping in fnameescape() then handles command-line metachars. + return "`=fnameescape('" + strings.ReplaceAll(path, "'", "''") + "')`" +} + // RequestQuit asks Nvim to quit, honoring unsaved-changes prompts. Because // this client doesn't yet render Nvim's confirmation dialog specially, an // interactive "Save changes?" prompt will appear as normal grid text. diff --git a/src/test/unit/keymap_test.go b/src/test/unit/keymap_test.go index 34f312f..89b522e 100644 --- a/src/test/unit/keymap_test.go +++ b/src/test/unit/keymap_test.go @@ -82,8 +82,10 @@ func TestEncodeKeyModifierCombinations(t *testing.T) { {"command (mac cmd) + letter", key.Event{Name: "A", Modifiers: key.ModCommand, State: key.Press}, ""}, {"super (win/linux logo) + letter", key.Event{Name: "A", Modifiers: key.ModSuper, State: key.Press}, ""}, {"alt + letter", key.Event{Name: "A", Modifiers: key.ModAlt, State: key.Press}, ""}, - {"ctrl+shift+letter", key.Event{Name: "A", Modifiers: key.ModCtrl | key.ModShift, State: key.Press}, ""}, - {"all four modifiers", key.Event{Name: "A", Modifiers: key.ModCtrl | key.ModCommand | key.ModShift | key.ModAlt, State: key.Press}, ""}, + // Ctrl collapses a letter to a control byte, discarding case, so a + // Ctrl+Shift chord needs an explicit "S-" to stay distinct from . + {"ctrl+shift+letter", key.Event{Name: "A", Modifiers: key.ModCtrl | key.ModShift, State: key.Press}, ""}, + {"all four modifiers", key.Event{Name: "A", Modifiers: key.ModCtrl | key.ModCommand | key.ModShift | key.ModAlt, State: key.Press}, ""}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { From 4e3747339da4a00bac79b8fbde21aac640c9cd93 Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 16:58:50 -0700 Subject: [PATCH 3/6] COMMIT-2026_09_02-16_58_48 --- src/test/unit/inputfilter_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/unit/inputfilter_test.go b/src/test/unit/inputfilter_test.go index ac781d3..b0ca31d 100644 --- a/src/test/unit/inputfilter_test.go +++ b/src/test/unit/inputfilter_test.go @@ -105,11 +105,14 @@ func TestInputFiltersDeliverModifiedKeys(t *testing.T) { want: "", }, { - // Shift is already encoded in the glyph ("A" not "a"), so - // an extra "S-" would make Nvim see a different key. + // Ctrl folds a letter to a control byte, which discards case: + // "" and "" are the same byte. So Ctrl+Shift has + // nowhere to carry the Shift except an explicit "S-" prefix. + // (Alt differs -- the shifted glyph itself encodes Shift.) + // See EncodeKey in internal/input/keymap.go. desc: "ctrl-shift combination", ev: key.Event{Name: "A", Modifiers: key.ModCtrl | key.ModShift, State: key.Press}, - want: "", + want: "", }, { desc: "named key with a modifier", From e3cacc1e199b5a1192c9c0ed4b1d2698a76b3e17 Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 17:47:30 -0700 Subject: [PATCH 4/6] COMMIT-2026_09_02-17_47_30 --- src/internal/app/openfile_darwin.go | 72 ++----------------------- src/internal/app/openfile_darwin.m | 83 +++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 67 deletions(-) create mode 100644 src/internal/app/openfile_darwin.m diff --git a/src/internal/app/openfile_darwin.go b/src/internal/app/openfile_darwin.go index 4bd233d..ff43041 100644 --- a/src/internal/app/openfile_darwin.go +++ b/src/internal/app/openfile_darwin.go @@ -6,73 +6,11 @@ package editorapp #cgo CFLAGS: -x objective-c -fmodules -fobjc-arc #cgo LDFLAGS: -framework Cocoa -#import - -void snv_onOpenFile(char *path); - -// SNVOpenFileHandler receives the 'odoc' (kAEOpenDocuments) Apple Event, -// which is what Finder sends when a file is opened with this app. -// -// Why an Apple Event handler rather than the NSApplicationDelegate method: -// Gio owns the delegate (GioAppDelegate in os_macos.m) and implements only -// application:openURLs:, which fires for registered URL *schemes*, not for -// file opens. Replacing Gio's delegate would fight the toolkit for -// ownership and break on upgrade. Registering directly with -// NSAppleEventManager is additive and leaves Gio untouched: AppKit's -// built-in 'odoc' handler is merely what would otherwise forward to -// application:openFile:, so claiming that one event changes nothing else. -@interface SNVOpenFileHandler : NSObject -@end - -@implementation SNVOpenFileHandler - -- (void)handleOpenDocs:(NSAppleEventDescriptor *)event - withReplyEvent:(NSAppleEventDescriptor *)reply { - NSAppleEventDescriptor *list = [event paramDescriptorForKeyword:keyDirectObject]; - if (list == nil) { - return; - } - // Apple Event descriptor lists are 1-based. - for (NSInteger i = 1; i <= [list numberOfItems]; i++) { - NSAppleEventDescriptor *item = [list descriptorAtIndex:i]; - NSString *path = nil; - - // Finder sends typeFileURL; older senders use an alias/FSRef, - // which coercing to typeFileURL normalises. - NSAppleEventDescriptor *urlDesc = [item coerceToDescriptorType:typeFileURL]; - if (urlDesc != nil) { - NSString *s = [[NSString alloc] initWithData:[urlDesc data] - encoding:NSUTF8StringEncoding]; - path = [[NSURL URLWithString:s] path]; - } - if (path == nil) { - path = [item stringValue]; - } - if (path != nil) { - snv_onOpenFile((char *)[path UTF8String]); - } - } -} - -@end - -static SNVOpenFileHandler *snvHandler = nil; - -void snv_install_open_file_handler(void) { - // NSAppleEventManager is not thread-safe and must be registered - // against the main run loop. - dispatch_async(dispatch_get_main_queue(), ^{ - if (snvHandler != nil) { - return; - } - snvHandler = [[SNVOpenFileHandler alloc] init]; - [[NSAppleEventManager sharedAppleEventManager] - setEventHandler:snvHandler - andSelector:@selector(handleOpenDocs:withReplyEvent:) - forEventClass:kCoreEventClass - andEventID:kAEOpenDocuments]; - }); -} +// Declarations only. The implementation lives in openfile_darwin.m -- +// this preamble is prepended to every translation unit cgo generates for +// the package, so defining the class or function here would compile them +// more than once and fail the link with duplicate symbols. +void snv_install_open_file_handler(void); */ import "C" diff --git a/src/internal/app/openfile_darwin.m b/src/internal/app/openfile_darwin.m new file mode 100644 index 0000000..7012ca3 --- /dev/null +++ b/src/internal/app/openfile_darwin.m @@ -0,0 +1,83 @@ +// Apple Event handler for Finder's "open document" ('odoc') event. +// +// This lives in a real .m file rather than in the cgo preamble of +// openfile_darwin.go. A preamble is textually prepended to *every* C +// translation unit cgo generates for that package, so any function or ObjC +// class *defined* (not merely declared) there is compiled more than once and +// the link fails with duplicate symbols: +// +// duplicate symbol '_OBJC_CLASS_$_SNVOpenFileHandler' +// duplicate symbol '_snv_install_open_file_handler' +// +// That is guaranteed to happen once the same file also uses //export, because +// cgo then emits an extra translation unit for the exported thunks. The rule +// is: preambles declare, .m/.c files define. + +#import + +// Implemented in Go (openfile_darwin.go, //export snv_onOpenFile). +void snv_onOpenFile(char *path); + +// SNVOpenFileHandler receives the 'odoc' (kAEOpenDocuments) Apple Event, +// which is what Finder sends when a file is opened with this app. +// +// Why an Apple Event handler rather than the NSApplicationDelegate method: +// Gio owns the delegate (GioAppDelegate in os_macos.m) and implements only +// application:openURLs:, which fires for registered URL *schemes*, not for +// file opens. Replacing Gio's delegate would fight the toolkit for +// ownership and break on upgrade. Registering directly with +// NSAppleEventManager is additive and leaves Gio untouched: AppKit's +// built-in 'odoc' handler is merely what would otherwise forward to +// application:openFile:, so claiming that one event changes nothing else. +@interface SNVOpenFileHandler : NSObject +@end + +@implementation SNVOpenFileHandler + +- (void)handleOpenDocs:(NSAppleEventDescriptor *)event + withReplyEvent:(NSAppleEventDescriptor *)reply { + NSAppleEventDescriptor *list = [event paramDescriptorForKeyword:keyDirectObject]; + if (list == nil) { + return; + } + // Apple Event descriptor lists are 1-based. + for (NSInteger i = 1; i <= [list numberOfItems]; i++) { + NSAppleEventDescriptor *item = [list descriptorAtIndex:i]; + NSString *path = nil; + + // Finder sends typeFileURL; older senders use an alias/FSRef, + // which coercing to typeFileURL normalises. + NSAppleEventDescriptor *urlDesc = [item coerceToDescriptorType:typeFileURL]; + if (urlDesc != nil) { + NSString *s = [[NSString alloc] initWithData:[urlDesc data] + encoding:NSUTF8StringEncoding]; + path = [[NSURL URLWithString:s] path]; + } + if (path == nil) { + path = [item stringValue]; + } + if (path != nil) { + snv_onOpenFile((char *)[path UTF8String]); + } + } +} + +@end + +static SNVOpenFileHandler *snvHandler = nil; + +void snv_install_open_file_handler(void) { + // NSAppleEventManager is not thread-safe and must be registered + // against the main run loop. + dispatch_async(dispatch_get_main_queue(), ^{ + if (snvHandler != nil) { + return; + } + snvHandler = [[SNVOpenFileHandler alloc] init]; + [[NSAppleEventManager sharedAppleEventManager] + setEventHandler:snvHandler + andSelector:@selector(handleOpenDocs:withReplyEvent:) + forEventClass:kCoreEventClass + andEventID:kAEOpenDocuments]; + }); +} From 8cd825f3f51ccaeb699e2872ca61f834f53fb6c6 Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 18:02:06 -0700 Subject: [PATCH 5/6] COMMIT-2026_09_02-18_02_05 --- src/internal/app/openfile_test.go | 126 +++++++++++++++++++++++++ src/internal/nvimproc/openfile_test.go | 73 ++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 src/internal/app/openfile_test.go create mode 100644 src/internal/nvimproc/openfile_test.go diff --git a/src/internal/app/openfile_test.go b/src/internal/app/openfile_test.go new file mode 100644 index 0000000..4168e5b --- /dev/null +++ b/src/internal/app/openfile_test.go @@ -0,0 +1,126 @@ +package editorapp + +import ( + "fmt" + "sync" + "testing" +) + +// resetPendingOpens clears global queue state so tests don't leak into +// each other. +func resetPendingOpens(t *testing.T) { + t.Helper() + takeQueuedOpens() + t.Cleanup(func() { takeQueuedOpens() }) +} + +func TestQueueOpenFileRoundTrips(t *testing.T) { + resetPendingOpens(t) + + queueOpenFile("/tmp/a.txt") + queueOpenFile("/tmp/b.txt") + + got := takeQueuedOpens() + want := []string{"/tmp/a.txt", "/tmp/b.txt"} + if len(got) != len(want) { + t.Fatalf("takeQueuedOpens() = %v, want %v", got, want) + } + // Order matters: opening several files should edit them in the order + // the desktop environment listed them. + for i := range want { + if got[i] != want[i] { + t.Errorf("path %d = %q, want %q", i, got[i], want[i]) + } + } +} + +// TestQueueOpenFileIgnoresEmpty guards the queue against a path the +// platform failed to decode: an empty string would become a bare ":edit". +func TestQueueOpenFileIgnoresEmpty(t *testing.T) { + resetPendingOpens(t) + + queueOpenFile("") + if got := takeQueuedOpens(); got != nil { + t.Errorf("takeQueuedOpens() = %v, want nil", got) + } +} + +// TestTakeQueuedOpensDrains verifies the queue is emptied by a read, so a +// file is opened exactly once rather than on every subsequent frame. +func TestTakeQueuedOpensDrains(t *testing.T) { + resetPendingOpens(t) + + queueOpenFile("/tmp/a.txt") + if got := takeQueuedOpens(); len(got) != 1 { + t.Fatalf("first take = %v, want 1 path", got) + } + if got := takeQueuedOpens(); got != nil { + t.Errorf("second take = %v, want nil (queue should be drained)", got) + } +} + +// TestQueueOpenFileIsThreadSafe exercises the reason the queue exists. +// +// The platform delivers paths on its own thread (the AppKit main thread on +// macOS) while the editor drains them from Gio's event loop. Run with +// -race, this asserts those two sides cannot corrupt the slice. +func TestQueueOpenFileIsThreadSafe(t *testing.T) { + resetPendingOpens(t) + + const writers, perWriter = 8, 50 + + var wg sync.WaitGroup + wg.Add(writers) + for w := 0; w < writers; w++ { + go func(w int) { + defer wg.Done() + for i := 0; i < perWriter; i++ { + queueOpenFile(fmt.Sprintf("/tmp/%d-%d.txt", w, i)) + } + }(w) + } + + // Drain concurrently with the writers, collecting as we go. + done := make(chan int) + go func() { + seen := 0 + for { + seen += len(takeQueuedOpens()) + select { + case <-done: + done <- seen + len(takeQueuedOpens()) + return + default: + } + } + }() + + wg.Wait() + done <- 0 + total := <-done + + if want := writers * perWriter; total != want { + t.Errorf("collected %d paths, want %d (none may be lost or duplicated)", total, want) + } +} + +// TestDrainOpenRequestsWithoutNvimKeepsPaths is the regression test for +// opening an empty editor. +// +// On macOS the Apple Event naming the file arrives during launch, before +// Nvim has been spawned. If drain discarded the queue while a.proc was +// nil, the file that caused the launch would be silently dropped -- the +// app would come up blank. +func TestDrainOpenRequestsWithoutNvimKeepsPaths(t *testing.T) { + resetPendingOpens(t) + + queueOpenFile("/tmp/launch.txt") + + a := &App{} // proc is nil: Nvim has not started yet + a.drainOpenRequests() + + got := takeQueuedOpens() + if len(got) != 1 || got[0] != "/tmp/launch.txt" { + t.Errorf("after draining with no Nvim, queue = %v, want the path still pending", got) + } +} diff --git a/src/internal/nvimproc/openfile_test.go b/src/internal/nvimproc/openfile_test.go new file mode 100644 index 0000000..bc4edac --- /dev/null +++ b/src/internal/nvimproc/openfile_test.go @@ -0,0 +1,73 @@ +package nvimproc + +import "testing" + +// TestVimEscapeQuotesSingleQuotes is the injection guard for filenames. +// +// A path is untrusted input that ends up on Nvim's command line. Inside a +// single-quoted Vim string the only metacharacter is the quote itself, +// which is escaped by doubling; get that wrong and a file named +// `'|qall!|'` would terminate the string and run whatever follows as +// commands. +func TestVimEscapeQuotesSingleQuotes(t *testing.T) { + cases := []struct { + name string + path string + want string + }{ + { + name: "plain path", + path: "/tmp/notes.txt", + want: "`=fnameescape('/tmp/notes.txt')`", + }, + { + name: "spaces are left to fnameescape", + path: "/tmp/my notes.txt", + want: "`=fnameescape('/tmp/my notes.txt')`", + }, + { + name: "single quote is doubled", + path: "/tmp/it's.txt", + want: "`=fnameescape('/tmp/it''s.txt')`", + }, + { + name: "command injection attempt stays inside the string", + path: "/tmp/'|qall!|'.txt", + want: "`=fnameescape('/tmp/''|qall!|''.txt')`", + }, + { + name: "percent and hash are left to fnameescape", + path: "/tmp/100%_#1.txt", + want: "`=fnameescape('/tmp/100%_#1.txt')`", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := vimEscape(tc.path); got != tc.want { + t.Errorf("vimEscape(%q) = %q, want %q", tc.path, got, tc.want) + } + }) + } +} + +// TestOpenFileIgnoresEmptyPath ensures an empty request never reaches Nvim, +// where ":edit" with no argument would reload the current buffer. +func TestOpenFileIgnoresEmptyPath(t *testing.T) { + p := &Process{cmds: make(chan func(), 1)} + p.OpenFile("") + if len(p.cmds) != 0 { + t.Errorf("OpenFile(\"\") queued %d commands, want 0", len(p.cmds)) + } +} + +// TestOpenFileQueuesCommand verifies the request is enqueued on the same +// serialized channel as every other outgoing call, so it cannot race ahead +// of pending input. +func TestOpenFileQueuesCommand(t *testing.T) { + p := &Process{cmds: make(chan func(), 1)} + p.OpenFile("/tmp/x.txt") + if len(p.cmds) != 1 { + t.Fatalf("OpenFile queued %d commands, want 1", len(p.cmds)) + } +} From 371db2e7df99a81fa81b6adbae38378c22442d7b Mon Sep 17 00:00:00 2001 From: Kevin Gao Date: Wed, 2 Sep 2026 18:11:55 -0700 Subject: [PATCH 6/6] COMMIT-2026_09_02-18_11_55 --- src/test/unit/windows_test.go | 55 +++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/test/unit/windows_test.go b/src/test/unit/windows_test.go index 261ccf3..e1dbedc 100644 --- a/src/test/unit/windows_test.go +++ b/src/test/unit/windows_test.go @@ -149,6 +149,61 @@ func TestMsgSetPosRowNegativeOneHidesTheGrid(t *testing.T) { } } +func TestHitTestFindsGridContainingPosition(t *testing.T) { + windows := []uistate.Placement{ + {GridID: 2, Row: 0, Col: 0, Width: 40, Height: 24}, + } + grid, gridRow, gridCol, ok := uistate.HitTest(windows, 5, 10) + if !ok { + t.Fatalf("HitTest missed a position inside the only window") + } + if grid != 2 || gridRow != 5 || gridCol != 10 { + t.Fatalf("HitTest = (grid=%d, row=%d, col=%d), want (2, 5, 10)", grid, gridRow, gridCol) + } +} + +func TestHitTestTranslatesToGridRelativeCoordinates(t *testing.T) { + // Window starts at (3, 4); a click at (7, 12) should land at (4, 8) + // relative to the grid's own origin. + windows := []uistate.Placement{ + {GridID: 5, Row: 3, Col: 4, Width: 20, Height: 20}, + } + grid, gridRow, gridCol, ok := uistate.HitTest(windows, 7, 12) + if !ok || grid != 5 || gridRow != 4 || gridCol != 8 { + t.Fatalf("HitTest = (grid=%d, row=%d, col=%d, ok=%v), want (5, 4, 8, true)", grid, gridRow, gridCol, ok) + } +} + +func TestHitTestReturnsFalseWhenNothingContainsPosition(t *testing.T) { + windows := []uistate.Placement{ + {GridID: 2, Row: 0, Col: 0, Width: 10, Height: 10}, + } + // Just past the bottom-right corner: row/col == Row+Height / Col+Width + // is exclusive, so this must miss. + _, _, _, ok := uistate.HitTest(windows, 10, 10) + if ok { + t.Fatalf("HitTest matched a position exactly on the window's exclusive edge") + } + + _, _, _, ok = uistate.HitTest(nil, 0, 0) + if ok { + t.Fatalf("HitTest matched against an empty window list") + } +} + +func TestHitTestPrefersTopmostOverlappingWindow(t *testing.T) { + // Both windows cover (5,5). ordered() places the topmost window last, + // so HitTest -- which walks backwards -- must return the second one. + windows := []uistate.Placement{ + {GridID: 2, Row: 0, Col: 0, Width: 20, Height: 20}, + {GridID: 3, Row: 0, Col: 0, Width: 20, Height: 20}, + } + grid, _, _, ok := uistate.HitTest(windows, 5, 5) + if !ok || grid != 3 { + t.Fatalf("HitTest = (grid=%d, ok=%v), want the topmost (last) grid 3", grid, ok) + } +} + func TestUpsertPreservesZIndexAcrossUpdates(t *testing.T) { s := uistate.New() s.Apply(batch(