Skip to content

Commit

Permalink
Fix file watching in pixlet serve (#1068)
Browse files Browse the repository at this point in the history
File watching was not working when using `pixlet serve` for a file in
the working directory, or for `pixlet serve .`. This was due to a bug in
how we handle file paths in the watcher.

Fixes #1067.
  • Loading branch information
rohansingh committed May 2, 2024
1 parent e09293d commit e41032e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 12 deletions.
6 changes: 6 additions & 0 deletions cmd/serve.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cmd

import (
"fmt"

"github.com/spf13/cobra"

"tidbyt.dev/pixlet/server"
Expand Down Expand Up @@ -33,6 +35,10 @@ containing multiple Starlark files and resources.`,
}

func serve(cmd *cobra.Command, args []string) error {
if watch && cmd.Flags().Changed("watch") {
fmt.Printf("explicitly setting --watch is unnecessary, since it's the default\n\n")
}

s, err := server.NewServer(host, port, watch, args[0], maxDuration, timeout)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewServer(host string, port int, watch bool, path string, maxDuration int,
// check if path exists, and whether it is a directory or a file
info, err := os.Stat(path)
if err != nil {
return nil, fmt.Errorf("failed to stat %s: %w", path, err)
return nil, fmt.Errorf("stat'ing %s: %w", path, err)
}

var fs fs.FS
Expand All @@ -43,7 +43,7 @@ func NewServer(host string, port int, watch bool, path string, maxDuration int,
}

fs = tools.NewSingleFileFS(path)
w = NewWatcher(filepath.Dir(path), fileChanges)
w = NewWatcher(path, fileChanges)
}

updatesChan := make(chan loader.Update, 100)
Expand Down
35 changes: 25 additions & 10 deletions server/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ package server

import (
"fmt"
"log"
"os"
"path/filepath"
"strings"

"github.com/fsnotify/fsnotify"
)
Expand All @@ -29,37 +28,53 @@ func NewWatcher(filename string, fileChanges chan bool) *Watcher {
// If there is an error, it's returned. It's up to the caller to respawn the
// watcher if it's desireable to keep watching.
//
// The reason it watches a directory is becausde some editers like VIM write
// The reason it watches a directory is becausde some editors like VIM write
// to a swap file and recreate the original file. So we can't simply watch the
// original file, we have to watch the directory. This is also why we check both
// the WRITE and CREATE events since VIM will write to a swap and then create
// the file on save. VSCode does a WRITE and then a CHMOD, so tracking WRITE
// catches the changes for VSCode exactly once.
func (w *Watcher) Run() error {
// check if path exists, and whether it is a directory or a file
info, err := os.Stat(w.path)
if err != nil {
return fmt.Errorf("stat'ing %s: %w", w.path, err)
}
isDir := info.IsDir()

watcher, err := fsnotify.NewWatcher()
if err != nil {
return fmt.Errorf("error watching for changes: %w", err)
return fmt.Errorf("watching for changes: %w", err)
}
defer watcher.Close()

watcher.Add(w.path)
if isDir {
watcher.Add(w.path)
} else {
watcher.Add(filepath.Dir(w.path))
}

for {
select {
case event, ok := <-watcher.Events:
if !ok {
return fmt.Errorf("something is weird with the file watcher")
return fmt.Errorf("watcher events channel closed unexpectedly")
}
log.Println(event.Name)
if strings.HasPrefix(event.Name, w.path) && shouldNotify(event.Op) {

if !isDir && event.Name != w.path {
// if we're watching a single file, ignore changes to other files
continue
}

if shouldNotify(event.Op) {
w.fileChanges <- true
}

case err, ok := <-watcher.Errors:
if !ok {
return fmt.Errorf("something is weird with the file watcher around error handling")
return fmt.Errorf("watcher errors channel closed unexpectedly")
}
return fmt.Errorf("error in file watcher: %w", err)
return fmt.Errorf("watcher: %w", err)
}
}
}
Expand Down

0 comments on commit e41032e

Please sign in to comment.