Fix: space delimited args for -i and -v (#440)

Author: shueybubbles

README.md

@@ -121,6 +121,9 @@ The `sqlcmd` project aims to be a complete port of the original ODBC sqlcmd to t
 
 ### Changes in behavior from the ODBC based sqlcmd
 
+- `/` is not accepted as a flag specifier, only `-`
+- There are new posix-style versions of each flag, such as `--input-file` for `-i`. `sqlcmd -?` will print those parameter names. Those new names do not preserve backward compatibility with ODBC `sqlcmd`. For example, to specify multiple input file names using `--input-file`, the file names must be comma-delimited, not space-delimited.
+
 The following switches have different behavior in this version of `sqlcmd` compared to the original ODBC based `sqlcmd`.
 - `-r` requires a 0 or 1 argument
 - `-R` switch is ignored. The go runtime does not provide access to user locale information, and it's not readily available through syscall on all supported platforms.
@@ -133,19 +136,19 @@ The following switches have different behavior in this version of `sqlcmd` compa
 - `-u` The generated Unicode output file will have the UTF16 Little-Endian Byte-order mark (BOM) written to it.
 - Some behaviors that were kept to maintain compatibility with `OSQL` may be changed, such as alignment of column headers for some data types.
 - All commands must fit on one line, even `EXIT`. Interactive mode will not check for open parentheses or quotes for commands and prompt for successive lines. The ODBC sqlcmd allows the query run by `EXIT(query)` to span multiple lines.
-- `-i` now requires multiple arguments for the switch to be separated by `,`.
-- `-v` requires multiple variable setters to be comma-separated. eg: `-v var1=v1,var2=v2 -v "var3=v 3"`
+- `-i` doesn't handle a comma `,` in a file name correctly unless the file name argument is triple quoted. For example:
+  `sqlcmd -i """select,100.sql"""` will try to open a file named `sql,100.sql` while `sqlcmd -i "select,100.sql"` will try to open two files `select` and `100.sql`
+- If using a single `-i` flag  to pass multiple file names, there must be a space after the `-i`. Example: `-i file1.sql file2.sql`
 - `-M` switch is ignored. Sqlcmd always enables multi-subnet failover.
 
+
 ### Switches not available in the new sqlcmd (go-sqlcmd) yet
 
 There are a few switches yet to be implemented in the new `sqlcmd` (go-sqlcmd) compared
 to the original ODBC based `sqlcmd`, discussion [#293](https://github.com/microsoft/go-sqlcmd/discussions/292) 
 lists these switches. Please provide feedback in the discussion on which 
 switches are most important to you to have implemented next in the new sqlcmd.
 
-Also, the XML Output command `:XML [On]|[Off]` is not implemented yet
-in the new sqlcmd (go-sqlcmd).
 
 ### Miscellaneous enhancements
 

cmd/sqlcmd/sqlcmd.go

@@ -218,11 +218,45 @@ func Execute(version string) {
 			fmt.Println()
 		})
 	})
+	rootCmd.SetArgs(convertOsArgs(os.Args[1:]))
 	if err := rootCmd.Execute(); err != nil {
 		os.Exit(1)
 	}
 }
 
