search

microsoft / go-sqlcmd

The new sqlcmd, CLI for SQL Server and Azure SQL (winget install sqlcmd / sqlcmd create mssql / sqlcmd open ads)
https://learn.microsoft.com/sql/tools/sqlcmd/go-sqlcmd-utility
MIT License
323 stars 57 forks source link

Fix incorrectly wrapped strings by localization routines #414

Closed apoorvdeshmukh closed 1 year ago

apoorvdeshmukh commented 1 year ago

Fixes #413 

git\go-sqlcmd>.\sqlcmd.exe start
Starting "mcr.microsoft.com/mssql/server:latest" for context "mssql"

git\go-sqlcmd>.\sqlcmd.exe stop
Stopping "mcr.microsoft.com/mssql/server:latest" for context "mssql"

git\go-sqlcmd>.\sqlcmd.exe remove --force --yes
Removing context mssql
Container "4909e2af7b209c3dd395911747882d90a90ee43ed7ed0ea335fdeb96481abd29" no longer exists, continuing to remove context...
Operation completed successfully
shueybubbles commented 1 year ago

func (o Output) Errorf(format string, a ...any) {

can you update the linter to look for calls to Output functions that could lead to this bug in the future? Today the linter doesn't block the build but at least someone running build\build locally could see the violation.

Refers to: internal/output/output.go:36 in 567c8e9. [](commit_id = 567c8e954c951be3625d6a325eb8a8f41dbd6928, deletion_comment = False)

shueybubbles commented 1 year ago 
  output.FatalfWithHints(

this call is fine because there are no user-provided arguments, but please check other callers of these Fatalf methods.

Refers to: cmd/modern/root/install/mssql-base.go:627 in 567c8e9. [](commit_id = 567c8e954c951be3625d6a325eb8a8f41dbd6928, deletion_comment = False)

apoorvdeshmukh commented 1 year ago

func (o Output) Errorf(format string, a ...any) {

can you update the linter to look for calls to Output functions that could lead to this bug in the future? Today the linter doesn't block the build but at least someone running build\build locally could see the violation.

Refers to: internal/output/output.go:36 in 567c8e9. [](commit_id = 567c8e9, deletion_comment = False)

I'll take this as a follow up item. Since we are using localization routines in all of these functions (except few places) , I have added the non f variants of the existing print functions and have replaced it everywhere.

apoorvdeshmukh commented 1 year ago 
  output.FatalfWithHints(

this call is fine because there are no user-provided arguments, but please check other callers of these Fatalf methods.

Refers to: cmd/modern/root/install/mssql-base.go:627 in 567c8e9. [](commit_id = 567c8e9, deletion_comment = False)

There were similar issues with other f functions too. Fixed them.