Address Copilot review comments (round 7)

- Add validation to error when -f arg is non-empty but no codepage parsed
  (e.g., ',' or whitespace-only input)
- Add unit tests for comma-only, whitespace-only, and multiple-comma inputs
- Fix misleading BOM comments to accurately describe BOMOverride behavior
- Remove unused skipOnEncError field from test table

Author: dlevy-msft-sql

pkg/sqlcmd/codepage.go

@@ -155,6 +155,12 @@ func ParseCodePage(arg string) (*CodePageSettings, error) {
 		}
 	}
 
+	// If a non-empty argument was provided but no codepage was parsed,
+	// treat this as an error rather than silently disabling codepage handling.
+	if settings.InputCodePage == 0 && settings.OutputCodePage == 0 {
+		return nil, localizer.Errorf("invalid codepage: %s", arg)
+	}
+
 	// Validate codepages
 	if settings.InputCodePage != 0 {
 		if _, err := GetEncoding(settings.InputCodePage); err != nil {

pkg/sqlcmd/codepage_test.go

@@ -92,6 +92,24 @@ func TestParseCodePage(t *testing.T) {
 			wantErr:     true,
 			errContains: "codepage", // Error message varies by platform
 		},
+		{
+			name:        "comma only produces no codepage",
+			arg:         ",",
+			wantErr:     true,
+			errContains: "invalid codepage",
+		},
+		{
+			name:        "whitespace only produces no codepage",
+			arg:         "   ",
+			wantErr:     true,
+			errContains: "invalid codepage",
+		},
+		{
+			name:        "multiple commas produce no codepage",
+			arg:         ",,,",
+			wantErr:     true,
+			errContains: "invalid codepage",
+		},
 		{
 			name:       "Japanese Shift JIS",
 			arg:        "932",

pkg/sqlcmd/commands_test.go

@@ -461,11 +461,10 @@ func TestExitCommandAppendsParameterToCurrentBatch(t *testing.T) {
 
 func TestOutputCodePageCommand(t *testing.T) {
 	tests := []struct {
-		name           string
-		codepage       int
-		expectedBytes  []byte
-		inputText      string
-		skipOnEncError bool
+		name          string
+		codepage      int
+		expectedBytes []byte
+		inputText     string
 	}{
 		{
 			name:          "UTF-8 output",

pkg/sqlcmd/sqlcmd.go

@@ -352,12 +352,12 @@ func (s *Sqlcmd) IncludeFile(path string, processAll bool) error {
 				reader = transform.NewReader(f, enc.NewDecoder())
 			}
 		} else {
-			// UTF-8 codepage: still apply BOM stripping
+			// UTF-8 codepage: use BOMOverride to strip UTF-8 BOM and auto-detect UTF-16 BOMs, defaulting to UTF-8 otherwise
 			utf8bom := unicode.BOMOverride(unicode.UTF8.NewDecoder())
 			reader = transform.NewReader(f, utf8bom)
 		}
 	} else {
-		// Default: auto-detect BOM for UTF-16, fallback to UTF-8
+		// Default: auto-detect BOMs (UTF-8/UTF-16) and decode accordingly, falling back to UTF-8 when no BOM is present
 		utf16bom := unicode.BOMOverride(unicode.UTF8.NewDecoder())
 		reader = transform.NewReader(f, utf16bom)
 	}