diff --git a/internal/cmd/app.go b/internal/cmd/app.go index 76adb44..c625a7f 100644 --- a/internal/cmd/app.go +++ b/internal/cmd/app.go @@ -40,13 +40,9 @@ type App struct { refreshTimeout time.Duration } -// NewApp creates an App. -// -// The logger it installs is silent: nothing is emitted until PersistentPreRunE -// calls output.SetupLogger again with the resolved --debug and --output flags. -// This first call exists only so that a failure before flag parsing has a -// default logger to reach, and it writes to os.Stderr because no command — and -// therefore no cmd.ErrOrStderr() — exists yet. +// NewApp creates an App. The silent logger it installs writes to os.Stderr +// because no command — and therefore no cmd.ErrOrStderr() — exists yet; see +// output.SetupLogger for why it is installed twice. func NewApp() *App { output.SetupLogger(os.Stderr, output.FormatPretty, false) diff --git a/internal/cmd/template_list.go b/internal/cmd/template_list.go index af901bc..a4289bf 100644 --- a/internal/cmd/template_list.go +++ b/internal/cmd/template_list.go @@ -32,11 +32,6 @@ func isTrackable(meta *pkgtemplate.Metadata) bool { // templateRow is one row of `template list`. // -// The json tags are what a consumer's jq filter matches on; the column headings -// beside them are prose for a reader and can be reworded without changing a -// single key. That separation is the point of the type: one string used to do -// both jobs, and `Name` was a JSON key only because it was also a heading. -// // Created and Updated carry the timestamps themselves rather than the "3 days // ago" the table shows — a relative phrase is a reading aid, not something a // script can compute with. A field with no value is absent rather than "-", diff --git a/internal/cmd/template_use.go b/internal/cmd/template_use.go index 543e988..1d39db3 100644 --- a/internal/cmd/template_use.go +++ b/internal/cmd/template_use.go @@ -65,8 +65,8 @@ func newTemplateUseCmd(app *App) *cobra.Command { return cmd } -// executeTemplate is the shared execution logic reused by specs template use (Phase 7) -// and specs use (Phase 8). +// executeTemplate is the shared execution logic reused by `specs template use` +// and `specs use`. func (a *App) executeTemplate(templateRoot, targetDir string, opts executeOpts) error { cfg := a.templateConfig() cfg.ContinueOnRenderError = opts.continueOnError @@ -143,7 +143,6 @@ func (a *App) executeTemplate(templateRoot, targetDir string, opts executeOpts) } } - // Emit one log line per key in alphabetical order with its final resolved source. for _, k := range sortedKeys(finalSource) { slog.Debug("context key resolved", "key", k, "source", finalSource[k]) } @@ -364,7 +363,6 @@ func promptContext( // Iterative conditional passes: each round handles one dependency layer. for len(remaining) > 0 { - // Find keys whose gate variables are all resolved (or not in schema). var ready []string for k := range remaining { @@ -415,7 +413,7 @@ func promptContext( // // When there is nothing that could answer the form, it returns an error naming // every key it would have asked for rather than blocking on a read nobody will -// answer — the CI hang this guard exists to prevent. +// answer. See prompter.cannotPrompt. func runPromptPass( p prompter, ctx map[string]any, diff --git a/internal/cmd/template_use_test.go b/internal/cmd/template_use_test.go index 5304a85..00cc6e1 100644 --- a/internal/cmd/template_use_test.go +++ b/internal/cmd/template_use_test.go @@ -680,8 +680,6 @@ func TestTemplateUse_SafeMode_SkipsHooks(t *testing.T) { } } -// TestExecuteTemplate_RemoteHooks_RunsWithYes verifies that when remote=true and yes=true, -// hooks execute without an interactive prompt. func TestExecuteTemplate_RemoteHooks_RunsWithYes(t *testing.T) { dir := t.TempDir() sentinel := filepath.Join(t.TempDir(), "hook-ran") @@ -705,8 +703,7 @@ func TestExecuteTemplate_RemoteHooks_RunsWithYes(t *testing.T) { } } -// TestExecuteTemplate_RemoteHooks_SafeMode verifies that safe-mode skips hooks -// even for remote sources (no confirmation needed). +// Safe mode skips hooks before the remote confirmation is ever reached. func TestExecuteTemplate_RemoteHooks_SafeMode(t *testing.T) { dir := t.TempDir() sentinel := filepath.Join(t.TempDir(), "hook-ran") @@ -729,7 +726,6 @@ func TestExecuteTemplate_RemoteHooks_SafeMode(t *testing.T) { } } -// TestExecuteTemplate_SafeMode_AllowHooks verifies that --allow-hooks overrides safe-mode. func TestExecuteTemplate_SafeMode_AllowHooks(t *testing.T) { dir := t.TempDir() sentinel := filepath.Join(t.TempDir(), "hook-ran") @@ -779,9 +775,8 @@ func TestTemplateUse_AmbiguousProjectFiles(t *testing.T) { } } -// A non-interactive run that still needs a value fails immediately, naming the -// variable, rather than blocking on a form nobody can answer. That hang — with -// no output explaining it — is what this guard exists to prevent. +// A run that still needs a value fails immediately, naming the variable, rather +// than blocking on a form nobody can answer. func TestTemplateUse_NoTerminal_NamesTheMissingVariables(t *testing.T) { withTempRegistry(t) src := makeTemplateWithVar(t, "Name", "world") diff --git a/internal/host/source.go b/internal/host/source.go index 85aa661..e6d645e 100644 --- a/internal/host/source.go +++ b/internal/host/source.go @@ -135,7 +135,6 @@ func parseGitHub(rest, input string) (*Source, error) { return s, nil } -// validateGitHubName checks that a GitHub owner or repo name is valid. func validateGitHubName(kind, name string, maxLen int) error { if name == "" { return fmt.Errorf("%s is empty", kind) diff --git a/internal/template/context.go b/internal/template/context.go index ad6dd22..374859e 100644 --- a/internal/template/context.go +++ b/internal/template/context.go @@ -134,7 +134,6 @@ func ApplyComputed(ctx map[string]any, defs map[string]string, funcMap texttempl return ctx, nil } - // Build dependency graph among computed keys. keys := make([]string, 0, len(defs)) for k := range defs { keys = append(keys, k) @@ -150,7 +149,6 @@ func ApplyComputed(ctx map[string]any, defs map[string]string, funcMap texttempl return nil, fmt.Errorf("computed values: %w", err) } - // Copy the context so we don't mutate the caller's map. result := make(map[string]any, len(ctx)+len(defs)) maps.Copy(result, ctx) @@ -299,7 +297,6 @@ func extractComputed(raw map[string]any) (map[string]string, error) { // topological order so that each key's pre-fill value is correct before the user is prompted. // It returns a new map; the caller's input is never modified. func resolveReferencedDefaults(ctx map[string]any, funcMap texttemplate.FuncMap, delims specs.Delimiters) (map[string]any, error) { - // Find keys whose string value is a template expression. var refKeys []string for k, v := range ctx { @@ -322,7 +319,6 @@ func resolveReferencedDefaults(ctx map[string]any, funcMap texttemplate.FuncMap, return nil, fmt.Errorf("referenced defaults: %w", err) } - // Copy the input map so the caller's map is not mutated. result := make(map[string]any, len(ctx)) maps.Copy(result, ctx) diff --git a/internal/template/template.go b/internal/template/template.go index c86234e..714638c 100644 --- a/internal/template/template.go +++ b/internal/template/template.go @@ -236,7 +236,6 @@ func (t *Template) Execute(targetDir string) error { return nil // skip the root itself } - // 1. Skip OS/editor metadata files. if ignoredFiles[d.Name()] { if d.IsDir() { return filepath.SkipDir @@ -245,7 +244,6 @@ func (t *Template) Execute(targetDir string) error { return nil } - // 2. Render the relative path as a template to get the destination path. destRel, err := t.renderName(rel, ctx) if err != nil || strings.TrimSpace(destRel) == "" { slog.Debug("skipping path", "path", rel, "error", err) @@ -261,7 +259,8 @@ func (t *Template) Execute(targetDir string) error { return nil } - // 3. Skip if any path segment is empty (conditional directory exclusion). + // An empty path segment means a conditional rendered away a directory + // name, which excludes the whole subtree. if hasEmptySegment(destRel) { if !d.IsDir() { skipped++ @@ -276,12 +275,10 @@ func (t *Template) Execute(targetDir string) error { destPath := filepath.Join(targetDir, destRel) - // 4. Directory: create it. if d.IsDir() { return os.MkdirAll(destPath, 0755) } - // 5. File: determine copy strategy. relForward := filepath.ToSlash(rel) if t.verbatim.Matches(relForward) || isBinary(srcPath) { slog.Debug("file decision", "path", rel, "dest", destPath, "action", "verbatim") diff --git a/internal/template/validate.go b/internal/template/validate.go index 74257c4..beb4a43 100644 --- a/internal/template/validate.go +++ b/internal/template/validate.go @@ -98,7 +98,6 @@ func (t *Template) Validate() ([]ValidationIssue, error) { } } - // Scan template files and path expressions for unknown variable references. srcRoot := filepath.Join(t.Root, specs.TemplateDirFile) err := filepath.WalkDir(srcRoot, func(path string, d fs.DirEntry, err error) error { @@ -113,7 +112,7 @@ func (t *Template) Validate() ([]ValidationIssue, error) { reportPath := filepath.ToSlash(relFromRoot) - // Check the entry's name as a path template expression. + // A file or directory name is itself a path template expression. for _, k := range extractRefs(d.Name(), t.funcMap, t.cfg.delims()) { if !allDefined[k] { addIssue(ErrUnknownVariable, k, reportPath) @@ -124,7 +123,6 @@ func (t *Template) Validate() ([]ValidationIssue, error) { return nil } - // Check file content. data, readErr := os.ReadFile(path) if readErr != nil { return readErr @@ -142,15 +140,14 @@ func (t *Template) Validate() ([]ValidationIssue, error) { return nil, err } - // Unused variables: defined in project.yaml but not referenced anywhere. - // Template.Referenced already includes variables used in computed expressions. + // Template.Referenced already includes variables used in computed + // expressions. for k := range t.Context { if !t.Referenced[k] { addIssue(ErrUnusedVariable, k, "") } } - // Unused computed: defined under "computed" but not referenced anywhere. for k := range t.ComputedDefs { if !t.Referenced[k] { addIssue(ErrUnusedComputed, k, "") diff --git a/internal/util/output/rows.go b/internal/util/output/rows.go index 4ed72d7..1c9e36a 100644 --- a/internal/util/output/rows.go +++ b/internal/util/output/rows.go @@ -47,8 +47,7 @@ type TableData struct { // // It is a function rather than a Writer method because Go has no generic // methods. Going through it is also what guarantees every row has exactly one -// cell per header — alignment a raw [][]Cell can silently get wrong — and what -// keeps the two forms of a row derived from the same value. +// cell per header — alignment a raw [][]Cell can silently get wrong. func Table[T any](w Writer, rows []T, cols ...Column[T]) { data := TableData{ Headers: make([]string, len(cols)), diff --git a/internal/util/output/tty.go b/internal/util/output/tty.go index a878c64..9695be9 100644 --- a/internal/util/output/tty.go +++ b/internal/util/output/tty.go @@ -10,7 +10,7 @@ type fileDescriptor interface { // IsTTY reports whether stream is an interactive terminal. // // The parameter is any rather than io.Writer deliberately: the question that -// matters most here is about **stdin**, because the failure being prevented is a +// matters most here is about stdin, because the failure being prevented is a // read with nobody to answer it. A job with a terminal on stderr and its stdin // closed must still refuse to prompt, and cmd.InOrStdin() hands back an // io.Reader. diff --git a/internal/util/output/writer.go b/internal/util/output/writer.go index 0909df9..9873361 100644 --- a/internal/util/output/writer.go +++ b/internal/util/output/writer.go @@ -240,9 +240,7 @@ func (w *JSONWriter) WriteErr(err error) { fmt.Fprintln(w.stderr, string(data)) } -// WriteTable emits one JSON object per row: the row values themselves, so a -// number stays a number and the keys come from the row type's json tags rather -// than from a column heading written for a reader. +// WriteTable emits one JSON object per row. // // One object per line, not one array. An array cannot be parsed until its // closing bracket arrives, so a run that is killed or fails partway leaves