Skip to content

Commit 65f30b0

Browse files
committed
Refactor: simplify GetRepository function and remove refresh flag
1 parent ad7ab5d commit 65f30b0

6 files changed

Lines changed: 28 additions & 66 deletions

File tree

cmd/browse.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ Flags --scope (-s) and --force (-f) match fontget add (user/machine install scop
2929
return err
3030
}
3131

32-
refresh, _ := cmd.Flags().GetBool("refresh")
33-
r, err := cmdutils.GetRepository(refresh, GetLogger())
32+
r, err := cmdutils.GetRepository(GetLogger())
3433
if err != nil {
3534
return err
3635
}
@@ -86,6 +85,4 @@ func init() {
8685
rootCmd.AddCommand(browseCmd)
8786
browseCmd.Flags().StringP("scope", "s", "", "Installation scope (user or machine)")
8887
browseCmd.Flags().BoolP("force", "f", false, "Force installation even if font is already installed")
89-
browseCmd.Flags().Bool("refresh", false, "Force refresh of font manifest before browse")
90-
_ = browseCmd.Flags().MarkHidden("refresh")
9188
}

cmd/info.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ Use --license to show only license information.`,
9494

9595
// Get repository (using cached manifest)
9696
output.GetVerbose().Info("Initializing repository for font lookup")
97-
r, err := cmdutils.GetRepository(false, GetLogger())
97+
r, err := cmdutils.GetRepository(GetLogger())
9898
if err != nil {
9999
return err
100100
}

cmd/integration_test.go

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -121,43 +121,25 @@ func TestGetRepository_Integration(t *testing.T) {
121121
t.Fatalf("Failed to initialize manifest: %v", err)
122122
}
123123

124-
tests := []struct {
125-
name string
126-
refresh bool
127-
}{
128-
{
129-
name: "get repository without refresh",
130-
refresh: false,
131-
},
132-
{
133-
name: "get repository with refresh",
134-
refresh: true,
135-
},
136-
}
124+
repo, err := cmdutils.GetRepository(GetLogger())
137125

138-
for _, tt := range tests {
139-
t.Run(tt.name, func(t *testing.T) {
140-
repo, err := cmdutils.GetRepository(tt.refresh, GetLogger())
141-
142-
if err != nil {
143-
t.Errorf("getRepository() unexpected error: %v", err)
144-
return
145-
}
126+
if err != nil {
127+
t.Errorf("getRepository() unexpected error: %v", err)
128+
return
129+
}
146130

147-
if repo == nil {
148-
t.Errorf("getRepository() returned nil repository")
149-
return
150-
}
131+
if repo == nil {
132+
t.Errorf("getRepository() returned nil repository")
133+
return
134+
}
151135

152-
// Verify repository is usable
153-
manifest, err := repo.GetManifest()
154-
if err != nil {
155-
t.Errorf("getRepository() returned repository that failed to get manifest: %v", err)
156-
}
157-
if manifest == nil {
158-
t.Errorf("getRepository() returned repository with nil manifest")
159-
}
160-
})
136+
// Verify repository is usable
137+
manifest, err := repo.GetManifest()
138+
if err != nil {
139+
t.Errorf("getRepository() returned repository that failed to get manifest: %v", err)
140+
}
141+
if manifest == nil {
142+
t.Errorf("getRepository() returned repository with nil manifest")
161143
}
162144
}
163145

cmd/search.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ Use -s without a value to list sources.`,
194194
// Get arguments (already validated by Args function)
195195
category, _ := cmd.Flags().GetString("category")
196196
source, _ := cmd.Flags().GetString("source")
197-
refresh, _ := cmd.Flags().GetBool("refresh")
198197
var query string
199198
if len(args) > 0 {
200199
query = args[0]
@@ -207,7 +206,7 @@ Use -s without a value to list sources.`,
207206
}
208207

209208
// Log search parameters (always log to file)
210-
GetLogger().Info("Search parameters - Query: %s, Category: %s, Source: %s, Refresh: %v", query, category, source, refresh)
209+
GetLogger().Info("Search parameters - Query: %s, Category: %s, Source: %s", query, category, source)
211210

212211
// Handle category-only mode (show all categories) - early return
213212
// Check if category flag was provided but no value was given (NoOptDefVal = "list")
@@ -252,15 +251,14 @@ Use -s without a value to list sources.`,
252251
}
253252

