Skip to content
Merged
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
2 changes: 2 additions & 0 deletions cmd/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command {
clients.Config.SetFlags(cmd)
return cmdutil.IsValidProjectDirectory(clients)
},
// DEPRECATED(semver:major): remove RunE so this command prints help like other parent commands
// and remove "manifest" from the ignore list in internal/update/update.go
RunE: func(cmd *cobra.Command, args []string) error {
return runInfoCommand(cmd, clients)
},
Expand Down
29 changes: 23 additions & 6 deletions internal/update/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import (
"net/http"
"os"
"reflect"
"strings"
"sync"
"time"

"github.com/slackapi/slack-cli/internal/api"
"github.com/slackapi/slack-cli/internal/goutils"
"github.com/slackapi/slack-cli/internal/shared"
"github.com/slackapi/slack-cli/internal/slackerror"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -223,13 +223,30 @@ func (u *UpdateNotification) isCI() bool {

// isIgnoredCommand returns true when the process is in the list of commands.
func (u *UpdateNotification) isIgnoredCommand() bool {
ignoredCommands := []string{"_fingerprint", "api", "version"}
osStr := os.Args[0:]
if len(osStr) < 2 {
// "manifest" is included because it's an alias that runs "manifest info"
ignoredCommands := []string{"_fingerprint", "api", "manifest", "manifest info", "version"}
if len(os.Args) < 2 {
return false
}
commandName := osStr[1]
return goutils.Contains(ignoredCommands, commandName, true)
commandStr := strings.Join(os.Args[1:], " ")
for _, ignored := range ignoredCommands {
if commandStr == ignored {
return true
}
// Match commands with additional flags (e.g. "manifest info --source local")
// but not subcommands (e.g. "manifest validate" should not match "manifest")
if strings.HasPrefix(commandStr, ignored+" ") {
rest := commandStr[len(ignored)+1:]
if strings.HasPrefix(rest, "-") {
return true
}
// Allow prefix match for multi-word commands (e.g. "manifest info --flag")
if strings.Contains(ignored, " ") {
return true
}
}
}
return false
}

// isLastUpdateCheckedAtGreaterThan returns true when the time since the last update check is greater
Expand Down
23 changes: 22 additions & 1 deletion internal/update/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package update
import (
"context"
"os"
"strings"
"testing"

"github.com/slackapi/slack-cli/internal/config"
Expand Down Expand Up @@ -165,14 +166,34 @@ func Test_UpdateNotification_isIgnoredCommand(t *testing.T) {
command: "version",
expected: true,
},
"manifest command": {
command: "manifest",
expected: true,
},
"manifest command with flags": {
command: "manifest --source local",
expected: true,
},
"manifest info command": {
command: "manifest info",
expected: true,
},
"manifest info command with flags": {
command: "manifest info --source local",
expected: true,
},
"manifest validate command": {
command: "manifest validate",
expected: false,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👁️‍🗨️ praise: Thanks! This makes me think much about tests as spec more than unit.

},
"auth command": {
command: "auth",
expected: false,
},
} {
t.Run(name, func(t *testing.T) {
if tc.command != "" {
os.Args = []string{"placeholder", tc.command}
os.Args = append([]string{"placeholder"}, strings.Split(tc.command, " ")...)
} else {
os.Args = []string{"placeholder"}
}
Expand Down
Loading