Skip to content

Commit

Permalink
Merge pull request #5078 from vvoland/hint-no-empty
Browse files Browse the repository at this point in the history
plugins/hooks: Don't show empty hook messages
  • Loading branch information
vvoland committed May 20, 2024
2 parents 803b980 + 296a6f5 commit 49c0e19
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 1 deletion.
27 changes: 26 additions & 1 deletion cli-plugins/manager/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/docker/cli/cli-plugins/hooks"
"github.com/docker/cli/cli/command"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)
Expand Down Expand Up @@ -100,11 +101,35 @@ func invokeAndCollectHooks(ctx context.Context, dockerCli command.Cli, rootCmd,
if err != nil {
continue
}
nextSteps = append(nextSteps, processedHook...)

var appended bool
nextSteps, appended = appendNextSteps(nextSteps, processedHook)
if !appended {
logrus.Debugf("Plugin %s responded with an empty hook message %q. Ignoring.", pluginName, string(hookReturn))
}
}
return nextSteps
}

// appendNextSteps appends the processed hook output to the nextSteps slice.
// If the processed hook output is empty, it is not appended.
// Empty lines are not stripped if there's at least one non-empty line.
func appendNextSteps(nextSteps []string, processed []string) ([]string, bool) {
empty := true
for _, l := range processed {
if strings.TrimSpace(l) != "" {
empty = false
break
}
}

if empty {
return nextSteps, false
}

return append(nextSteps, processed...), true
}

// pluginMatch takes a plugin configuration and a string representing the
// command being executed (such as 'image ls' – the root 'docker' is omitted)
// and, if the configuration includes a hook for the invoked command, returns
Expand Down
33 changes: 33 additions & 0 deletions cli-plugins/manager/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)

func TestGetNaiveFlags(t *testing.T) {
Expand Down Expand Up @@ -108,3 +109,35 @@ func TestPluginMatch(t *testing.T) {
assert.Equal(t, match, tc.expectedMatch)
}
}

func TestAppendNextSteps(t *testing.T) {
testCases := []struct {
processed []string
expectedOut []string
}{
{
processed: []string{},
expectedOut: []string{},
},
{
processed: []string{"", ""},
expectedOut: []string{},
},
{
processed: []string{"Some hint", "", "Some other hint"},
expectedOut: []string{"Some hint", "", "Some other hint"},
},
{
processed: []string{"Hint 1", "Hint 2"},
expectedOut: []string{"Hint 1", "Hint 2"},
},
}

for _, tc := range testCases {
t.Run("", func(t *testing.T) {
got, appended := appendNextSteps([]string{}, tc.processed)
assert.Check(t, is.DeepEqual(got, tc.expectedOut))
assert.Check(t, is.Equal(appended, len(got) > 0))
})
}
}

0 comments on commit 49c0e19

Please sign in to comment.