Repository: dolthub/dolt
File: go/cmd/dolt/commands/stash.go (Go)

Code before:
	if len(apr.Args) > 0 {
		subcommand = strings.ToLower(apr.Arg(0))
	}

	queryist, err := cliCtx.QueryEngine(ctx)
	if err != nil {
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		return 1
	}

	idx := 0
	if apr.NArg() > 1 {
		idx, err = parseStashIndex(apr.Arg(1))
		if err != nil {
			cli.PrintErrln(errhand.VerboseErrorFromError(err))
			return 1
		}
	}

	switch subcommand {
	case PushCmdRef:
		err = stashPush(queryist.Queryist, queryist.Context, apr, subcommand)
	case PopCmdRef, DropCmdRef:
		err = stashRemove(queryist.Queryist, queryist.Context, cliCtx, apr, subcommand, idx)
	case ApplyCmdRef:
		err = stashApply(queryist.Queryist, queryist.Context, cliCtx, apr, subcommand, idx)
	case ListCmdRef:
		err = stashList(ctx, cliCtx)
	case ClearCmdRef:
		err = stashClear(queryist.Queryist, queryist.Context, apr, subcommand)
	default:
		err = fmt.Errorf("unknown stash subcommand %s", subcommand)
	}

	if err != nil {
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		if strings.Contains(err.Error(), "No local changes to save") {
			return 0
		}
		return 1
	}
	return 0
}

func stashPush(queryist cli.Queryist, sqlCtx *sql.Context, apr *argparser.ArgParseResults, subcommand string) error {
	rowIter, err := stashQuery(queryist, sqlCtx, apr, subcommand)
	if err != nil {
		return err
	}

	stashes, err := getStashesSQL(sqlCtx, queryist, 1)

Review comment (bug):
Passing the subcommand as an argument is kinda silly. It's always the same thing. The only function which has an actual variable subcommand is stashRemove.

And the stashRemove and stashApply functions are basically the same code. Something smells wrong about this to me. Suggestions to simplify?

Code after:
	if len(apr.Args) > 0 {
		subcommand = strings.ToLower(apr.Arg(0))
	}

	queryist, err := cliCtx.QueryEngine(ctx)
	if err != nil {
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		return 1
	}

	idx := 0
	if apr.NArg() > 1 {
		idx, err = parseStashIndex(apr.Arg(1))
		if err != nil {
			cli.PrintErrln(errhand.VerboseErrorFromError(err))
			return 1
		}
	}

	// List queries a system table, unlike the procedure the other commands use, so we handle it in a special case
	if subcommand == ListCmdRef {
		return stashList(ctx, cliCtx)
	}

	// Pre-query: Pop, Apply, and Drop commands need to confirm that the given id is valid.
	// We'll also check that the subcommand is valid
	var stash *doltdb.Stash
	switch subcommand {
	case PopCmdRef, ApplyCmdRef, DropCmdRef:
		stash, err = validateStashAtIdx(queryist.Queryist, queryist.Context, idx)
		if err != nil {
			cli.PrintErrln(errhand.VerboseErrorFromError(err))
			return 1
		}
	case PushCmdRef, ClearCmdRef:
	default:
		err = fmt.Errorf("unknown stash subcommand %s", subcommand)
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		return 1
	}

	// Now build the call to DOLT_STASH and run it
	interpolatedQuery, err := generateStashSql(apr, subcommand)
	if err != nil {
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		return 1
	}
	_, rowIter, _, err := queryist.Queryist.Query(queryist.Context, interpolatedQuery)
	if err != nil {
		cli.PrintErrln(errhand.VerboseErrorFromError(err))
		if strings.Contains(err.Error(), "No local changes to save") {