Skip to content

Commit

Permalink
Merge pull request #8248 from dolthub/macneale4/slash-checkout
Browse files Browse the repository at this point in the history
support \checkout, \merge, \show in `dolt sql` shell.
  • Loading branch information
macneale4 authored Aug 12, 2024
2 parents 182e8c5 + 4d2c95c commit 1db4df7
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 35 deletions.
7 changes: 6 additions & 1 deletion go/cmd/dolt/cli/cli_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ import (

// LateBindQueryist is a function that will be called the first time Queryist is needed for use. Input is a context which
// is appropriate for the call to commence. Output is a Queryist, a sql.Context, a closer function, and an error.
// The closer function is called when the Queryist is no longer needed, typically a defer right after getting it.
//
// The closer function is called when the Queryist is no longer needed, typically a defer right after getting it. If a nil
// closer function is returned, then the caller knows that the queryist returned is being managed by another command. Effectively
// this means you are running in another command's session. This is particularly interesting when running a \checkout in a
// dolt sql session. It makes sense to do so in the context of `dolt sql`, but not in the context of `dolt checkout` when
// connected to a remote server.
type LateBindQueryist func(ctx context.Context) (Queryist, *sql.Context, func(), error)

// CliContexct is used to pass top level command information down to subcommands.
Expand Down
2 changes: 1 addition & 1 deletion go/cmd/dolt/cli/messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ package cli
const (
// Single variable - the name of the command. `dolt <command>` is how the commandString is formated in calls to the Exec method
// for dolt commands.
RemoteUnsupportedMsg = "%s can not currently be used when there is a local server running. Please stop your dolt sql-server and try again."
RemoteUnsupportedMsg = "%s can not currently be used when there is a local server running. Please stop your dolt sql-server or connect using `dolt sql` instead."
)
22 changes: 12 additions & 10 deletions go/cmd/dolt/commands/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,15 +97,17 @@ func (cmd CheckoutCmd) Exec(ctx context.Context, commandStr string, args []strin
}
if closeFunc != nil {
defer closeFunc()
}

_, ok := queryEngine.(*engine.SqlEngine)
if !ok {
// Currently checkout does not fully support remote connections. Prevent them from being used until we have better
// CLI session support.
msg := fmt.Sprintf(cli.RemoteUnsupportedMsg, commandStr)
cli.Println(msg)
return 1
// We only check for this case when checkout is the first command in a session. The reason for this is that checkout
// when connected to a remote server will not work as it won't set the branch. But when operating within the context
// of another session, specifically a \checkout in a dolt sql session, this makes sense. Since no closeFunc would be
// returned, we don't need to check for this case.
_, ok := queryEngine.(*engine.SqlEngine)
if !ok {
msg := fmt.Sprintf(cli.RemoteUnsupportedMsg, commandStr)
cli.Println(msg)
return 1
}
}

// Argument validation in the CLI is strictly nice to have. The stored procedure will do the same, but the errors
Expand Down Expand Up @@ -165,8 +167,8 @@ func (cmd CheckoutCmd) Exec(ctx context.Context, commandStr string, args []strin
return HandleVErrAndExitCode(errhand.BuildDError("no 'message' field in response from %s", sqlQuery).Build(), usage)
}

var message string
if message, ok = rows[0][1].(string); !ok {
message, ok := rows[0][1].(string)
if !ok {
return HandleVErrAndExitCode(errhand.BuildDError("expected string value for 'message' field in response from %s ", sqlQuery).Build(), usage)
}

Expand Down
10 changes: 5 additions & 5 deletions go/cmd/dolt/commands/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,12 @@ func (cmd MergeCmd) Exec(ctx context.Context, commandStr string, args []string,
}
if closeFunc != nil {
defer closeFunc()
}

if _, ok := queryist.(*engine.SqlEngine); !ok {
msg := fmt.Sprintf(cli.RemoteUnsupportedMsg, commandStr)
cli.Println(msg)
return 1
if _, ok := queryist.(*engine.SqlEngine); !ok {
msg := fmt.Sprintf(cli.RemoteUnsupportedMsg, commandStr)
cli.Println(msg)
return 1
}
}

ok := validateDoltMergeArgs(apr, usage, cliCtx)
Expand Down
11 changes: 10 additions & 1 deletion go/cmd/dolt/commands/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu

sqlCtx := sql.NewContext(subCtx, sql.WithSession(sqlCtx.Session))

subCmd, foundCmd := findSlashCmd(query)
subCmd, foundCmd := isSlashQuery(query)
if foundCmd {
err := handleSlashCommand(sqlCtx, subCmd, query, cliCtx)
if err != nil {
Expand Down Expand Up @@ -831,6 +831,15 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu
return nil
}

func isSlashQuery(query string) (cli.Command, bool) {
// strip leading whitespace
query = strings.TrimLeft(query, " \t\n\r\v\f")
if strings.HasPrefix(query, "\\") {
return findSlashCmd(query[1:])
}
return nil, false
}

// postCommandUpdate is a helper function that is run after the shell has completed a command. It updates the the database
// if needed, and generates new prompts for the shell (based on the branch and if the workspace is dirty).
func postCommandUpdate(sqlCtx *sql.Context, qryist cli.Queryist) (string, string) {
Expand Down
1 change: 1 addition & 0 deletions go/cmd/dolt/commands/sql_slash.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var slashCmds = []cli.Command{
StatusCmd{},
DiffCmd{},
LogCmd{},
ShowCmd{},
AddCmd{},
CommitCmd{},
CheckoutCmd{},
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/bats/sql-local-remote.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ SQL
run dolt checkout br
[ $status -eq 1 ]

[[ $output =~ "dolt checkout can not currently be used when there is a local server running. Please stop your dolt sql-server and try again." ]] || false
[[ $output =~ "dolt checkout can not currently be used when there is a local server running. Please stop your dolt sql-server or connect using \`dolt sql\` instead." ]] || false
}

@test "sql-local-remote: verify unmigrated command will fail with warning" {
Expand Down
67 changes: 51 additions & 16 deletions integration-tests/bats/sql-shell-slash-cmds.expect
Original file line number Diff line number Diff line change
Expand Up @@ -24,46 +24,81 @@ proc expect_with_defaults {pattern action} {
}
}
}

proc expect_with_defaults_2 {patternA patternB action} {
# First, match patternA
expect {
-re $patternA {
# puts "Matched pattern: $patternA"
exp_continue
}
-re $patternB {
# puts "Matched pattern: $patternB"
eval $action
puts "<<Matched expected pattern A: $patternA>>"
# Now match patternB
expect {
-re $patternB {
puts "<<Matched expected pattern B: $patternB>>"
eval $action
}
timeout {
puts "<<Timeout waiting for pattern B>>"
exit 1
}
eof {
puts "<<End of File reached while waiting for pattern B>>"
exit 1
}
failed {
puts "<<Failed while waiting for pattern B>>"
exit 1
}
}
}
timeout {
puts "<<Timeout>>";
puts "<<Timeout waiting for pattern A>>"
exit 1
}
eof {
puts "<<End of File reached>>";
puts "<<End of File reached while waiting for pattern A>>"
exit 1
}
failed {
puts "<<Failed>>";
puts "<<Failed while waiting for pattern A>>"
exit 1
}
}
}



spawn dolt sql

expect_with_defaults {dolt-repo-[0-9]+/main\*> } { send "\\commit -A -m \"sql-shell-slash-cmds commit\"\r"; }
expect_with_defaults {dolt-repo-[0-9]+/main\*> } { send "\\commit -A -m \"sql-shell-slash-cmds commit\"\r"; }

expect_with_defaults {dolt-repo-[0-9]+/main> } { send "\\log -n 1;\r"; }

expect_with_defaults_2 {sql-shell-slash-cmds commit} {dolt-repo-[0-9]+/main> } { send "\\status\r"; }

expect_with_defaults_2 {nothing to commit, working tree clean} {dolt-repo-[0-9]+/main> } { send "\\checkout -b br1\r"; }

expect_with_defaults_2 {Switched to branch 'br1'} {dolt-repo-[0-9]+/br1> } { send "\\commit --allow-empty -m \"empty cmt\"\r"; }

expect_with_defaults_2 {empty cmt} {dolt-repo-[0-9]+/br1> } { send "\\checkout main\r"; }

expect_with_defaults_2 {Switched to branch 'main'} {dolt-repo-[0-9]+/main> } { send "\\commit --allow-empty -m \"main cmt\"\r"; }

expect_with_defaults_2 {main cmt} {dolt-repo-[0-9]+/main> } { send "\\merge br1\r"; }

expect_with_defaults_2 {Everything up-to-date} {dolt-repo-[0-9]+/main> } { send "\\show\r"; }

expect_with_defaults_2 {Merge branch 'br1'} {dolt-repo-[0-9]+/main> } { send "\\log -n 3\r"; }

expect_with_defaults_2 {empty cmt} {dolt-repo-[0-9]+/main> } { send "\\checkout br1\r"; }

expect_with_defaults {dolt-repo-[0-9]+/main> } { send "\\log -n 1;\r"; }
expect_with_defaults_2 {Switched to branch 'br1'} {dolt-repo-[0-9]+/br1> } { send "\\merge main\r"; }

expect_with_defaults_2 {sql-shell-slash-cmds commit} {dolt-repo-[0-9]+/main> } { send "\\status\r"; }
expect_with_defaults_2 {Fast-forward} {dolt-repo-[0-9]+/br1> } { send "\\reset HEAD~3;\r"; }

expect_with_defaults {dolt-repo-[0-9]+/main> } { send "\\reset HEAD~1;\r"; }
expect_with_defaults {dolt-repo-[0-9]+/br1\*> } { send "\\diff\r"; }

expect_with_defaults {dolt-repo-[0-9]+/main\*> } { send "\\diff\r"; }
expect_with_defaults_2 {diff --dolt a/test b/test} {dolt-repo-[0-9]+/br1\*> } { send "\\reset main\r"; }

expect_with_defaults_2 {diff --dolt a/tbl b/tbl} {dolt-repo-[0-9]+/main\*> } {send "quit\r";}
expect_with_defaults {dolt-repo-[0-9]+/br1> } { send "quit\r" }

expect eof
exit

0 comments on commit 1db4df7

Please sign in to comment.