254253
// Verbose-level information for users - show operational details
255-
output.GetVerbose().Info("Search parameters - Query: %s, Category: %s, Source: %s, Refresh: %v", query, category, source, refresh)
254+
output.GetVerbose().Info("Search parameters - Query: %s, Category: %s, Source: %s", query, category, source)
256255
// Verbose section ends with blank line per spacing framework (only if verbose was shown)
257256
if output.IsVerboseOutputEnabled() {
258257
fmt.Println()
259258
}
260-
output.GetDebug().State("Starting font search with parameters: query='%s', category='%s', source='%s', refresh=%v", query, category, source, refresh)
259+
output.GetDebug().State("Starting font search with parameters: query='%s', category='%s', source='%s'", query, category, source)
261260

262-
// Get repository with optional refresh
263-
r, err := cmdutils.GetRepository(refresh, GetLogger())
261+
r, err := cmdutils.GetRepository(GetLogger())
264262
if err != nil {
265263
return err
266264
}
@@ -510,10 +508,6 @@ func init() {
510508
searchCmd.Flags().StringP("source", "s", "", "Filter by source (short ID like \"google\", \"nerd\", \"squirrel\" or full name like \"Google Fonts\")")
511509
searchCmd.Flags().Lookup("source").NoOptDefVal = "list"
512510

513-
// Hidden flag for development/testing only
514-
searchCmd.Flags().Bool("refresh", false, "Force refresh of font manifest before search")
515-
searchCmd.Flags().MarkHidden("refresh")
516-
517511
// Helper function for category completion (shared by both short and long flags)
518512
categoryCompletionFunc := func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
519513
r, err := repo.GetRepositoryForShellCompletion()

docs/development/guidelines/codebase-layout-guidelines.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func SortSources(sources []SourceItem) {
351351

352352
```go
353353
// internal/cmdutils/repository.go
354-
func GetRepository(refresh bool, logger Logger) (*repo.Repository, error) {
354+
func GetRepository(logger Logger) (*repo.Repository, error) {
355355
// CLI-specific: logging, error formatting
356356
r, err := repo.GetRepository() // Business logic in repo package
357357
if err != nil {

internal/cmdutils/repository.go

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,17 @@ import (
77
"fontget/internal/repo"
88
)
99

10-
// GetRepository gets the font repository with optional refresh.
11-
// If refresh is true, forces a refresh of the font manifest.
10+
// GetRepository gets the font repository (standard caching/refresh policy in repo).
1211
// Returns standardized error handling for repository initialization.
1312
//
1413
// logger can be nil (for testing or when logging is not needed).
1514
//
1615
// NOTE: This function is tested via integration tests (see cmd/integration_test.go)
1716
// because it depends on package-level repo functions that are difficult to mock.
18-
func GetRepository(refresh bool, logger Logger) (*repo.Repository, error) {
19-
var r *repo.Repository
20-
var err error
21-
22-
if refresh {
23-
// Force refresh of font manifest before use
24-
output.GetVerbose().Info("Forcing refresh of font manifest")
25-
output.GetDebug().State("Using GetRepositoryWithRefresh() to force source updates")
26-
r, err = repo.GetRepositoryWithRefresh()
27-
} else {
28-
output.GetVerbose().Info("Using cached font manifest")
29-
output.GetDebug().State("Using GetRepository() with cached sources")
30-
r, err = repo.GetRepository()
31-
}
17+
func GetRepository(logger Logger) (*repo.Repository, error) {
18+
output.GetVerbose().Info("Loading font repository")
19+
output.GetDebug().State("Calling repo.GetRepository()")
20+
r, err := repo.GetRepository()
3221

3322
if err != nil {
3423
if logger != nil {

0 commit comments

Comments
 (0)