Enable directory recursion in Dir()

[BiggerNoise]

Interface is as you described in issue #10. Because it doesn't have access to the logger supplied to reload.Do(), it's just silently skipping directories that encounter a read error.

Since the actual directories monitored are listed by do(), I didn't think this was that big a deal, but happy to try something else if you'd like these errors to be surfaced.

Thanks for the library, it's working really well for me!

[arp242]

Thanks!

Ideally what I'd like is for this is for Dir() to be recursive when the path ends with /... or \...:

// Current non-recursive
reload.Do(log.Print, cb, reload.Dir("./tpl", reloadTpl)...)

// Recursive
reload.Do(log.Print, cb, reload.Dir("./tpl/...", reloadTpl)...)

IMHO this is a nicer API, and will also be the syntax the fsnotify library will use once recursion will be supported.

I know that in my previous comment I mentioned reload.ListDirs() like this, but that was a few years ago, and I didn't really think that deeply about it then 😅

[BiggerNoise]

Nope. You're irrevocably committed to your position from three years ago. No changing your mind allowed. :)

Let me see if I can whip something up.

[BiggerNoise]

Have a look at this. Now able to log when elements are skipped due to inability to read the directory. I do think this is the right thing to do, but certainly willing to discuss.

[BiggerNoise]

Shoot, just realized that you had an ellipsis after your slash. Let me rework this a bit

[BiggerNoise]

OK, now it should be good to go.

[arp242]

One thing this won't do is watch for new directories; for example if I use ./tpl/... and something created ./tpl/new-dir, then changes to ./tpl/new-dir/file.gohtml won't trigger anything.

To add this, you'll have to check if something is a directory on a fsnotify event and add that too, in the case event := <-watcher.Events:

if !slices.Contains(dirs, event.Name) {
	s, err := os.Stat(event.Name)
	if err != nil && !errors.Is(err, os.ErrNotExist) {
		log("...")
	}
	if s.IsDir() {
		err = watcher.Add(event.Name)
		// add to dirs, timers, maybe need to normalize?
	}
}

This kind of thing kind of becomes too complex and subtle to add without a test though; adding a test for "additional directories" isn't too hard; here's a test that works for the current master:

func TestAdditional(t *testing.T) {
	var (
		nonRecur = t.TempDir()
		recur    = t.TempDir()
		n        atomic.Int32
		w        = make(chan struct{})
		f        = func() { n.Add(1); w <- struct{}{} }
		wait     = func(t *testing.T, want int) {
			t.Helper()
			to := time.NewTimer(400 * time.Millisecond)
			start := n.Load()
			select {
			case <-to.C:
				if want != -1 {
					t.Fatal("timeout")
				}
				if nn := n.Load(); nn != int32(start) {
					t.Fatalf("counter changed from %d to %d while it shouldn't", start, nn)
				}
			case <-w:
				if nn := n.Load(); nn != int32(want) {
					t.Fatalf("wrong counter: %d; want %d", nn, want)
				}
				return
			}
		}
	)
	if err := os.WriteFile(filepath.Join(nonRecur, "/file"), []byte("xxx"), 0o644); err != nil {
		t.Fatal(err)
	}
	if err := os.MkdirAll(filepath.Join(nonRecur, "/dir1/dir2/dir3"), 0o755); err != nil {
		t.Fatal(err)
	}
	if err := os.WriteFile(filepath.Join(recur, "/file"), []byte("xxx"), 0o644); err != nil {
		t.Fatal(err)
	}
	if err := os.MkdirAll(filepath.Join(recur, "/dir1/dir2/dir3"), 0o755); err != nil {
		t.Fatal(err)
	}

	go func() {
		err := Do(
			func(f string, a ...interface{}) { w <- struct{}{}; log.Printf(f, a...) },
			Dir(nonRecur, f),
			Dir(filepath.Join(recur, "/..."), f),
		)
		if err != nil {
			panic(err)
		}
	}()
	wait(t, 0)

	// Non-recursive
	{
		// New file
		if err := os.WriteFile(filepath.Join(nonRecur, "/test"), []byte("xxx"), 0o644); err != nil {
			t.Fatal(err)
		}
		wait(t, 1)

		// Change new.
		fp, err := os.Create(filepath.Join(nonRecur, "/test"))
		if err != nil {
			t.Fatal(err)
		}
		if _, err := fp.WriteString("qqq"); err != nil {
			t.Fatal(err)
		}
		if err := fp.Close(); err != nil {
			t.Fatal(err)
		}
		wait(t, 2)

		// Change existing
		fp, err = os.Create(filepath.Join(nonRecur, "/file"))
		if err != nil {
			t.Fatal(err)
		}
		if _, err := fp.WriteString("qqq"); err != nil {
			t.Fatal(err)
		}
		if err := fp.Close(); err != nil {
			t.Fatal(err)
		}
		wait(t, 3)

		// Not recursive.
		if err := os.WriteFile(filepath.Join(nonRecur, "/dir1/test"), []byte("xxx"), 0o644); err != nil {
			t.Fatal(err)
		}
		wait(t, -1)
	}

	n.Store(0)

	// Recursive
	{
		// New file
		if err := os.WriteFile(filepath.Join(recur, "/test"), []byte("xxx"), 0o644); err != nil {
			t.Fatal(err)
		}
		wait(t, 1)

		// Change new.
		fp, err := os.Create(filepath.Join(recur, "/test"))
		if err != nil {
			t.Fatal(err)
		}
		if _, err := fp.WriteString("qqq"); err != nil {
			t.Fatal(err)
		}
		if err := fp.Close(); err != nil {
			t.Fatal(err)
		}
		wait(t, 2)

		// Change existing
		fp, err = os.Create(filepath.Join(recur, "/file"))
		if err != nil {
			t.Fatal(err)
		}
		if _, err := fp.WriteString("qqq"); err != nil {
			t.Fatal(err)
		}
		if err := fp.Close(); err != nil {
			t.Fatal(err)
		}
		wait(t, 3)

		// Not recursive.
		if err := os.WriteFile(filepath.Join(recur, "/dir1/test"), []byte("xxx"), 0o644); err != nil {
			t.Fatal(err)
		}
		// We get an event for both the directory and file, so wait twice.
		wait(t, 4)
		wait(t, 5)
	}

	if err := closeWatcher(); err != nil {
		t.Fatal(err)
	}
	wait(t, -1)
}

Just add new testcases for new directories and such at the bottom and test with wait() (pass -1 if you expect no change).

[arp242]

Suggested change

	return
	return result

[arp242]

IMHO it's better to return an error on this (from reload.Do()), rather than skipping it. It's on startup, and skipping directories (even with a print) is somewhat confusing, IMHO.

[arp242]

You'll need to update the comment on the Dir() function as well.