From 31559f0dba2b6e3fd8a308e07f75668cc84622f1 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Tue, 13 May 2025 16:38:33 -0700 Subject: [PATCH 1/5] feat: display manifest source in human-readable format --- cmd/app/link.go | 6 +++--- internal/config/manifest.go | 13 +++++++++++++ internal/config/manifest_test.go | 26 ++++++++++++++++++++++++-- internal/pkg/create/create.go | 2 +- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/cmd/app/link.go b/cmd/app/link.go index 4defb31b..a8b0735e 100644 --- a/cmd/app/link.go +++ b/cmd/app/link.go @@ -174,10 +174,10 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty Text: "Warning", Secondary: []string{ "Existing apps have manifests configured by app settings", - "Linking existing apps requires the manifest source to be " + config.ManifestSourceRemote.String(), - fmt.Sprintf(`Manifest source can be "%s" or "%s"`, config.ManifestSourceLocal.String(), config.ManifestSourceRemote.String()), + "Linking existing apps requires the manifest source to use " + config.ManifestSourceRemote.Human(), + fmt.Sprintf(`Manifest source can be "%s" or "%s"`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), " ", - fmt.Sprintf(style.Highlight(`Your manifest source is "%s"`), manifestSource.String()), + fmt.Sprintf(style.Highlight(`Your manifest source is "%s"`), manifestSource.Human()), fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()), fmt.Sprintf("- %s: uses manifest from app settings for each app", config.ManifestSourceRemote.String()), " ", diff --git a/internal/config/manifest.go b/internal/config/manifest.go index e5a4e8e2..43ab17e5 100644 --- a/internal/config/manifest.go +++ b/internal/config/manifest.go @@ -13,6 +13,8 @@ package config +import "fmt" + type ManifestSource string const ( @@ -35,6 +37,17 @@ func (ms ManifestSource) String() string { return string(ms) } +// Human returns the string value as a human-friendly name +func (ms ManifestSource) Human() string { + switch ms { + case ManifestSourceLocal: + return fmt.Sprintf("project (%s)", ms.String()) + case ManifestSourceRemote: + return fmt.Sprintf("app settings (%s)", ms.String()) + } + return "" +} + type ManifestConfig struct { // Source of the manifest using either "local" or "remote" values Source string `json:"source,omitempty"` diff --git a/internal/config/manifest_test.go b/internal/config/manifest_test.go index a2942d68..2c0da3c3 100644 --- a/internal/config/manifest_test.go +++ b/internal/config/manifest_test.go @@ -84,11 +84,11 @@ func Test_Config_ManifestSource_String(t *testing.T) { a ManifestSource expected string }{ - "project manifest source is local": { + "local manifest source": { a: ManifestSourceLocal, expected: "local", }, - "remote manifest source is remote": { + "remote manifest source": { a: ManifestSourceRemote, expected: "remote", }, @@ -100,3 +100,25 @@ func Test_Config_ManifestSource_String(t *testing.T) { }) } } + +func Test_Config_ManifestSource_Human(t *testing.T) { + tests := map[string]struct { + a ManifestSource + expected string + }{ + "local manifest source is the project (local)": { + a: ManifestSourceLocal, + expected: "project (local)", + }, + "remote manifest source is app settings (remote)": { + a: ManifestSourceRemote, + expected: "app settings (remote)", + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + actual := tt.a.Human() + assert.Equal(t, tt.expected, actual) + }) + } +} diff --git a/internal/pkg/create/create.go b/internal/pkg/create/create.go index 449ae3d9..f334aa5c 100644 --- a/internal/pkg/create/create.go +++ b/internal/pkg/create/create.go @@ -466,7 +466,7 @@ func InstallProjectDependencies( clients.IO.PrintDebug(ctx, "Error setting manifest source in project-level config: %s", err) } else { configJSONFilename := config.ProjectConfigJSONFilename - manifestSourceStyled := style.Highlight(manifestSource.String()) + manifestSourceStyled := style.Highlight(manifestSource.Human()) outputs = append(outputs, fmt.Sprintf( "Updated %s manifest source to %s", From 5c270116c66c405c424fbf47c24a21898ce74dfe Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Tue, 13 May 2025 20:11:20 -0700 Subject: [PATCH 2/5] test: fix failing tests --- cmd/app/link.go | 9 +++++---- cmd/project/init_test.go | 2 +- internal/pkg/create/create_test.go | 6 +++--- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cmd/app/link.go b/cmd/app/link.go index a8b0735e..3fbb210c 100644 --- a/cmd/app/link.go +++ b/cmd/app/link.go @@ -173,14 +173,15 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty Emoji: "warning", Text: "Warning", Secondary: []string{ - "Existing apps have manifests configured by app settings", - "Linking existing apps requires the manifest source to use " + config.ManifestSourceRemote.Human(), - fmt.Sprintf(`Manifest source can be "%s" or "%s"`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), + "Linking an existing app requires the app manifest source to be", + fmt.Sprintf("managed by %s.", config.ManifestSourceRemote.Human()), " ", - fmt.Sprintf(style.Highlight(`Your manifest source is "%s"`), manifestSource.Human()), + fmt.Sprintf(`App manifest source can be "%s" or "%s":`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()), fmt.Sprintf("- %s: uses manifest from app settings for each app", config.ManifestSourceRemote.String()), " ", + fmt.Sprintf(style.Highlight(`Your manifest source is "%s"`), manifestSource.Human()), + " ", fmt.Sprintf("Current manifest source in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))), fmt.Sprintf(style.Highlight(` %s: "%s"`), "manifest.source", manifestSource.String()), " ", diff --git a/cmd/project/init_test.go b/cmd/project/init_test.go index 5c8e9033..c104440e 100644 --- a/cmd/project/init_test.go +++ b/cmd/project/init_test.go @@ -80,7 +80,7 @@ func Test_Project_InitCommand(t *testing.T) { require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack")) require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack", ".gitignore")) require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack", "hooks.json")) - require.Contains(t, output, "Updated config.json manifest source to local") + require.Contains(t, output, "Updated config.json manifest source to project (local)") // Assert prompt to add existing apps was called cm.IO.AssertCalled( t, diff --git a/internal/pkg/create/create_test.go b/internal/pkg/create/create_test.go index 6c2e552e..a3e55459 100644 --- a/internal/pkg/create/create_test.go +++ b/internal/pkg/create/create_test.go @@ -195,17 +195,17 @@ func Test_Create_installProjectDependencies(t *testing.T) { "Detected a project using Deno", }, }, - "When no manifest source, default to local": { + "When no manifest source, default to project (local)": { experiments: []string{"bolt"}, expectedOutputs: []string{ - "Updated config.json manifest source to local", + "Updated config.json manifest source to project (local)", }, }, "When manifest source is provided, should set it": { experiments: []string{"bolt"}, manifestSource: config.ManifestSourceRemote, expectedOutputs: []string{ - "Updated config.json manifest source to remote", + "Updated config.json manifest source to app settings (remote)", }, }, } From acd2a00b1904c1438fc48b83d723d28f2c4813d4 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 14 May 2025 14:32:15 -0700 Subject: [PATCH 3/5] Update cmd/app/link.go Co-authored-by: Eden Zimbelman --- cmd/app/link.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/app/link.go b/cmd/app/link.go index 3fbb210c..52be79e0 100644 --- a/cmd/app/link.go +++ b/cmd/app/link.go @@ -173,8 +173,8 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty Emoji: "warning", Text: "Warning", Secondary: []string{ - "Linking an existing app requires the app manifest source to be", - fmt.Sprintf("managed by %s.", config.ManifestSourceRemote.Human()), + "Linking an existing app requires the app manifest source to be managed by", + fmt.Sprintf("%s.", config.ManifestSourceRemote.Human()), " ", fmt.Sprintf(`App manifest source can be "%s" or "%s":`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()), From 3913b139af7bb057865cede55ec447e0a491e06e Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 14 May 2025 14:36:07 -0700 Subject: [PATCH 4/5] feat: handle unknown manifest source --- internal/config/manifest.go | 2 +- internal/config/manifest_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/config/manifest.go b/internal/config/manifest.go index 43ab17e5..ed7da145 100644 --- a/internal/config/manifest.go +++ b/internal/config/manifest.go @@ -45,7 +45,7 @@ func (ms ManifestSource) Human() string { case ManifestSourceRemote: return fmt.Sprintf("app settings (%s)", ms.String()) } - return "" + return ms.String() } type ManifestConfig struct { diff --git a/internal/config/manifest_test.go b/internal/config/manifest_test.go index 2c0da3c3..7d6f1026 100644 --- a/internal/config/manifest_test.go +++ b/internal/config/manifest_test.go @@ -114,6 +114,10 @@ func Test_Config_ManifestSource_Human(t *testing.T) { a: ManifestSourceRemote, expected: "app settings (remote)", }, + "unknown manifest source uses String()": { + a: "unknown", + expected: "unknown", + }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { From 4eaea06b858bc76ecf592ce7c5cc8bc863310e6f Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 14 May 2025 14:55:31 -0700 Subject: [PATCH 5/5] feat: quote the human-friendly name --- cmd/app/link.go | 4 ++-- cmd/project/init_test.go | 2 +- internal/config/manifest.go | 4 ++-- internal/config/manifest_test.go | 4 ++-- internal/pkg/create/create_test.go | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/app/link.go b/cmd/app/link.go index 52be79e0..69dae262 100644 --- a/cmd/app/link.go +++ b/cmd/app/link.go @@ -176,11 +176,11 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty "Linking an existing app requires the app manifest source to be managed by", fmt.Sprintf("%s.", config.ManifestSourceRemote.Human()), " ", - fmt.Sprintf(`App manifest source can be "%s" or "%s":`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), + fmt.Sprintf(`App manifest source can be %s or %s:`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()), fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()), fmt.Sprintf("- %s: uses manifest from app settings for each app", config.ManifestSourceRemote.String()), " ", - fmt.Sprintf(style.Highlight(`Your manifest source is "%s"`), manifestSource.Human()), + fmt.Sprintf(style.Highlight(`Your manifest source is set to %s.`), manifestSource.Human()), " ", fmt.Sprintf("Current manifest source in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))), fmt.Sprintf(style.Highlight(` %s: "%s"`), "manifest.source", manifestSource.String()), diff --git a/cmd/project/init_test.go b/cmd/project/init_test.go index c104440e..4fcea41c 100644 --- a/cmd/project/init_test.go +++ b/cmd/project/init_test.go @@ -80,7 +80,7 @@ func Test_Project_InitCommand(t *testing.T) { require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack")) require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack", ".gitignore")) require.Contains(t, output, "Added "+filepath.Join("project-name", ".slack", "hooks.json")) - require.Contains(t, output, "Updated config.json manifest source to project (local)") + require.Contains(t, output, `Updated config.json manifest source to "project" (local)`) // Assert prompt to add existing apps was called cm.IO.AssertCalled( t, diff --git a/internal/config/manifest.go b/internal/config/manifest.go index ed7da145..8f5c2f5f 100644 --- a/internal/config/manifest.go +++ b/internal/config/manifest.go @@ -41,9 +41,9 @@ func (ms ManifestSource) String() string { func (ms ManifestSource) Human() string { switch ms { case ManifestSourceLocal: - return fmt.Sprintf("project (%s)", ms.String()) + return fmt.Sprintf(`"project" (%s)`, ms.String()) case ManifestSourceRemote: - return fmt.Sprintf("app settings (%s)", ms.String()) + return fmt.Sprintf(`"app settings" (%s)`, ms.String()) } return ms.String() } diff --git a/internal/config/manifest_test.go b/internal/config/manifest_test.go index 7d6f1026..2e2651b3 100644 --- a/internal/config/manifest_test.go +++ b/internal/config/manifest_test.go @@ -108,11 +108,11 @@ func Test_Config_ManifestSource_Human(t *testing.T) { }{ "local manifest source is the project (local)": { a: ManifestSourceLocal, - expected: "project (local)", + expected: `"project" (local)`, }, "remote manifest source is app settings (remote)": { a: ManifestSourceRemote, - expected: "app settings (remote)", + expected: `"app settings" (remote)`, }, "unknown manifest source uses String()": { a: "unknown", diff --git a/internal/pkg/create/create_test.go b/internal/pkg/create/create_test.go index a3e55459..97e28bc2 100644 --- a/internal/pkg/create/create_test.go +++ b/internal/pkg/create/create_test.go @@ -198,14 +198,14 @@ func Test_Create_installProjectDependencies(t *testing.T) { "When no manifest source, default to project (local)": { experiments: []string{"bolt"}, expectedOutputs: []string{ - "Updated config.json manifest source to project (local)", + `Updated config.json manifest source to "project" (local)`, }, }, "When manifest source is provided, should set it": { experiments: []string{"bolt"}, manifestSource: config.ManifestSourceRemote, expectedOutputs: []string{ - "Updated config.json manifest source to app settings (remote)", + `Updated config.json manifest source to "app settings" (remote)`, }, }, }