feat: Added disclaimer for branch checkout #8957
Conversation
There was a problem hiding this comment.
This needs to be a bit more robust.
Also we need a BATS test in:
https://github.com/dolthub/dolt/blob/main/integration-tests/bats/sql-checkout.bats
Some of those tests will need to be updated because we use dolt sql -q "call dolt_checkout('-b', ...)" a bunch of places.
| @@ -176,6 +178,10 @@ func doDoltCheckout(ctx *sql.Context, args []string) (statusCode int, successMes | |||
| if err != nil { | |||
| return 1, "", err | |||
| } | |||
| terminal_command:=strings.Join(os_args, " ") | |||
There was a problem hiding this comment.
I think we need to be a bit more clever here. I think we need to check and make sure that call dolt_checkout() with the -b option is the only sql statement being run.
There was a problem hiding this comment.
@timsehn
Okay but what if the user calls the command like dolt sql --q "call dolt_checkout('myBranch2');" i.e. without the -b option.
I believe this also needs to be handled.
There was a problem hiding this comment.
The stored procedure can't return color escape codes in its warning (or any other message) since ORMs and other tools don't consistently handle them correctly. Remember that this is an programming interface :-)
I assume you want the terminal experience to show the warning, and in color? That should all be handled either in the CLI commands or in the sql shell.
|
A possible better first step would be: #8875 |
|
@macneale4 So should I remove the color from :- |
|
No, I think the approach doesn't fit with the way the application works. For example 'os.Args[1:]' should definitely not be used anywhere close to the sql engine. There are many layers between where the command line arguments are parsed and the procedure code. And in the case of CLI commands, they are generally in separate processes. The command line is connecting to a server in some cases. I think a better plan it to surface warnings by default in |
To fix #6876