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

Code before:
	}

	queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx)
	if err != nil {
		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
	}
	if closeFunc != nil {
		defer closeFunc()
	}

	if apr.Contains(cli.SystemFlag) {
		_, _, _, err = queryist.Query(sqlCtx, "SET @@dolt_show_system_tables = 1")
		if err != nil {
			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
		}
	}

	dArgs, err := parseDiffArgs(queryist, sqlCtx, apr)
	if err != nil {
		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
	}

	verr = diffUserTables(queryist, sqlCtx, dArgs)

	if apr.Contains(cli.SystemFlag) {
		_, _, _, err = queryist.Query(sqlCtx, "SET @@dolt_show_system_tables = 0")
		if err != nil {
			return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
		}
	}
	return HandleVErrAndExitCode(verr, usage)
}

func (cmd DiffCmd) validateArgs(apr *argparser.ArgParseResults) errhand.VerboseError {
	if apr.Contains(StatFlag) || apr.Contains(SummaryFlag) {
		if apr.Contains(SchemaFlag) || apr.Contains(DataFlag) {
			return errhand.BuildDError("invalid Arguments: --stat and --summary cannot be combined with --schema or --data").Build()
		}
	}

	if apr.Contains(NameOnlyFlag) {
		if apr.Contains(SchemaFlag) || apr.Contains(DataFlag) || apr.Contains(StatFlag) || apr.Contains(SummaryFlag) {
			return errhand.BuildDError("invalid Arguments: --name-only cannot be combined with --schema, --data, --stat, or --summary").Build()
		}
	}

	f, _ := apr.GetValue(FormatFlag)
	switch strings.ToLower(f) {
	case "tabular", "sql", "json", "":
	default:
		return errhand.BuildDError("invalid output format: %s", f).Build()

Review comment (suggestion):
The only reason to unset the flag (I think) is because the session is continuous in the shell, so the `\diff` operation would be sticky from one invocation to the next. Is that the reason for this?

One thing to call out is that if a user sets that flag, then `\diff --system ...` will unset it, which would be confusing.

Code after:
	}

	queryist, sqlCtx, closeFunc, err := cliCtx.QueryEngine(ctx)
	if err != nil {
		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
	}
	if closeFunc != nil {
		defer closeFunc()
	}

	updateSystemVar, err := setSystemVar(queryist, sqlCtx, apr.Contains(cli.SystemFlag))
	if err != nil {
		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
	}

	dArgs, err := parseDiffArgs(queryist, sqlCtx, apr)
	if err != nil {
		return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
	}

	verr = diffUserTables(queryist, sqlCtx, dArgs)
	if verr != nil {
		return HandleVErrAndExitCode(verr, usage)
	}

	if updateSystemVar != nil {
		err = updateSystemVar()
	}

	return HandleVErrAndExitCode(errhand.VerboseErrorFromError(err), usage)
}

// setSystemVar sets the @@dolt_show_system_tables variable if necessary, and returns the value it must be
// set to after the commands completion, if necessary.
func setSystemVar(queryist cli.Queryist, sqlCtx *sql.Context, showSystem bool) (func() error, error) {
	_, rowIter, _, err := queryist.Query(sqlCtx, "SHOW VARIABLES WHERE VARIABLE_NAME='dolt_show_system_tables'")
	if err != nil {
		return nil, err
	}

	row, err := sql.RowIterToRows(sqlCtx, rowIter)
	if err != nil {
		return nil, err
	}
	prevVal, err := GetInt8ColAsBool(row[0][1])
	if err != nil {
		return nil, err
	}

	newVal := false
	var update func() error