From 1c5f57fecdb28b254f25e29fa311f087a262f858 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 14:12:35 +0000 Subject: [PATCH] feat(config): recognize devbox.jsonc as a config filename (#2602) devbox.json is parsed as JSONC and supports comments, but editors and GitHub diffs flag comments in a .json file as syntax errors. Allow users to name their config devbox.jsonc so standard tooling highlights it correctly, without any per-repo configuration. - Add configfile.AltName ("devbox.jsonc") and configfile.ValidNames, and search both names during directory discovery (devbox.json wins when both are present, preserving existing behavior). - Make SaveTo preserve the config's original filename via a new FileName() helper so `devbox add` and friends write back to devbox.jsonc instead of creating a stray devbox.json. Configs without an on-disk path (e.g. pulled from a URL) still default to devbox.json. Closes #2602 --- internal/devconfig/config.go | 5 +- internal/devconfig/config_test.go | 80 ++++++++++++++++++++++ internal/devconfig/configfile/file.go | 29 +++++++- internal/devconfig/configfile/file_test.go | 51 ++++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go index 1151deb14a3..86166e790b1 100644 --- a/internal/devconfig/config.go +++ b/internal/devconfig/config.go @@ -142,8 +142,7 @@ func Find(path string) (*Config, error) { // searchDir looks for a config file in dir. It does not search parent // directories. func searchDir(dir string) (*Config, error) { - try := []string{configfile.DefaultName} - for _, name := range try { + for _, name := range configfile.ValidNames { path := filepath.Join(dir, name) slog.Debug("trying config file", "path", path) @@ -156,7 +155,7 @@ func searchDir(dir string) (*Config, error) { if errors.Is(err, os.ErrNotExist) { continue } - // Ignore directories named devbox.json. + // Ignore directories that happen to share a config filename. if errors.Is(err, errIsDirectory) { continue } diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go index 9f747796156..17f8e038d7b 100644 --- a/internal/devconfig/config_test.go +++ b/internal/devconfig/config_test.go @@ -281,6 +281,86 @@ func TestFindError(t *testing.T) { }) } +func TestJSONCConfig(t *testing.T) { + const jsonc = "{\n // devbox lets you comment your config\n \"packages\": []\n}\n" + + t.Run("OpenDiscoversJSONC", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, configfile.AltName) + if err := os.WriteFile(path, []byte(jsonc), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Open(dir) + if err != nil { + t.Fatalf("Open(%q) error: %v", dir, err) + } + if cfg.Root.AbsRootPath != path { + t.Errorf("cfg.Root.AbsRootPath = %q, want %q", cfg.Root.AbsRootPath, path) + } + }) + + t.Run("FindDiscoversJSONCInParent", func(t *testing.T) { + root, child, _ := mkNestedDirs(t) + path := filepath.Join(root, configfile.AltName) + if err := os.WriteFile(path, []byte(jsonc), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Find(child) + if err != nil { + t.Fatalf("Find(%q) error: %v", child, err) + } + if cfg.Root.AbsRootPath != path { + t.Errorf("cfg.Root.AbsRootPath = %q, want %q", cfg.Root.AbsRootPath, path) + } + }) + + t.Run("DefaultNameWinsWhenBothExist", func(t *testing.T) { + dir := t.TempDir() + jsonPath := filepath.Join(dir, configfile.DefaultName) + if err := os.WriteFile(jsonPath, []byte(`{"packages": []}`), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, configfile.AltName), []byte(jsonc), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Open(dir) + if err != nil { + t.Fatalf("Open(%q) error: %v", dir, err) + } + if cfg.Root.AbsRootPath != jsonPath { + t.Errorf("cfg.Root.AbsRootPath = %q, want %q", cfg.Root.AbsRootPath, jsonPath) + } + }) + + t.Run("SaveWritesBackToJSONC", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, configfile.AltName) + if err := os.WriteFile(path, []byte(jsonc), 0o644); err != nil { + t.Fatal(err) + } + + cfg, err := Open(dir) + if err != nil { + t.Fatalf("Open(%q) error: %v", dir, err) + } + if err := cfg.Root.SaveTo(dir); err != nil { + t.Fatalf("SaveTo(%q) error: %v", dir, err) + } + + // Saving must write back to devbox.jsonc, not create a devbox.json. + if _, err := os.Stat(filepath.Join(dir, configfile.DefaultName)); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("SaveTo created a %s; want it to update %s in place", + configfile.DefaultName, configfile.AltName) + } + if _, err := os.Stat(path); err != nil { + t.Errorf("os.Stat(%q) after save: %v", path, err) + } + }) +} + // mkNestedDirs sets up a nested directory structure for Find and Open tests. func mkNestedDirs(t *testing.T) (root, child, nested string) { t.Helper() diff --git a/internal/devconfig/configfile/file.go b/internal/devconfig/configfile/file.go index 6857f037ce7..30a43403937 100644 --- a/internal/devconfig/configfile/file.go +++ b/internal/devconfig/configfile/file.go @@ -21,8 +21,19 @@ import ( const ( DefaultName = "devbox.json" + // AltName is an alternate config filename that devbox also recognizes. + // devbox.json already permits comments (it is parsed as JSONC), but + // editors and GitHub diffs flag comments in a .json file as errors. Naming + // the file devbox.jsonc lets those tools highlight it correctly without any + // extra configuration. See https://github.com/jetify-com/devbox/issues/2602 + AltName = "devbox.jsonc" ) +// ValidNames are the config filenames devbox recognizes, in the order they are +// searched for within a directory. devbox.json is listed first so it wins when +// a directory happens to contain both files. +var ValidNames = []string{DefaultName, AltName} + // ConfigFile defines a devbox environment as JSON. type ConfigFile struct { // AbsRootPath is the absolute path to the devbox.json or plugin.json file @@ -114,9 +125,21 @@ func (c *ConfigFile) InitHook() *shellcmd.Commands { return c.Shell.InitHook } -// SaveTo writes the config to a file. -func (c *ConfigFile) SaveTo(path string) error { - return os.WriteFile(filepath.Join(path, DefaultName), c.Bytes(), 0o644) +// FileName returns the base name of the config file (e.g. "devbox.json" or +// "devbox.jsonc"). It preserves whatever name the config was loaded from so +// that saving writes back to the same file. It falls back to [DefaultName] when +// the config has no on-disk path (for example, a config loaded from a URL). +func (c *ConfigFile) FileName() string { + if c.AbsRootPath != "" { + return filepath.Base(c.AbsRootPath) + } + return DefaultName +} + +// SaveTo writes the config into the directory dir, using the config's original +// filename (see [ConfigFile.FileName]). +func (c *ConfigFile) SaveTo(dir string) error { + return os.WriteFile(filepath.Join(dir, c.FileName()), c.Bytes(), 0o644) } // TODO: Can we remove SaveTo and just use Save()? diff --git a/internal/devconfig/configfile/file_test.go b/internal/devconfig/configfile/file_test.go index c58f8a08086..66804c6e306 100644 --- a/internal/devconfig/configfile/file_test.go +++ b/internal/devconfig/configfile/file_test.go @@ -4,6 +4,8 @@ package configfile import ( "encoding/json" "io" + "os" + "path/filepath" "strings" "testing" @@ -782,3 +784,52 @@ func TestNixpkgsValidation(t *testing.T) { }) } } + +func TestFileName(t *testing.T) { + testCases := map[string]struct { + absRootPath string + want string + }{ + "empty_falls_back_to_default": {"", DefaultName}, + "json": {"/home/user/project/devbox.json", DefaultName}, + "jsonc": {"/home/user/project/devbox.jsonc", AltName}, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + c := &ConfigFile{AbsRootPath: testCase.absRootPath} + if got := c.FileName(); got != testCase.want { + t.Errorf("FileName() = %q, want %q", got, testCase.want) + } + }) + } +} + +func TestSaveToPreservesFileName(t *testing.T) { + testCases := map[string]struct { + absRootPath string + wantName string + }{ + "json": {"/anywhere/devbox.json", DefaultName}, + "jsonc": {"/anywhere/devbox.jsonc", AltName}, + "no_path_uses_default": {"", DefaultName}, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + in, err := LoadBytes([]byte(`{"packages": []}`)) + if err != nil { + t.Fatalf("LoadBytes error: %v", err) + } + in.AbsRootPath = testCase.absRootPath + + dir := t.TempDir() + if err := in.SaveTo(dir); err != nil { + t.Fatalf("SaveTo(%q) error: %v", dir, err) + } + if _, err := os.Stat(filepath.Join(dir, testCase.wantName)); err != nil { + t.Errorf("expected %q to be written: %v", testCase.wantName, err) + } + }) + } +}