feat: implement -R and -f flags for regional settings and codepage

[David Levy (dlevy-msft-sql)]

Summary

Implements the -R flag for locale-aware formatting and -f flag for codepage/encoding control, matching ODBC sqlcmd behavior.

Changes

-R (Regional Settings)

• Detect user locale from Windows LCID or Unix environment variables
• Apply regional formatting to:
  • DECIMAL/NUMERIC/MONEY: Locale-specific thousand separators
  • DATE/TIME/DATETIME: Locale-specific date and time formats

-f (Code Page)

• Specifies input/output encoding
• Format: codepage | i:codepage[,o:codepage] | o:codepage[,i:codepage]
• Use 65001 for UTF-8
• --list-codepages shows all supported encodings

Usage

# Regional formatting
sqlcmd -S server -R -Q "SELECT GETDATE(), 1234.56"

# UTF-8 output
sqlcmd -S server -f 65001 -Q "SELECT name FROM sys.databases"

[Copilot]

Pull request overview

This PR implements locale-aware formatting for query results when -R is specified and introduces configurable code page handling for input/output, plus associated CLI flags and documentation updates.

Changes:

• Add RegionalSettings with platform-specific locale detection (detectUserLocale) and integrate it into the default formatter so that -R controls locale-aware numeric and date/time formatting.
• Introduce code page parsing and encoding support (ParseCodePage, GetEncoding, SupportedCodePages) and wire it into file input (:R), output (:OUT, :ERROR), and new CLI flags -f/--code-page and --list-codepages.
• Extend tests to cover regional formatting helpers, formatter construction, code page parsing/encoding, CLI argument parsing/validation, and document new -R and -f behaviors in README.md.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pkg/sqlcmd/sqlcmd.go	Adds CodePage *CodePageSettings to Sqlcmd and updates IncludeFile to honor configured input code pages or BOM-based UTF-16 auto-detection when reading :R files.
pkg/sqlcmd/regional.go	Implements RegionalSettings (locale detection via detectUserLocale), number/money/date/time formatting, and locale-specific separators and date/time layouts.
pkg/sqlcmd/regional_windows.go	Windows-only locale detection using GetUserDefaultLCID and a mapping from LCID values to BCP 47 language tags.
pkg/sqlcmd/regional_linux.go	Linux-only locale detection from LC_ALL, LC_MESSAGES, and LANG, plus Unix locale string parsing to BCP 47 tags.
pkg/sqlcmd/regional_darwin.go	macOS-only locale detection using environment variables or defaults read -g AppleLocale, with Unix locale parsing similar to Linux.
pkg/sqlcmd/regional_test.go	Unit tests for RegionalSettings enable/disable behavior, NULL/empty handling, separators, date/time format selection, helper functions, and basic formatter construction with/without regional settings.
pkg/sqlcmd/format.go	Extends sqlCmdFormatterType with a regional *RegionalSettings field, adds NewSQLCmdDefaultFormatterWithRegional, and applies regional formatting to numeric and date/time columns in scanRow when -R is enabled.
pkg/sqlcmd/commands.go	Updates :OUT and :ERROR commands to write using either UTF-16 (for -u) or a configured output code page via GetEncoding, falling back to raw UTF-8 when appropriate.
pkg/sqlcmd/codepage.go	Adds CodePageSettings, ParseCodePage for -f syntax, GetEncoding for many Windows and related code pages, and SupportedCodePages metadata for listing.
pkg/sqlcmd/codepage_test.go	Tests ParseCodePage (including error cases and specific code pages) and GetEncoding for successful encodings and error handling for unsupported code pages.
cmd/sqlcmd/sqlcmd.go	Extends SQLCmdArguments with CodePage, ListCodePages, and UseRegionalSettings, validates -f, adds --code-page, --list-codepages, and -R flag wiring, lists supported code pages when requested, parses code page settings before running, and uses NewSQLCmdDefaultFormatterWithRegional to honor -R.
cmd/sqlcmd/sqlcmd_test.go	Adds CLI argument parsing tests for -f variations and --list-codepages, plus invalid -f cases that exercise Validate; reuses existing test harness for command-line normalization and error formatting.
README.md	Updates the description of -R to reflect new locale-aware formatting behavior and documents the new -f code page option and --list-codepages helper, including examples of supported code pages.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

[Copilot]

🟡 Changes recommended

Confirmed functional issues in new locale/codepage handling (Norwegian decimal separator and UTF-8 BOM handling when -f 65001 is specified) need to be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

pkg/sqlcmd/regional.go:233

• getDecimalSeparator includes "no" but not "nb"/"nn". However getThousandSeparator already treats "nb" and "nn" as Norwegian locales (NBSP separator). Since language.Tag.Base() commonly returns nb/nn for Norwegian, the current code will incorrectly default to "." as the decimal separator for nb-NO/nn-NO.
  pkg/sqlcmd/regional_test.go:65
• There’s coverage for several locales in TestGetDecimalSeparator, but none for Norwegian Bokmål/Nynorsk (nb-NO/nn-NO). Since getThousandSeparator already has special-casing for nb/nn, adding these test cases would prevent regressions where the decimal separator incorrectly defaults to ..