+// We need to rewrite the arguments to add -i and -v in front of each space-delimited value to be Cobra-friendly.
+func convertOsArgs(args []string) (cargs []string) {
+	flag := ""
+	first := true
+	for _, a := range args {
+		if flag != "" {
+			// If the user has a file named "-i" the only way they can pass it on the command line
+			// is with triple quotes: sqlcmd -i """-i""" which will convince the flags parser to
+			// inject `"-i"` into the string slice. Same for any file with a comma in its name.
+			if isFlag(a) {
+				flag = ""
+			} else if !first {
+				cargs = append(cargs, flag)
+			}
+			first = false
+		}
+		if isListFlag(a) {
+			flag = a
+			first = true
+		}
+		cargs = append(cargs, a)
+	}
+	return
+}
+
+func isFlag(arg string) bool {
+	return len(arg) == 2 && arg[0] == '-'
+}
+
+func isListFlag(arg string) bool {
+	return arg == "-v" || arg == "-i"
+}
+
 func formatDescription(description string, maxWidth, indentWidth int) string {
 	var lines []string
 	words := strings.Fields(description)

cmd/sqlcmd/sqlcmd_test.go

@@ -31,6 +31,9 @@ func TestValidCommandLineToArgsConversion(t *testing.T) {
 		{[]string{}, func(args SQLCmdArguments) bool {
 			return args.Server == "" && !args.UseTrustedConnection && args.UserName == "" && args.ScreenWidth == nil && args.ErrorsToStderr == -1 && args.EncryptConnection == "default"
 		}},
+		{[]string{"-v", "a=b", "x=y", "-E"}, func(args SQLCmdArguments) bool {
+			return len(args.Variables) == 2 && args.Variables["a"] == "b" && args.Variables["x"] == "y" && args.UseTrustedConnection
+		}},
 		{[]string{"-c", "MYGO", "-C", "-E", "-i", "file1", "-o", "outfile", "-i", "file2"}, func(args SQLCmdArguments) bool {
 			return args.BatchTerminator == "MYGO" && args.TrustServerCertificate && len(args.InputFile) == 2 && strings.HasSuffix(args.OutputFile, "outfile")
 		}},
@@ -84,6 +87,15 @@ func TestValidCommandLineToArgsConversion(t *testing.T) {
 		{[]string{"-y", "100", "-Y", "200", "-P", "placeholder"}, func(args SQLCmdArguments) bool {
 			return *args.FixedTypeWidth == 200 && *args.VariableTypeWidth == 100 && args.Password == "placeholder"
 		}},
+		{[]string{"-E", "-v", "a=b", "x=y", "-i", "a.sql", "b.sql", "-v", "f=g", "-i", "c.sql", "-C", "-v", "ab=cd", "ef=hi"}, func(args SQLCmdArguments) bool {
+			return args.UseTrustedConnection && args.Variables["x"] == "y" && len(args.InputFile) == 3 && args.InputFile[0] == "a.sql" && args.TrustServerCertificate
+		}},
+		{[]string{"-i", `comma,text.sql`}, func(args SQLCmdArguments) bool {
+			return args.InputFile[0] == "comma" && args.InputFile[1] == "text.sql"
+		}},
+		{[]string{"-i", `"comma,text.sql"`}, func(args SQLCmdArguments) bool {
+			return args.InputFile[0] == "comma,text.sql"
+		}},
 	}
 
 	for _, test := range commands {
@@ -105,7 +117,7 @@ func TestValidCommandLineToArgsConversion(t *testing.T) {
 		cmd.SetOut(new(bytes.Buffer))
 		cmd.SetErr(new(bytes.Buffer))
 		setFlags(cmd, arguments)
-		cmd.SetArgs(test.commandLine)
+		cmd.SetArgs(convertOsArgs(test.commandLine))
 		err := cmd.Execute()
 		msg := ""
 		if err != nil {
@@ -479,6 +491,33 @@ func TestStartupScript(t *testing.T) {
 	}
 }
 
+func TestConvertOsArgs(t *testing.T) {
+	type test struct {
+		name     string
+		in       []string
+		expected []string
+	}
+
+	tests := []test{
+		{
+			"Multiple variables/one switch",
+			[]string{"-E", "-v", "a=b", "x=y", "f=g", "-C"},
+			[]string{"-E", "-v", "a=b", "-v", "x=y", "-v", "f=g", "-C"},
+		},
+		{
+			"Multiple variables and files/multiple switches",
+			[]string{"-E", "-v", "a=b", "x=y", "-i", "a.sql", "b.sql", "-v", "f=g", "-i", "c.sql", "-C", "-v", "ab=cd", "ef=hi"},
+			[]string{"-E", "-v", "a=b", "-v", "x=y", "-i", "a.sql", "-i", "b.sql", "-v", "f=g", "-i", "c.sql", "-C", "-v", "ab=cd", "-v", "ef=hi"},
+		},
+	}
+	for _, c := range tests {
+		t.Run(c.name, func(t *testing.T) {
+			actual := convertOsArgs(c.in)
+			assert.ElementsMatch(t, c.expected, actual, "Incorrect converted args")
+		})
+	}
+}
+
 // Assuming public Azure, use AAD when SQLCMDUSER environment variable is not set
 func canTestAzureAuth() bool {
 	server := os.Getenv(sqlcmd.SQLCMDSERVER)