Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions documentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func (c *documentationCommand) writeIndex(w io.Writer) error {
}
// TODO: handle subcommands ??
}
_, err = fmt.Fprintf(w, "---\n\n")
_, err = fmt.Fprintf(w, "\n---\n\n")
return err
}

Expand Down Expand Up @@ -357,7 +357,7 @@ func (c *documentationCommand) linkForCommand(cmd string) string {
func (c *documentationCommand) formatCommand(ref commandReference, title bool, commandSeq []string) string {
var fmtedTitle string
if title {
fmtedTitle = strings.ToUpper(strings.Join(commandSeq[1:], " "))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a mess. We're changing the documentation command because the output doesn't match the current implementation of the rendering framework we're using at the time.

If this isn't tail-wagging the dog, I don't know what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on this? On the line you highlighted for example, I made the change as Teodora suggested that titles shouldn't be COMMAND but rather <binary> command, i.e. not the command alone in uppercase, but rather the tool name and then the command in lowercase.
This isn't related to the rendering framework.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change as Teodora suggested that titles shouldn't be COMMAND but rather <binary> command,

That's a breaking change. This package is publicly used and versioned, and as such this should be a new change. We have to hold our changes accountable here. I know I'm being pedantic, and there might not be other consumers of this package that are using this piece of code, but you don't know that.

Assume that you were using this documentation command, and that upstream did change the command output. It's unexpected and surprising, even if it's just "rendering". We should be careful of the consequences of just changing something without seeing it through to the end (a version bump).

I'm not going to stop this landing, but at some-point we should just call it done and come up with a better way. This isn't it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I understand you now. Totally fair, I was working under the impression that, like the juju/juju repo, we weren't so strict with backwards compatibility here. But I see that is not the case after reading your response, so I'll keep that in mind going forward.

fmtedTitle = strings.ToLower(strings.Join(commandSeq, " "))
}

var buf bytes.Buffer
Expand All @@ -380,7 +380,7 @@ func (c *documentationCommand) formatCommand(ref commandReference, title bool, c
return fmt.Sprintf("%s%s", prefix, target)
},
LinkForSubcommand: func(s string) string {
return c.linkForCommand(strings.Join(append(commandSeq[1:], s), "_"))
return c.linkForCommand(strings.Join(append(commandSeq[1:], s), "-"))
},
})
return buf.String()
Expand Down
16 changes: 7 additions & 9 deletions markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,13 @@ func PrintMarkdown(w io.Writer, cmd InfoCommand, opts MarkdownOptions) error {
fmt.Fprintln(&doc)

// Usage
if strings.TrimSpace(info.Args) != "" {
fmt.Fprintln(&doc, "## Usage")
fmt.Fprintf(&doc, "```")
fmt.Fprint(&doc, opts.UsagePrefix)
fmt.Fprintf(&doc, "%s [%ss] %s", info.Name, getFlagsName(info.FlagKnownAs), info.Args)
fmt.Fprintf(&doc, "```")
fmt.Fprintln(&doc)
fmt.Fprintln(&doc)
}
fmt.Fprintln(&doc, "## Usage")
fmt.Fprintf(&doc, "```")
fmt.Fprint(&doc, opts.UsagePrefix)
fmt.Fprintf(&doc, "%s [%ss] %s", info.Name, getFlagsName(info.FlagKnownAs), info.Args)
fmt.Fprintf(&doc, "```")
fmt.Fprintln(&doc)
fmt.Fprintln(&doc)

// Options
printFlags(&doc, cmd)
Expand Down
56 changes: 56 additions & 0 deletions markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,59 @@ func (*markdownSuite) TestOutput(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
c.Check(buf.String(), gc.Equals, string(expected))
}

// TestOutputWithoutArgs tests that the output of the PrintMarkdown function is
// correct when a command does not need arguments, e.g. list commands.
func (*markdownSuite) TestOutputWithoutArgs(c *gc.C) {
seeAlso := []string{"add-cloud", "update-cloud", "remove-cloud", "update-credential"}
subcommands := map[string]string{
"foo": "foo the bar baz",
"bar": "bar the baz foo",
"baz": "baz the foo bar",
}

command := &docTestCommand{
info: &cmd.Info{
Name: "clouds",
Args: "", //Empty args should still result in a usage field.
Purpose: "List clouds.",
Doc: "details for clouds...",
Examples: "examples for clouds...",
SeeAlso: seeAlso,
Aliases: []string{"list-clouds"},
Subcommands: subcommands,
},
}

// These functions verify the provided argument is in the expected set.
linkForCommand := func(s string) string {
for _, cmd := range seeAlso {
if cmd == s {
return "https://docs.com/" + cmd
}
}
c.Fatalf("linkForCommand called with unexpected command %q", s)
return ""
}

linkForSubcommand := func(s string) string {
_, ok := subcommands[s]
if !ok {
c.Fatalf("linkForSubcommand called with unexpected subcommand %q", s)
}
return "https://docs.com/clouds/" + s
}

expected, err := os.ReadFile("testdata/list-clouds.md")
c.Assert(err, jc.ErrorIsNil)

var buf bytes.Buffer
err = cmd.PrintMarkdown(&buf, command, cmd.MarkdownOptions{
Title: `Command "juju clouds"`,
UsagePrefix: "juju ",
LinkForCommand: linkForCommand,
LinkForSubcommand: linkForSubcommand,
})
c.Assert(err, jc.ErrorIsNil)
c.Check(buf.String(), gc.Equals, string(expected))
}
48 changes: 35 additions & 13 deletions supercommand.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package cmd

import (
stderr "errors"
"fmt"
"io/ioutil"
"sort"
Expand All @@ -17,6 +18,11 @@ import (

var logger = loggo.GetLogger("cmd")

// ErrCommandMissing can be returned during the Init() method
// of a command to trigger the super command's missingCallback
// if one is set.
var ErrCommandMissing = stderr.New("missing command")

type topic struct {
short string
long func() string
Expand Down Expand Up @@ -458,24 +464,30 @@ func (c *SuperCommand) Init(args []string) error {
}
found := false

setupMissingCallback := func() {
c.action = commandReference{
command: &missingCommand{
callback: c.missingCallback,
superName: c.Name,
name: args[0],
args: args[1:],
},
}
}

// Look for the command.
if c.action, found = c.subcmds[args[0]]; !found {
if c.missingCallback != nil {
c.action = commandReference{
command: &missingCommand{
callback: c.missingCallback,
superName: c.Name,
name: args[0],
args: args[1:],
},
}
setupMissingCallback()
// Yes return here, no Init called on missing Command.
return nil
}
return fmt.Errorf("unrecognized command: %s %s", c.Name, args[0])
}

args = args[1:]
// Keep the original args
cleanArgs := make([]string, len(args[1:]))
copy(cleanArgs, args[1:])
subcmd := c.action.command
if subcmd.IsSuperCommand() {
f := gnuflag.NewFlagSetWithFlagKnownAs(c.Info().Name, gnuflag.ContinueOnError, FlagAlias(subcmd, "flag"))
Expand All @@ -484,17 +496,27 @@ func (c *SuperCommand) Init(args []string) error {
} else {
subcmd.SetFlags(c.commonflags)
}
if err := c.commonflags.Parse(subcmd.AllowInterspersedFlags(), args); err != nil {
if err := c.commonflags.Parse(subcmd.AllowInterspersedFlags(), cleanArgs); err != nil {
return err
}

args = c.commonflags.Args()
cleanArgs = c.commonflags.Args()
if c.showHelp {
// We want to treat help for the command the same way we would if we went "help foo".
args = []string{c.action.name}
cleanArgs = []string{c.action.name}
c.action = c.subcmds["help"]
}
return c.action.command.Init(args)

err := c.action.command.Init(cleanArgs)

// Commands may intentionally return a command missing
// error during init to trigger their missing callback.
if !stderr.Is(err, ErrCommandMissing) || c.missingCallback == nil {
return err
}

setupMissingCallback()
return nil
}

// Run executes the subcommand that was selected in Init.
Expand Down
46 changes: 46 additions & 0 deletions supercommand_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,52 @@ func (s *SuperCommandSuite) TestMissingCallbackContextWiredIn(c *gc.C) {
c.Assert(cmdtesting.Stderr(s.ctx), gc.Equals, "this is std err")
}

type simpleWithInitError struct {
cmd.CommandBase
name string
initError error
}

var _ cmd.Command = (*simpleWithInitError)(nil)

func (s *simpleWithInitError) Info() *cmd.Info {
return &cmd.Info{Name: s.name, Purpose: "to be simple"}
}

func (s *simpleWithInitError) Init(args []string) error {
return s.initError
}

func (s *simpleWithInitError) Run(_ *cmd.Context) error {
return errors.New("unexpected-error")
}

func (s *SuperCommandSuite) TestMissingCallbackSetOnError(c *gc.C) {
callback := func(ctx *cmd.Context, subcommand string, args []string) error {
fmt.Fprint(ctx.Stdout, "reached callback: "+strings.Join(args, " "))
return nil
}

jc := cmd.NewSuperCommand(cmd.SuperCommandParams{
Name: "jujutest",
Log: &cmd.Log{},
MissingCallback: callback,
})
jc.Register(&simpleWithInitError{name: "foo", initError: cmd.ErrCommandMissing})
jc.Register(&simpleWithInitError{name: "bar", initError: errors.New("my-fake-error")})

code := cmd.Main(jc, s.ctx, []string{"bar"})
c.Assert(code, gc.Equals, 2)
c.Assert(cmdtesting.Stderr(s.ctx), gc.Equals, "ERROR my-fake-error\n")

// Verify that a call to foo, which returns a ErrCommandMissing error
// triggers the command missing callback and ensure all expected
// args were correctly sent to the callback.
code = cmd.Main(jc, s.ctx, []string{"foo", "bar", "baz", "--debug"})
c.Assert(code, gc.Equals, 0)
c.Assert(cmdtesting.Stdout(s.ctx), gc.Equals, "reached callback: bar baz --debug")
}

func (s *SuperCommandSuite) TestSupercommandAliases(c *gc.C) {
jc := cmd.NewSuperCommand(cmd.SuperCommandParams{
Name: "jujutest",
Expand Down
23 changes: 23 additions & 0 deletions testdata/list-clouds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Command "juju clouds"

> See also: [add-cloud](https://docs.com/add-cloud), [update-cloud](https://docs.com/update-cloud), [remove-cloud](https://docs.com/remove-cloud), [update-credential](https://docs.com/update-credential)

**Aliases:** list-clouds

## Summary
List clouds.

## Usage
```juju clouds [options] ```

## Examples
examples for clouds...

## Details
details for clouds...

## Subcommands
- [bar](https://docs.com/clouds/bar)
- [baz](https://docs.com/clouds/baz)
- [foo](https://docs.com/clouds/foo)