• Files reviewed: 17/17 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Output/error redirection with encoding transforms can leak file descriptors because the transformer wrapper may be closed without closing the underlying file handle.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

pkg/sqlcmd/commands.go:329

• When output is redirected with a transformer (UTF-16 or -f output codepage), the file handle o is not guaranteed to be closed later: transform.NewWriter only receives an io.Writer, so closing the transformer (via SetOutput / SetError) flushes but cannot close the underlying file. In interactive sessions (or multiple OUT/ERROR calls), this can leak file descriptors. Consider wrapping the transformer and the file in a single io.WriteCloser that closes (flushes) the transformer and then closes o.

This issue also appears on line 370 of the same file.

pkg/sqlcmd/commands.go:385

• Same underlying-file-close issue exists for the ERROR redirection when wrapping with transform.NewWriter: SetError will close the transformer but cannot close the underlying *os.File because the transformer was created from an io.Writer. This can leak descriptors if ERROR output is redirected multiple times during a session.

		// Apply output codepage if configured
		if s.CodePage != nil && s.CodePage.OutputCodePage != 0 {
			enc, err := GetEncoding(s.CodePage.OutputCodePage)
			if err != nil {
				if cerr := o.Close(); cerr != nil {
					return fmt.Errorf("%w (and closing error file %s failed: %v)", err, filePath, cerr)
				}
				return err
			}
			if enc == nil {
				// No transformation required (e.g., UTF-8), write directly
				s.SetError(o)
			} else {
				encoder := transform.NewWriter(o, enc.NewEncoder())
				s.SetError(encoder)
			}

• Files reviewed: 17/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Output/error codepage transforms currently risk leaking file handles and not closing underlying files when using transform.NewWriter, and new localized strings need regeneration/commit of the translation catalog.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

cmd/sqlcmd/sqlcmd.go:539

• New user-facing strings were added via localizer.Sprintf(...) for -R, -f/--code-page, and --list-codepages, but the translation catalog under internal/translations/ does not contain these msgids (grep shows no matches). Please regenerate and commit the updated gotext catalog (go generate ./... or the build script) so these strings are available for localization.

pkg/sqlcmd/commands.go:385

• errorCommand has the same transform.NewWriter(o, ...) pattern: SetError(encoder) will later close only the transformer, not the underlying *os.File, since transform.Writer.Close() doesn't close its wrapped writer. This can leak the error file handle and can drop final bytes for stateful encodings. Use a wrapper io.WriteCloser that closes both the transformer and o.

			if enc == nil {
				// No transformation required (e.g., UTF-8), write directly
				s.SetError(o)
			} else {
				encoder := transform.NewWriter(o, enc.NewEncoder())
				s.SetError(encoder)
			}

• Files reviewed: 17/17 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are user-facing documentation/localization inconsistencies that should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

• Files reviewed: 29/30 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

pkg/sqlcmd/format.go assumes f.regional is always non-nil and can panic on f.regional.IsEnabled() in legitimate in-package constructions unless guarded or always initialized.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

Suppressed comments (2)

pkg/sqlcmd/format.go:553

• f.regional.IsEnabled() can panic if f.regional is nil. This is a pointer field, so defensive nil-checking is needed here as well before applying regional time formatting.

			case time.Time:
				// Apply regional formatting when -R is enabled
				if f.regional.IsEnabled() {
					switch typeName {

pkg/sqlcmd/format.go:600

• f.regional.IsEnabled() can panic if f.regional is nil. Add a nil check before applying regional formatting in the default case too.

			default:
				val := fmt.Sprintf("%v", x)
				// Apply regional formatting for numeric types
				if f.regional.IsEnabled() {
					switch typeName {

• Files reviewed: 29/30 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Regional datetime formatting currently uses a derived scale for DATETIME in the -R path (risking loss of fractional seconds), and macOS locale detection should avoid PATH-resolved command execution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

• Files reviewed: 29/30 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Regional formatting currently isn’t applied when numeric/currency values are scanned as []byte, which can prevent -R from taking effect for DECIMAL/NUMERIC/MONEY in common driver representations.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

pkg/sqlcmd/format.go:535

• When values are scanned as []byte (common for DECIMAL/NUMERIC/MONEY when scanning into interface{}), the regional formatting path is skipped and the raw string is emitted. Apply the same regional formatting logic in the []byte branch so -R consistently formats numeric/currency values regardless of driver representation.
  README.md:154
• The implementation of -R also changes the decimal separator (e.g., '.' -> ',') for many locales (see FormatNumber/FormatMoney), but the README only mentions thousand separators. Updating this sentence avoids misleading users about the output format changes.

• Files reviewed: 29/30 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are cohesive, well-covered by targeted tests (including BOM and formatting edge cases), and correctly wired into the legacy CLI with updated localization artifacts.

Review details

Files not reviewed (1)

• internal/translations/catalog.go: Generated file

• Files reviewed: 29/30 changed files
• Comments generated: 0 new
• Review effort level: Lite