summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-04-18 08:47:27 +0300
committerPaul Buetow <paul@buetow.org>2026-04-18 08:47:27 +0300
commit60f717b97ce6c375679080472750e60aab9dcd8f (patch)
treeb1028ea0e51ce6134ed3ea06da2a7205457319c5
parent526e3bd1bea7e2ef67c6984b91cdb9b3ac3be4e5 (diff)
refactor: move Gitter interface to consumer package internal/cli (task p6)
Per Go best practice (100 Go Mistakes #6): interfaces belong where they are used, not where they are implemented. Move Gitter from internal/git to internal/cli/git.go. Compile-time assertions (var _ Gitter = ...) kept in the new location. internal/git retains concrete *Git and *NoOp types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
-rw-r--r--internal/cli/cli.go12
-rw-r--r--internal/cli/cli_backend.go10
-rw-r--r--internal/cli/git.go48
-rw-r--r--internal/git/git.go34
-rw-r--r--internal/git/git_test.go13
-rw-r--r--internal/git/noop.go9
6 files changed, 76 insertions, 50 deletions
diff --git a/internal/cli/cli.go b/internal/cli/cli.go
index 94c9f57..01342be 100644
--- a/internal/cli/cli.go
+++ b/internal/cli/cli.go
@@ -22,7 +22,6 @@ import (
"codeberg.org/snonux/foostore/internal/backend"
"codeberg.org/snonux/foostore/internal/clipboard"
"codeberg.org/snonux/foostore/internal/config"
- "codeberg.org/snonux/foostore/internal/git"
"codeberg.org/snonux/foostore/internal/shell"
"codeberg.org/snonux/foostore/internal/store"
)
@@ -59,10 +58,11 @@ var SearchActions = map[string]store.Action{
// backend is *store.Store, which satisfies Backend via the compile-time check
// in internal/backend/backend.go.
//
-// g is declared as git.Gitter (interface) rather than *git.Git so that the
-// keepass backend can supply a git.NoOp when the kdbx file lives outside a git
-// repository. Dispatch code requires no nil checks; it always calls through the
-// interface regardless of whether real git operations or no-ops are performed.
+// g is declared as Gitter (defined in git.go in this package) rather than
+// *git.Git so that the keepass backend can supply a *git.NoOp when the kdbx
+// file lives outside a git repository. Dispatch code requires no nil checks;
+// it always calls through the interface regardless of whether real git
+// operations or no-ops are performed.
//
// effectiveBackend is the resolved backend name (after applying the --backend
// flag override on top of cfg.Backend). Guards such as cmdMigrateKDBX use
@@ -70,7 +70,7 @@ var SearchActions = map[string]store.Action{
type CLI struct {
cfg *config.Config
st backend.Backend
- g git.Gitter // real *git.Git or *git.NoOp when kdbx is outside a repo
+ g Gitter // real *git.Git or *git.NoOp when kdbx is outside a repo
clip *clipboard.Clipboard
sh *shell.Shell
openKDBX func(string, string) (KDBXStore, error)
diff --git a/internal/cli/cli_backend.go b/internal/cli/cli_backend.go
index 96058ae..698c9f3 100644
--- a/internal/cli/cli_backend.go
+++ b/internal/cli/cli_backend.go
@@ -38,7 +38,7 @@ func resolveBackend(flagValue, cfgValue string) string {
// buildBackend constructs the Backend and its associated Gitter based on
// effectiveBackend ("geheim" or "keepass"). Returns the Backend and git client
// so the caller can wire them into the CLI struct.
-func buildBackend(ctx context.Context, cfg *config.Config, effectiveBackend string) (backend.Backend, git.Gitter, error) {
+func buildBackend(ctx context.Context, cfg *config.Config, effectiveBackend string) (backend.Backend, Gitter, error) {
switch effectiveBackend {
case "keepass":
return buildKeepassBackend(ctx, cfg)
@@ -50,7 +50,7 @@ func buildBackend(ctx context.Context, cfg *config.Config, effectiveBackend stri
// buildGeheimBackend initialises the original AES-encrypted geheim backend:
// reads the PIN, builds the cipher, creates a *store.Store, and points git at
// cfg.DataDir via buildGeheimGit.
-func buildGeheimBackend(cfg *config.Config) (backend.Backend, git.Gitter, error) {
+func buildGeheimBackend(cfg *config.Config) (backend.Backend, Gitter, error) {
pin, err := readPIN()
if err != nil {
return nil, nil, fmt.Errorf("reading PIN: %w", err)
@@ -75,7 +75,7 @@ func buildGeheimBackend(cfg *config.Config) (backend.Backend, git.Gitter, error)
// directory is always a git repository (it is the store itself), so a real
// git client is always appropriate here — unlike the keepass backend which
// may live outside a repo and needs a NoOp fallback.
-func buildGeheimGit(cfg *config.Config) git.Gitter {
+func buildGeheimGit(cfg *config.Config) Gitter {
return git.New(cfg.DataDir)
}
@@ -89,7 +89,7 @@ func buildGeheimGit(cfg *config.Config) git.Gitter {
// repository, a *git.NoOp is returned instead — its methods print an
// informational message ("kdbx file is not in a git repo; skipping") and return
// nil, keeping the UX transparent without crashing.
-func buildKeepassBackend(ctx context.Context, cfg *config.Config) (backend.Backend, git.Gitter, error) {
+func buildKeepassBackend(ctx context.Context, cfg *config.Config) (backend.Backend, Gitter, error) {
passphrase, err := readKeepassPassphrase(cfg)
if err != nil {
return nil, nil, fmt.Errorf("reading keepass passphrase: %w", err)
@@ -113,7 +113,7 @@ func buildKeepassBackend(ctx context.Context, cfg *config.Config) (backend.Backe
// the kdbx file. When the directory is a git repository, a real *git.Git is
// returned. Otherwise, a *git.NoOp is returned so that callers receive
// informational messages rather than errors when running git commands.
-func buildKeepassGit(kdbxPath string) git.Gitter {
+func buildKeepassGit(kdbxPath string) Gitter {
kdbxDir := filepath.Dir(kdbxPath)
if git.IsGitRepo(kdbxDir) {
return git.New(kdbxDir)
diff --git a/internal/cli/git.go b/internal/cli/git.go
new file mode 100644
index 0000000..4921a38
--- /dev/null
+++ b/internal/cli/git.go
@@ -0,0 +1,48 @@
+// Package cli — git abstraction for the CLI layer.
+//
+// This file defines the Gitter interface, which the CLI uses to abstract over
+// real git operations (*git.Git) and no-op stubs (*git.NoOp). The interface
+// lives here in the consumer package (internal/cli) rather than in the
+// producer package (internal/git), following Go best practice #6 from
+// "100 Go Mistakes": interfaces should be defined where they are used, not
+// where they are implemented.
+//
+// Keeping the interface here avoids tight coupling between internal/git and its
+// callers: internal/git does not need to know who depends on it, and new
+// consumers can define their own narrower interfaces as needed.
+package cli
+
+import (
+ "context"
+
+ "codeberg.org/snonux/foostore/internal/git"
+)
+
+// Gitter abstracts git operations so that the CLI dispatch logic works
+// identically whether backed by a real git repository (*git.Git) or a no-op
+// stub (*git.NoOp — used when the KeePass database file lives outside a repo).
+type Gitter interface {
+ // Add stages a single file for the next commit.
+ Add(ctx context.Context, filePath string) error
+
+ // Remove stages a file deletion for the next commit.
+ Remove(ctx context.Context, filePath string) error
+
+ // Status prints the current git status of the working directory.
+ Status(ctx context.Context) error
+
+ // Commit records all staged changes with a generic commit message.
+ Commit(ctx context.Context) error
+
+ // Reset discards all uncommitted changes in the working directory.
+ Reset(ctx context.Context) error
+
+ // Sync pulls from and pushes to each configured remote repository.
+ Sync(ctx context.Context, syncRepos []string) error
+}
+
+// Compile-time assertions: both concrete git types must satisfy Gitter.
+// These assertions live in the consumer (cli), not the producer (git),
+// so that internal/git remains free of any dependency on this package.
+var _ Gitter = (*git.Git)(nil)
+var _ Gitter = (*git.NoOp)(nil)
diff --git a/internal/git/git.go b/internal/git/git.go
index 72d55af..e9a1b9b 100644
--- a/internal/git/git.go
+++ b/internal/git/git.go
@@ -2,10 +2,10 @@
// It mirrors the Git module from the original Ruby implementation (geheim.rb lines 79-123),
// running real git subprocesses rather than using a Go git library.
//
-// The package exposes a Gitter interface so that callers can accept either a
-// real *Git (backed by a git repository) or a *NoOp stub (for directories that
-// are not git repositories). This avoids nil-pointer panics in the CLI dispatch
-// loop and keeps git-related decisions local to this package.
+// The package exposes concrete types (*Git and *NoOp) but does NOT define a
+// Gitter interface. Per Go best practices (100 Go Mistakes #6), interfaces
+// belong in the consumer package — the Gitter interface lives in internal/cli,
+// which is the only package that needs to abstract over *Git vs *NoOp.
package git
import (
@@ -18,37 +18,11 @@ import (
"strings"
)
-// Gitter is the interface that both *Git (real git operations) and *NoOp
-// (informational no-ops) implement. The CLI holds a Gitter so that it can be
-// freely swapped without changing any dispatch logic.
-type Gitter interface {
- // Add stages a single file for the next commit.
- Add(ctx context.Context, filePath string) error
-
- // Remove stages a file deletion for the next commit.
- Remove(ctx context.Context, filePath string) error
-
- // Status prints the current git status of the working directory.
- Status(ctx context.Context) error
-
- // Commit records all staged changes with a generic commit message.
- Commit(ctx context.Context) error
-
- // Reset discards all uncommitted changes in the working directory.
- Reset(ctx context.Context) error
-
- // Sync pulls from and pushes to each configured remote repository.
- Sync(ctx context.Context, syncRepos []string) error
-}
-
// Git provides git operations scoped to the secret store's data directory.
type Git struct {
dataDir string
}
-// Compile-time assertion: *Git must satisfy Gitter.
-var _ Gitter = (*Git)(nil)
-
// New creates a Git helper for the given data directory.
func New(dataDir string) *Git {
return &Git{dataDir: dataDir}
diff --git a/internal/git/git_test.go b/internal/git/git_test.go
index 462c747..4c22fec 100644
--- a/internal/git/git_test.go
+++ b/internal/git/git_test.go
@@ -283,11 +283,14 @@ func TestIsGitRepo_outside(t *testing.T) {
}
}
-// TestNoOp_satisfies_Gitter verifies that *git.NoOp compiles as a Gitter and
-// that all its methods return nil (no-op, no error) so they are safe to call
-// unconditionally from CLI dispatch.
-func TestNoOp_satisfies_Gitter(t *testing.T) {
- var g git.Gitter = git.NewNoOp()
+// TestNoOp_methods verifies that all *git.NoOp methods return nil (no-op, no
+// error) so they are safe to call unconditionally from CLI dispatch.
+//
+// Note: the Gitter interface is defined in internal/cli (the consumer), not
+// here in internal/git (the producer), per Go best practice #6. The
+// compile-time assertion that *NoOp satisfies Gitter lives in internal/cli/git.go.
+func TestNoOp_methods(t *testing.T) {
+ g := git.NewNoOp()
ctx := context.Background()
table := []struct {
diff --git a/internal/git/noop.go b/internal/git/noop.go
index 44dc68c..f125644 100644
--- a/internal/git/noop.go
+++ b/internal/git/noop.go
@@ -9,7 +9,7 @@ import (
// because the kdbx file is not inside a git repository.
const noOpMessage = "kdbx file is not in a git repo; skipping"
-// NoOp is a Gitter implementation whose every method prints an informational
+// NoOp is a no-op git client whose every method prints an informational
// message and returns nil. It is used when the KeePass database file lives
// outside of a git repository so that sync/status/commit/reset commands remain
// functional and transparent rather than crashing or returning errors.
@@ -17,11 +17,12 @@ const noOpMessage = "kdbx file is not in a git repo; skipping"
// Keeping the no-op behaviour in its own type (rather than nil-checking in the
// CLI dispatch) respects the Open/Closed Principle: the CLI is open for
// extension (new backends, new git behaviours) without modification.
+//
+// NoOp satisfies the Gitter interface defined in internal/cli (the consumer),
+// not here in the producer — per Go best practice #6 from 100 Go Mistakes.
+// The compile-time assertion lives in internal/cli/git.go.
type NoOp struct{}
-// Compile-time assertion: *NoOp must satisfy Gitter.
-var _ Gitter = (*NoOp)(nil)
-
// NewNoOp returns a *NoOp that satisfies Gitter with all operations being
// informational no-ops.
func NewNoOp() *NoOp {