Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh alias delete #1103

Merged
merged 2 commits into from
Jun 5, 2020
Merged

gh alias delete #1103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bd898e
start on alias delete
vilmibm Jun 3, 2020
8963d80
better get func
vilmibm Jun 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 46 additions & 6 deletions command/alias.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func init() {
RootCmd.AddCommand(aliasCmd)
aliasCmd.AddCommand(aliasSetCmd)
aliasCmd.AddCommand(aliasListCmd)
aliasCmd.AddCommand(aliasDeleteCmd)
}

var aliasCmd = &cobra.Command{
Expand All @@ -23,10 +24,8 @@ var aliasCmd = &cobra.Command{

var aliasSetCmd = &cobra.Command{
Use: "set <alias> <expansion>",
// 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.`,
Expand Down Expand Up @@ -74,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),
)
}
Expand Down Expand Up @@ -166,3 +166,43 @@ func aliasList(cmd *cobra.Command, args []string) error {

return tp.Render()
}

var aliasDeleteCmd = &cobra.Command{
Use: "delete <alias>",
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)
}

expansion, ok := aliasCfg.Get(alias)
if !ok {
return fmt.Errorf("no such alias %s", 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
}
51 changes: 45 additions & 6 deletions command/alias_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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'
Expand All @@ -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)
}
4 changes: 2 additions & 2 deletions command/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions internal/config/alias_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -39,7 +33,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
}

Expand Down
15 changes: 15 additions & 0 deletions internal/config/config_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

This confused me, but I think it's mostly because iterating over YAML nodes is weird. But from what I can find this is just how it works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sadly yes this is how it works 😅

} else {
newContent = append(newContent, content[i])
}
}

cm.Root.Content = newContent
}

func NewConfig(root *yaml.Node) Config {
return &fileConfig{
ConfigMap: ConfigMap{Root: root.Content[0]},
Expand Down