Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrectly wrapped strings by localization routines #414

Merged
merged 4 commits into from
Jul 8, 2023

Conversation

apoorvdeshmukh
Copy link
Contributor

@apoorvdeshmukh apoorvdeshmukh commented Jul 6, 2023

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

Copy link
Collaborator

@shueybubbles shueybubbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@shueybubbles
Copy link
Collaborator

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)

@shueybubbles
Copy link
Collaborator

  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)

@apoorvdeshmukh
Copy link
Contributor Author

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
Copy link
Contributor Author

  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.

Copy link
Collaborator

@shueybubbles shueybubbles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@apoorvdeshmukh apoorvdeshmukh merged commit 64c3507 into main Jul 8, 2023
@apoorvdeshmukh apoorvdeshmukh deleted the apoorvdeshmukh/loc-issue branch July 8, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sqlcmd prints incorrectly strings
2 participants