diff --git a/tools/watch/api.go b/tools/watch/api.go index 1af77eb21..613651536 100644 --- a/tools/watch/api.go +++ b/tools/watch/api.go @@ -81,24 +81,44 @@ func get_unique_directories(paths []string) []string { return result } +// resolve_path resolves symlinks in the directory component of path so the result +// matches what the OS-level file-system watcher (FSEvents on macOS, inotify on Linux) +// reports. This is important on macOS where /tmp is a symlink to /private/tmp. +// If the directory cannot be resolved (e.g. it doesn't exist yet) the path is +// returned cleaned but unresolved. +func resolve_path(path string) string { + dir := filepath.Dir(path) + if resolved, err := filepath.EvalSymlinks(dir); err == nil { + return filepath.Join(resolved, filepath.Base(path)) + } + return filepath.Clean(path) +} + func get_set_of_config_files(config_paths []string) *utils.Set[string] { cp := config.ConfigParser{ AllIncludedFiles: utils.NewSet[string](), LineHandler: func(k, v string) error { return nil }} cp.ParseFiles(config_paths...) + // Resolve symlinks in all paths collected by the parser (important on macOS + // where /tmp -> /private/tmp causes mismatches with FSEvents-reported paths). + result := utils.NewSet[string](cp.AllIncludedFiles.Len() + len(config_paths)*4) + for _, p := range cp.AllIncludedFiles.AsSlice() { + result.Add(resolve_path(p)) + } for _, path := range config_paths { - path = filepath.Clean(path) - cp.AllIncludedFiles.Add(path) + path = resolve_path(path) + result.Add(path) + dir := filepath.Dir(path) for _, q := range []string{"dark-theme.auto.conf", "light-theme.auto.conf", "no-preference-theme.auto.conf"} { - q = filepath.Join(filepath.Dir(path), q) - cp.AllIncludedFiles.Add(filepath.Clean(q)) + result.Add(resolve_path(filepath.Join(dir, q))) } } - return cp.AllIncludedFiles + return result } -func watch_for_kitty_config_changes(action func() error, debounce_time time.Duration, config_paths []string) error { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() +// watch_for_config_changes watches the directories derived from config_paths and calls action +// whenever a watched config file (including includes and auto color scheme files) changes. +// It runs until ctx is cancelled. +func watch_for_config_changes(ctx context.Context, action func() error, debounce_time time.Duration, config_paths []string) error { event_chan := make(chan fswatcher.WatchEvent) all_paths := get_set_of_config_files(config_paths) dirs_to_watch := get_unique_directories(all_paths.AsSlice()) @@ -108,7 +128,7 @@ func watch_for_kitty_config_changes(action func() error, debounce_time time.Dura filtered_action := func(ev fswatcher.WatchEvent) error { all_paths := get_set_of_config_files(config_paths) - if all_paths.Has(filepath.Clean(ev.Path)) { + if all_paths.Has(resolve_path(ev.Path)) { return action() } return nil @@ -117,25 +137,30 @@ func watch_for_kitty_config_changes(action func() error, debounce_time time.Dura if err := watch_dirs(ctx, dirs_to_watch, debounce_time, event_chan); err != nil { return err } - stdinClosed := make(chan struct{}) - go func() { - scanner := bufio.NewScanner(os.Stdin) - for scanner.Scan() { - } - close(stdinClosed) - }() for { select { case event := <-event_chan: if err := filtered_action(event); err != nil { fmt.Fprintf(os.Stderr, "failed to signal kitty in event: %s with error: %s\n", event, err) } - case <-stdinClosed: + case <-ctx.Done(): return nil } } } +func watch_for_kitty_config_changes(action func() error, debounce_time time.Duration, config_paths []string) error { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + } + cancel() + }() + return watch_for_config_changes(ctx, action, debounce_time, config_paths) +} + func signal_kitty_to_reload_config(kitty_pid int) error { return unix.Kill(kitty_pid, unix.SIGUSR1) } diff --git a/tools/watch/api_test.go b/tools/watch/api_test.go new file mode 100644 index 000000000..76544f251 --- /dev/null +++ b/tools/watch/api_test.go @@ -0,0 +1,285 @@ +// License: GPLv3 Copyright: 2025, Kovid Goyal, + +package watch + +import ( + "context" + "fmt" + "os" + "path/filepath" + "sort" + "sync/atomic" + "testing" + "time" + + "github.com/google/go-cmp/cmp" +) + +func write_file(t *testing.T, path string, data string) { + t.Helper() + if err := os.WriteFile(path, []byte(data), 0o600); err != nil { + t.Fatal(err) + } +} + +func TestGetUniqueDirectories(t *testing.T) { + // Empty input + if result := get_unique_directories(nil); result != nil { + t.Fatalf("Expected nil for empty input, got %v", result) + } + + type tc struct { + name string + input []string + expect []string + } + + cases := []tc{ + { + name: "single file", + input: []string{"/a/b/file.conf"}, + expect: []string{"/a/b"}, + }, + { + name: "files in same directory", + input: []string{"/a/b/file1.conf", "/a/b/file2.conf"}, + expect: []string{"/a/b"}, + }, + { + name: "sibling directories", + input: []string{"/a/b/file.conf", "/a/c/file.conf"}, + expect: []string{"/a/b", "/a/c"}, + }, + { + name: "subdirectory is excluded", + // /a/b is a subdirectory of /a, so only /a should be in results + input: []string{"/a/file.conf", "/a/b/file.conf"}, + expect: []string{"/a"}, + }, + { + name: "deeply nested subdirectory is excluded", + input: []string{ + "/a/file.conf", + "/a/b/file.conf", + "/a/b/c/file.conf", + "/x/y/file.conf", + }, + expect: []string{"/a", "/x/y"}, + }, + { + name: "duplicate parent directories deduplicated", + input: []string{ + "/a/b/file1.conf", + "/a/b/file2.conf", + "/a/b/sub/file3.conf", + }, + expect: []string{"/a/b"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := get_unique_directories(tc.input) + sort.Strings(result) + sort.Strings(tc.expect) + if diff := cmp.Diff(tc.expect, result); diff != "" { + t.Fatalf("get_unique_directories mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestGetSetOfConfigFiles(t *testing.T) { + tdir := resolve_path(t.TempDir()) + subdir := filepath.Join(tdir, "sub") + if err := os.Mkdir(subdir, 0o700); err != nil { + t.Fatal(err) + } + + main_conf := filepath.Join(tdir, "kitty.conf") + included_conf := filepath.Join(subdir, "included.conf") + + // Main config that includes another file + write_file(t, main_conf, "include sub/included.conf\nfont_size 12\n") + write_file(t, included_conf, "background black\n") + + result := get_set_of_config_files([]string{main_conf}) + + // Must contain the main config file + if !result.Has(main_conf) { + t.Errorf("Expected set to contain main config %q", main_conf) + } + // Must contain the included file + if !result.Has(included_conf) { + t.Errorf("Expected set to contain included config %q", included_conf) + } + // Must contain auto color scheme files (even if they don't exist) + for _, name := range []string{"dark-theme.auto.conf", "light-theme.auto.conf", "no-preference-theme.auto.conf"} { + expected := filepath.Join(tdir, name) + if !result.Has(expected) { + t.Errorf("Expected set to contain auto color scheme file %q", expected) + } + } +} + +// wait_for_count waits until counter reaches at least target or timeout fires. +// Returns the count observed. +func wait_for_count(counter *atomic.Int32, target int32, timeout time.Duration) int32 { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + if counter.Load() >= target { + return counter.Load() + } + time.Sleep(10 * time.Millisecond) + } + return counter.Load() +} + +func TestWatchForConfigChanges(t *testing.T) { + tdir := t.TempDir() + subdir := filepath.Join(tdir, "sub") + if err := os.Mkdir(subdir, 0o700); err != nil { + t.Fatal(err) + } + + main_conf := filepath.Join(tdir, "kitty.conf") + included_conf := filepath.Join(subdir, "included.conf") + dark_theme := filepath.Join(tdir, "dark-theme.auto.conf") + unrelated := filepath.Join(tdir, "unrelated.txt") + + write_file(t, main_conf, "include sub/included.conf\n") + write_file(t, included_conf, "background black\n") + write_file(t, dark_theme, "background #000000\n") + write_file(t, unrelated, "this file should not trigger action\n") + + var action_count atomic.Int32 + action := func() error { + action_count.Add(1) + return nil + } + + const debounce = 50 * time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan error, 1) + go func() { + done <- watch_for_config_changes(ctx, action, debounce, []string{main_conf}) + }() + + // Give the watcher time to start + time.Sleep(200 * time.Millisecond) + + t.Run("main config change triggers action", func(t *testing.T) { + before := action_count.Load() + write_file(t, main_conf, "include sub/included.conf\nfont_size 13\n") + count := wait_for_count(&action_count, before+1, 2*time.Second) + if count <= before { + t.Fatalf("Expected action to be called after main config change, count=%d", count) + } + }) + + t.Run("included file change triggers action", func(t *testing.T) { + before := action_count.Load() + write_file(t, included_conf, "background white\n") + count := wait_for_count(&action_count, before+1, 2*time.Second) + if count <= before { + t.Fatalf("Expected action to be called after included file change, count=%d", count) + } + }) + + t.Run("auto color scheme file change triggers action", func(t *testing.T) { + before := action_count.Load() + write_file(t, dark_theme, "background #111111\n") + count := wait_for_count(&action_count, before+1, 2*time.Second) + if count <= before { + t.Fatalf("Expected action to be called after dark-theme.auto.conf change, count=%d", count) + } + }) + + t.Run("unrelated file change does not trigger action", func(t *testing.T) { + before := action_count.Load() + write_file(t, unrelated, "still unrelated\n") + // Wait debounce + a bit more to ensure no spurious call + time.Sleep(debounce + 200*time.Millisecond) + after := action_count.Load() + if after != before { + t.Fatalf("Expected action NOT to be called for unrelated file, count went from %d to %d", before, after) + } + }) + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("watch_for_config_changes returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("watch_for_config_changes did not exit after context cancel") + } +} + +func TestWatchForConfigChangesDebounce(t *testing.T) { + tdir := t.TempDir() + main_conf := filepath.Join(tdir, "kitty.conf") + write_file(t, main_conf, "font_size 12\n") + + var action_count atomic.Int32 + action := func() error { + action_count.Add(1) + return nil + } + + const debounce = 300 * time.Millisecond + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + done := make(chan error, 1) + go func() { + done <- watch_for_config_changes(ctx, action, debounce, []string{main_conf}) + }() + + // Give the watcher time to start + time.Sleep(200 * time.Millisecond) + + // Write to the file several times rapidly within the debounce window. + // The fswatcher debouncer drops events that occur within the cooldown period + // after the first event, so only the first write should produce an action call. + for i := 0; i < 5; i++ { + write_file(t, main_conf, fmt.Sprintf("font_size %d\n", 12+i)) + time.Sleep(20 * time.Millisecond) + } + + // Wait for up to one full debounce period for the first action to fire + count_after_burst := wait_for_count(&action_count, 1, debounce+500*time.Millisecond) + + // Debouncing should have collapsed the burst into at most 2 calls. + // The fswatcher debouncer uses leading-edge logic: the first event fires + // immediately and subsequent events within the cooldown window are dropped. + // A trailing event may fire at the end of the cooldown window, giving at most 2. + if count_after_burst == 0 { + t.Fatalf("Expected at least one action call after burst of writes, got 0") + } + if count_after_burst > 2 { + t.Fatalf("Expected debouncing to collapse burst: want ≤2 calls, got %d", count_after_burst) + } + + // After waiting well past the debounce window, a new write should trigger exactly one more call. + time.Sleep(debounce + 100*time.Millisecond) + before_new := action_count.Load() + write_file(t, main_conf, "font_size 20\n") + count_new := wait_for_count(&action_count, before_new+1, 2*time.Second) + if count_new <= before_new { + t.Fatalf("Expected action after post-debounce write, count went from %d to %d", before_new, count_new) + } + + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("watch_for_config_changes returned error: %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("watch_for_config_changes did not exit after context cancel") + } +}