From 6bd898ee542b7b3c3085d2a4c128f4052329ffc4 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 3 Jun 2020 17:45:20 -0500 Subject: [PATCH 1/2] start on alias delete --- command/alias.go | 47 +++++++++++++++++++++++++++--- command/alias_test.go | 51 +++++++++++++++++++++++++++++---- internal/config/alias_config.go | 8 +++++- internal/config/config_type.go | 15 ++++++++++ 4 files changed, 110 insertions(+), 11 deletions(-) diff --git a/command/alias.go b/command/alias.go index 26272d3b0..2922a7830 100644 --- a/command/alias.go +++ b/command/alias.go @@ -14,6 +14,7 @@ func init() { RootCmd.AddCommand(aliasCmd) aliasCmd.AddCommand(aliasSetCmd) aliasCmd.AddCommand(aliasListCmd) + aliasCmd.AddCommand(aliasDeleteCmd) } var aliasCmd = &cobra.Command{ @@ -23,10 +24,8 @@ var aliasCmd = &cobra.Command{ var aliasSetCmd = &cobra.Command{ Use: "set ", - // NB: Even when inside of a single-quoted string, cobra was noticing and parsing any flags - // used in an alias expansion string argument. Since this command needs no flags, I disabled their - // parsing. If we ever want to add flags to alias set we'll have to figure this out. I tested on - // linux in various shells against cobra 1.0; others on macos did /not/ see the same behavior. + // NB: this allows a user to eschew quotes when specifiying an alias expansion. We'll have to + // revisit it if we ever want to add flags to alias set but we have no current plans for that. DisableFlagParsing: true, Short: "Create a shortcut for a gh command", Long: `This command lets you write your own shortcuts for running gh. They can be simple strings or accept placeholder arguments.`, @@ -166,3 +165,43 @@ func aliasList(cmd *cobra.Command, args []string) error { return tp.Render() } + +var aliasDeleteCmd = &cobra.Command{ + Use: "delete ", + Short: "Delete an alias.", + Args: cobra.ExactArgs(1), + Example: "gh alias delete co", + RunE: aliasDelete, +} + +func aliasDelete(cmd *cobra.Command, args []string) error { + alias := args[0] + + ctx := contextForCommand(cmd) + cfg, err := ctx.Config() + if err != nil { + return fmt.Errorf("couldn't read config: %w", err) + } + + aliasCfg, err := cfg.Aliases() + if err != nil { + return fmt.Errorf("couldn't read aliases config: %w", err) + } + + if !aliasCfg.Exists(alias) { + return fmt.Errorf("no such alias %s", alias) + } + + expansion := aliasCfg.Get(alias) + + err = aliasCfg.Delete(alias) + if err != nil { + return fmt.Errorf("failed to delete alias %s: %w", alias, err) + } + + out := colorableOut(cmd) + redCheck := utils.Red("✓") + fmt.Fprintf(out, "%s Deleted alias %s; was %s\n", redCheck, alias, expansion) + + return nil +} diff --git a/command/alias_test.go b/command/alias_test.go index e8c5f8877..a0d894e20 100644 --- a/command/alias_test.go +++ b/command/alias_test.go @@ -183,10 +183,6 @@ aliases: func TestExpandAlias(t *testing.T) { cfg := `--- -hosts: - github.com: - user: OWNER - oauth_token: token123 aliases: co: pr checkout il: issue list --author="$1" --label="$2" @@ -266,11 +262,9 @@ aliases: initBlankContext(cfg, "OWNER/REPO", "trunk") output, err := RunCommand("alias list") - if err != nil { t.Fatalf("unexpected error: %s", err) } - expected := `clone repo clone co pr checkout cs config set editor 'quoted path' @@ -280,3 +274,48 @@ prs pr status eq(t, output.String(), expected) } + +func TestAliasDelete_nonexistent_command(t *testing.T) { + cfg := `--- +aliases: + co: pr checkout + il: issue list --author="$1" --label="$2" + ia: issue list --author="$1" --assignee="$1" +` + initBlankContext(cfg, "OWNER/REPO", "trunk") + + _, err := RunCommand("alias delete cool") + if err == nil { + t.Fatalf("expected error") + } + + eq(t, err.Error(), "no such alias cool") +} + +func TestAliasDelete(t *testing.T) { + cfg := `--- +aliases: + co: pr checkout + il: issue list --author="$1" --label="$2" + ia: issue list --author="$1" --assignee="$1" +` + initBlankContext(cfg, "OWNER/REPO", "trunk") + + mainBuf := bytes.Buffer{} + hostsBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, &hostsBuf)() + + output, err := RunCommand("alias delete co") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + test.ExpectLines(t, output.String(), "Deleted alias co; was pr checkout") + + expected := `aliases: + il: issue list --author="$1" --label="$2" + ia: issue list --author="$1" --assignee="$1" +` + + eq(t, mainBuf.String(), expected) +} diff --git a/internal/config/alias_config.go b/internal/config/alias_config.go index 979ab3b9a..fd99006c8 100644 --- a/internal/config/alias_config.go +++ b/internal/config/alias_config.go @@ -39,7 +39,13 @@ func (a *AliasConfig) Add(alias, expansion string) error { } func (a *AliasConfig) Delete(alias string) error { - // TODO when we get to gh alias delete + a.RemoveEntry(alias) + + err := a.Parent.Write() + if err != nil { + return fmt.Errorf("failed to write config: %w", err) + } + return nil } diff --git a/internal/config/config_type.go b/internal/config/config_type.go index f13666364..8db26f965 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -100,6 +100,21 @@ func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) { return ce, &NotFoundError{errors.New("not found")} } +func (cm *ConfigMap) RemoveEntry(key string) { + newContent := []*yaml.Node{} + + content := cm.Root.Content + for i := 0; i < len(content); i++ { + if content[i].Value == key { + i++ // skip the next node which is this key's value + } else { + newContent = append(newContent, content[i]) + } + } + + cm.Root.Content = newContent +} + func NewConfig(root *yaml.Node) Config { return &fileConfig{ ConfigMap: ConfigMap{Root: root.Content[0]}, From 8963d80942922928913e609f44bf2f795c02b06a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 5 Jun 2020 13:09:04 -0500 Subject: [PATCH 2/2] better get func --- command/alias.go | 11 ++++++----- command/root.go | 4 ++-- internal/config/alias_config.go | 12 +++--------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/command/alias.go b/command/alias.go index 2922a7830..da005a4ab 100644 --- a/command/alias.go +++ b/command/alias.go @@ -73,11 +73,12 @@ func aliasSet(cmd *cobra.Command, args []string) error { successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓")) - if aliasCfg.Exists(alias) { + oldExpansion, ok := aliasCfg.Get(alias) + if ok { successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", utils.Green("✓"), utils.Bold(alias), - utils.Bold(aliasCfg.Get(alias)), + utils.Bold(oldExpansion), utils.Bold(expansionStr), ) } @@ -188,11 +189,11 @@ func aliasDelete(cmd *cobra.Command, args []string) error { return fmt.Errorf("couldn't read aliases config: %w", err) } - if !aliasCfg.Exists(alias) { + expansion, ok := aliasCfg.Get(alias) + if !ok { return fmt.Errorf("no such alias %s", alias) - } - expansion := aliasCfg.Get(alias) + } err = aliasCfg.Delete(alias) if err != nil { diff --git a/command/root.go b/command/root.go index db76509ed..e7cea4c1a 100644 --- a/command/root.go +++ b/command/root.go @@ -464,9 +464,9 @@ func ExpandAlias(args []string) ([]string, error) { return empty, err } - if aliases.Exists(args[1]) { + expansion, ok := aliases.Get(args[1]) + if ok { extraArgs := []string{} - expansion := aliases.Get(args[1]) for i, a := range args[2:] { if !strings.Contains(expansion, "$") { extraArgs = append(extraArgs, a) diff --git a/internal/config/alias_config.go b/internal/config/alias_config.go index fd99006c8..148eb21f9 100644 --- a/internal/config/alias_config.go +++ b/internal/config/alias_config.go @@ -9,19 +9,13 @@ type AliasConfig struct { Parent Config } -func (a *AliasConfig) Exists(alias string) bool { +func (a *AliasConfig) Get(alias string) (string, bool) { if a.Empty() { - return false + return "", false } value, _ := a.GetStringValue(alias) - return value != "" -} - -func (a *AliasConfig) Get(alias string) string { - value, _ := a.GetStringValue(alias) - - return value + return value, value != "" } func (a *AliasConfig) Add(alias, expansion string) error {