Merge pull request #83 from davidovich/last-minute-fixes

Last minute fixes

cmd/list.go

@@ -18,7 +18,7 @@ type listCmdOpts struct {
 	cmd      *cobra.Command
 }
 
-func newListCmd(asOption bool, root *cobra.Command, driver summon.ConfigurableLister, main *mainCmd) *cobra.Command {
+func newListCmd(asOption bool, root *cobra.Command, driver summon.ConfigurableLister, main *mainCmd) {
 	listCmd := &listCmdOpts{
 		driver: driver,
 	}
@@ -42,8 +42,6 @@ func newListCmd(asOption bool, root *cobra.Command, driver summon.ConfigurableLi
 
 	listCmd.out = root.OutOrStdout()
 	root.Flags().BoolVar(&listCmd.tree, "tree", false, "Print pretty tree of data")
-
-	return root
 }
 
 func (l *listCmdOpts) run() error {

cmd/list_test.go

@@ -38,14 +38,14 @@ func TestListCmd(t *testing.T) {
 			s, _ := summon.New(cmdTestFS)
 
 			rootCmd := &cobra.Command{Use: "root", Run: func(cmd *cobra.Command, args []string) {}}
-			newRoot := newListCmd(false, rootCmd, s, &mainCmd{})
+			newListCmd(false, rootCmd, s, &mainCmd{})
 			if tt.args == nil {
 				tt.args = make([]string, 0)
 			}
 			rootCmd.SetArgs(tt.args)
 
 			b := &bytes.Buffer{}
-			newRoot.SetOut(b)
+			rootCmd.SetOut(b)
 			rootCmd.Execute()
 
 			assert.Contains(t, b.String(), tt.expected)

cmd/root.go

@@ -33,6 +33,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime/debug"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -111,18 +112,41 @@ func CreateRootCmd(driver *summon.Driver, args []string, options summon.MainOpti
 	rootCmd.Flags().StringVarP(&main.dest, "out", "o", driver.OutputDir(), "destination directory, or '-' for stdout")
 	rootCmd.Flags().BoolVarP(&showVersion, "version", "v", false, "output data version info and exit")
 
+	// we have a --help flag, hide the help sub-command
+	rootCmd.SetHelpCommand(&cobra.Command{Hidden: true})
+
 	// add ls cmd or --ls flag
-	listRootCmd := newListCmd(options.WithoutRunSubcmd, rootCmd, driver, main)
-	// configure summonables completion
-	list, _ := driver.List()
-	for _, summonable := range list {
-		listRootCmd.AddCommand(&cobra.Command{
-			Use:   summonable,
-			Short: "summon file to " + main.dest + "/ dir",
-			RunE: func(cmd *cobra.Command, args []string) error {
-				main.filename = cmd.Use
-				return main.run()
-			}})
+	newListCmd(options.WithoutRunSubcmd, rootCmd, driver, main)
+
+	if !driver.HideAssetsInHelp() {
+		// configure summonables completion
+		list, _ := driver.List()
+		for _, summonable := range list {
+			rootCmd.AddCommand(&cobra.Command{
+				Use:   summonable,
+				Short: "summon " + summonable + " file to " + main.dest + "/ dir",
+				Args: func(cmd *cobra.Command, args []string) error {
+					if main.json != "" && main.jsonFile != "" {
+						return fmt.Errorf("--json and --json-file are mutually exclusive")
+					}
+					return nil
+				},
+				RunE: func(cmd *cobra.Command, args []string) error {
+					main.filename = cmd.Use
+					return main.run()
+				}})
+		}
+	} else {
+		rootCmd.ValidArgsFunction = func(cmd *cobra.Command, cobraArgs []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+			var candidates []string
+			list, _ := driver.List()
+			for _, summonable := range list {
+				if strings.HasPrefix(summonable, toComplete) {
+					candidates = append(candidates, summonable)
+				}
+			}
+			return candidates, cobra.ShellCompDirectiveNoFileComp
+		}
 	}
 
 	// add run cmd, or root subcommands

cmd/root_test.go

@@ -6,11 +6,13 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"testing/fstest"
 
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 
 	"github.com/davidovich/summon/internal/testutil"
+	"github.com/davidovich/summon/pkg/config"
 	"github.com/davidovich/summon/pkg/summon"
 )
 
@@ -45,14 +47,16 @@ func Test_createRootCmd(t *testing.T) {
 		}
 	}
 
-	makeRootCmd := func(args ...string) *cobra.Command {
-		_, c := makeRootCmd(false, args...)
-		return c
+	makeRootCmd := func(args ...string) func(args ...string) *cobra.Command {
+		return func(a ...string) *cobra.Command {
+			_, c := makeRootCmd(false, args...)
+			return c
+		}
 	}
 
 	tests := []struct {
 		name     string
-		rootCmd  *cobra.Command
+		rootCmd  func(args ...string) *cobra.Command
 		expected string
 		in       string
 		wantErr  bool
@@ -117,11 +121,12 @@ func Test_createRootCmd(t *testing.T) {
 				defer tt.defered()()
 			}
 			b := &bytes.Buffer{}
-			tt.rootCmd.SetOut(b)
+			rootCmd := tt.rootCmd()
+			rootCmd.SetOut(b)
 			if tt.in != "" {
-				tt.rootCmd.SetIn(strings.NewReader(tt.in))
+				rootCmd.SetIn(strings.NewReader(tt.in))
 			}
-			if err := tt.rootCmd.Execute(); (err != nil) != tt.wantErr {
+			if err := rootCmd.Execute(); (err != nil) != tt.wantErr {
 				t.Errorf("Execute() error = %v, wantErr %v", err, tt.wantErr)
 			}
 
@@ -245,16 +250,99 @@ func Test_mainCmd_run(t *testing.T) {
 	}
 }
 
+func extractCommands(root *cobra.Command) (commands []string) {
+	for _, c := range root.Commands() {
+		commands = append(commands, c.Name())
+	}
+	return commands
+}
+
 func TestAssetsAreAlsoCommands(t *testing.T) {
-	_, rootCmd := makeRootCmd(false)
+	d, err := summon.New(cmdTestFS)
+	assert.NoError(t, err)
+
+	t.Run("assets-are-commands-with-run-cmd", func(t *testing.T) {
+		root, err := CreateRootCmd(d, []string{"program"}, summon.MainOptions{})
+		assert.NoError(t, err)
 
-	commands := []string{}
+		commands := extractCommands(root)
+		assert.Subset(t, commands, []string{"a.txt", "b.txt",
+			"json-for-template.json", "summon.config.yaml"})
+	})
 
-	lsCmd, _, err := rootCmd.Find([]string{"ls"})
+	t.Run("assets-are-commands-without-run-cmd", func(t *testing.T) {
+		root, err := CreateRootCmd(d, []string{"program"}, summon.MainOptions{WithoutRunSubcmd: true})
+		assert.NoError(t, err)
+
+		commands := extractCommands(root)
+		assert.Subset(t, commands, []string{"a.txt", "b.txt",
+			"json-for-template.json", "summon.config.yaml"})
+	})
+}
+
+func TestThatAssetsAreNotCommandsIfConfiguredSo(t *testing.T) {
+	configFile := `hideAssetsInHelp: true`
+
+	testFs := fstest.MapFS{}
+	testFs["assets/"+config.ConfigFileName] = &fstest.MapFile{Data: []byte(configFile)}
+
+	d, err := summon.New(testFs)
 	assert.NoError(t, err)
-	for _, c := range lsCmd.Commands() {
-		commands = append(commands, c.Name())
-	}
 
-	assert.ElementsMatch(t, []string{"a.txt", "b.txt", "json-for-template.json", "summon.config.yaml"}, commands)
+	t.Run("no-config-file-on-root", func(t *testing.T) {
+		root, err := CreateRootCmd(d, []string{"program"}, summon.MainOptions{WithoutRunSubcmd: true})
+		assert.NoError(t, err)
+
+		commands := extractCommands(root)
+		assert.ElementsMatch(t, []string{"completion"}, commands)
+		assert.NotContains(t, commands, config.ConfigFileName)
+	})
+}
+
+func TestThatAssetsAreNotCommandsAndPassedToRun(t *testing.T) {
+	configFile := `hideAssetsInHelp: true
+outputdir: "a"`
+
+	testFs := fstest.MapFS{}
+	testFs["assets/"+config.ConfigFileName] = &fstest.MapFile{Data: []byte(configFile)}
+	testFs["assets/b.txt"] = &fstest.MapFile{Data: []byte("b content")}
+
+	d, err := summon.New(testFs)
+	assert.NoError(t, err)
+
+	t.Run("test-json-exclusivity", func(t *testing.T) {
+		root, err := CreateRootCmd(d, []string{"program", "--json", "{}", "--json-file", "-", "summon.config.yaml"}, summon.MainOptions{WithoutRunSubcmd: true})
+		assert.NoError(t, err)
+
+		err = root.Execute()
+		assert.Error(t, err)
+	})
+
+	t.Run("file-as-arg", func(t *testing.T) {
+		defer testutil.ReplaceFs()()
+
+		root, err := CreateRootCmd(d, []string{"program", "b.txt"}, summon.MainOptions{WithoutRunSubcmd: true})
+		assert.NoError(t, err)
+
+		b := &bytes.Buffer{}
+		root.SetOut(b)
+
+		err = root.Execute()
+		assert.NoError(t, err)
+
+		assert.Contains(t, b.String(), "a/b.txt")
+	})
+
+	t.Run("completion", func(t *testing.T) {
+		root, err := CreateRootCmd(d, []string{"program", cobra.ShellCompNoDescRequestCmd, ""}, summon.MainOptions{WithoutRunSubcmd: true})
+		assert.NoError(t, err)
+
+		b := &bytes.Buffer{}
+		root.SetOut(b)
+
+		err = root.Execute()
+		assert.NoError(t, err)
+
+		assert.Contains(t, b.String(), "b.txt")
+	})
 }

cmd/run_test.go

@@ -41,6 +41,12 @@ func TestRunCmd(t *testing.T) {
 			args:      []string{"run", "ec"},
 			wantError: true,
 		},
+		{
+			desc:      "help-flag-on-root-works",
+			args:      []string{"--help"},
+			wantError: false,
+			noCalls:   true,
+		},
 		{
 			desc: "sub-param-passed",
 			args: []string{"run", "echo", "--unknown-arg", "last", "params"},

internal/scaffold/templates/scaffold/Makefile

@@ -6,6 +6,8 @@ all: bin/$(SUMMONER_NAME)
 
 bin/$(SUMMONER_NAME): $(SUMMONER_NAME)/$(SUMMONER_NAME).go $(ASSETS)
 	go build -o $@ $<
+	@echo testing created $(SUMMONER_NAME)
+	go run $< --help
 
 .PHONY: clean
 clean:

internal/scaffold/templates/scaffold/{{.SummonerName}}/assets/summon.config.yaml

@@ -3,4 +3,7 @@
 version: 1
 aliases: {}
 outputdir: ".summoned"
-exec: {}
+hideAssetsInHelp: false
+exec:
+  flags: {}
+  environments: {}

main.go

@@ -45,7 +45,7 @@ func Main(args []string, fs embed.FS, opts ...option) int {
 	summon.Name = args[0]
 	s, err := summon.New(fs)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "unable to create initial filesystem: %v", err)
+		fmt.Fprintf(os.Stderr, "unable to create initial filesystem: %v\n", err)
 		return 1
 	}
 
@@ -59,7 +59,7 @@ func Main(args []string, fs embed.FS, opts ...option) int {
 
 	rootCmd, err := cmd.CreateRootCmd(s, os.Args, *options)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "could not create command tree: %v", err)
+		fmt.Fprintf(os.Stderr, "could not create command tree: %v\n", err)
 		return 1
 	}
 

pkg/config/config.go

@@ -20,12 +20,12 @@ type Alias map[string]string
 
 // Config is the summon config
 type Config struct {
-	Version         int
-	Aliases         Alias       `yaml:"aliases"`
-	OutputDir       string      `yaml:"outputdir"`
-	TemplateContext string      `yaml:"templates"`
-	Exec            ExecContext `yaml:"exec"`
-	Help            string      `yaml:"help"`
+	Version          int
+	Aliases          Alias       `yaml:"aliases"`
+	OutputDir        string      `yaml:"outputdir"`
+	TemplateContext  string      `yaml:"templates"`
+	Exec             ExecContext `yaml:"exec"`
+	HideAssetsInHelp bool        `yaml:"hideAssetsInHelp"`
 }
 
 // ExecContext houses execution environments and global flags
@@ -54,13 +54,21 @@ type ArgSliceSpec []interface{}
 // Its SubCmd is an ExecDesc so it can be an ArgsSliceSpec or a CmdSpec
 // Its Flags can be a one line string flag or a FlagSpec
 type CmdDesc struct {
-	Args       ArgSliceSpec        `yaml:"args"`
-	SubCmd     map[string]ExecDesc `yaml:"subCmd,omitempty"`
-	Flags      map[string]FlagDesc `yaml:"flags,omitempty"`
-	Help       string              `yaml:"help,omitempty"`
-	Completion string              `yaml:"completion,omitempty"`
-	Hidden     bool                `yaml:"hidden,omitempty"`
-	Inline     *bool               `yaml:"join,omitempty"`
+	// Args contain the args that get appended to the ExecEnvironment
+	Args ArgSliceSpec `yaml:"args"`
+	// SubCmd describes a sub-command of current command
+	SubCmd map[string]ExecDesc `yaml:"subCmd,omitempty"`
+	// Flags of this command
+	Flags map[string]FlagDesc `yaml:"flags,omitempty"`
+	// Help line of this command
+	Help string `yaml:"help,omitempty"`
+	// Completion holds the command to invoke to have a completion of
+	// this command. It can contain templates.
+	Completion string `yaml:"completion,omitempty"`
+	// Hidden hides the command from help
+	Hidden bool `yaml:"hidden,omitempty"`
+	// Join joins arguments to form one argument of one line of text
+	Join *bool `yaml:"join,omitempty"`
 }
 
 // FlagDesc describes a simple string flag or complex FlagSpec

pkg/summon/driver.go

@@ -66,9 +66,8 @@ func New(filesystem fs.FS, opts ...Option) (*Driver, error) {
 	return d, nil
 }
 
-func (d Driver) OutputDir() string {
-	return d.config.OutputDir
-}
+func (d Driver) OutputDir() string      { return d.config.OutputDir }
+func (d Driver) HideAssetsInHelp() bool { return d.config.HideAssetsInHelp }
 
 // Configure is used to extract options and customize the summon.Driver.
 func (d *Driver) Configure(opts ...Option) error {

pkg/summon/run.go

@@ -202,8 +202,8 @@ func normalizeExecDesc(argsDesc interface{}, invoker string) (*commandSpec, erro
 		c.help = descType.Help
 		c.completion = descType.Completion
 		c.hidden = descType.Hidden
-		if descType.Inline != nil {
-			c.join = descType.Inline
+		if descType.Join != nil {
+			c.join = descType.Join
 		}
 		if descType.SubCmd != nil {
 			c.subCmd = make(map[string]*commandSpec)
@@ -433,11 +433,13 @@ func (d *Driver) setupArgs(root *cobra.Command) {
 		managedHelp = append(managedHelp, a)
 	}
 
-	// if help is requested on a managed command, let cobra manage the help flag
+	// if help is requested on:
+	//   * a managed command that has a help line
+	//   * on the root (no parameters)
 	var ownHelp bool
 	if helpFlag != "" {
 		cmd, _, _ := root.Root().Find(allArgs[:helpPos])
-		if cmd != root && cmd.Short != "" {
+		if cmd != root && cmd.Short != "" || len(managedHelp) == 0 {
 			ownHelp = true
 		}
 	}