Skip to content

gh-133439: Fix the error message in the sqlite3 CLI - #133807

Merged
encukou merged 10 commits into
python:mainfrom
StanFromIreland:sqlite3-fixup
Jun 19, 2025
Merged

gh-133439: Fix the error message in the sqlite3 CLI#133807
encukou merged 10 commits into
python:mainfrom
StanFromIreland:sqlite3-fixup

Conversation

@StanFromIreland

@StanFromIrelandStanFromIreland commented May 10, 2025

Copy link
Copy Markdown
Member
  • Use print for file consistency
  • Actually test for the full improved error message
  • Correct error message, messages do not have the '.'

Request: @erlend-aasland

@picnixzpicnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this PR is needed anymore, except maybe for the assertIn

@StanFromIreland

Copy link
Copy Markdown
MemberAuthor

Should the error message not be consistent with all other cpython error messages?

I think it is still better to use print for clarity (we don't have to look it up) and consistency, all other writing in the file is done so.

@picnixz

picnixz commented May 10, 2025

Copy link
Copy Markdown
Member

I think it is still better to use print for clarity (we don't have to look it up) and consistency, all other writing in the file is done so.

I guess so. I actually expected InteractiveConsole to have a write_stderr and a write_stdout but there's none, so maybe only using print makes sense. WDYT @erlend-aasland?

@picnixzpicnixz changed the title gh-133439: Fixup sqlite3 changes in 133440gh-133439: simplify how errors are reported by sqlite3 CLIMay 10, 2025
Comment threadLib/sqlite3/__main__.py Outdated
Comment threadLib/test/test_sqlite3/test_cli.py Outdated
Comment threadLib/sqlite3/__main__.py Outdated
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixzpicnixz added the needs backport to 3.14 bugs and security fixes label May 10, 2025
@StanFromIreland

Copy link
Copy Markdown
MemberAuthor

While working on something else I realized the current error message is error-ed even more:

sqlite> .HELP Error: unknowncommand or invalid arguments: "HELP".

Friendly ping @erlend-aasland

@erlend-aaslanderlend-aasland self-assigned this Jun 13, 2025
@erlend-aasland

Copy link
Copy Markdown
Contributor

Stan, did you want to include further amendments in this PR?

@StanFromIreland

Copy link
Copy Markdown
MemberAuthor

I don't mind, I can include it here. All that is to be done for the . help is to just use .rstrip instead of .strip.

Comment threadLib/sqlite3/__main__.py Outdated
Comment threadLib/sqlite3/__main__.py Outdated
t = theme.traceback
self.write(f'{t.type}Error{t.reset}:{t.message} unknown'
f'command or invalid arguments: "{unknown}".\n{t.reset}')
print(f'{t.type}Error{t.reset}:{t.message} unknown command '

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that the banner and tracebacks are written using self.write(). Why not use it here for consistence?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Errors (35) use print? And so do the other match statements branches.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Other branches output to stdout.

Line 35 perhaps should use self.write() too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can leave execute() for now. It is not worth to rewrite it just for this.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Suggested change
print(f'{t.type}Error{t.reset}:{t.message} unknown command '
self.write(f'{t.type}Error{t.reset}:{t.message} unknown command '

It is up to you/Erlend. The box to allow you to commit was checked.

@serhiy-storchakaserhiy-storchaka changed the title gh-133439: simplify how errors are reported by sqlite3 CLIgh-133439: Fix the error message in the sqlite3 CLIJun 13, 2025
Comment threadLib/sqlite3/__main__.py Outdated
Comment threadLib/test/test_sqlite3/test_cli.py Outdated
Comment threadLib/test/test_sqlite3/test_cli.py Outdated
@encukou
encukou merged commit ecd83e0 into python:mainJun 19, 2025
@miss-islington-app

Copy link
Copy Markdown

Thanks @StanFromIreland for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @StanFromIreland and @encukou, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker ecd83e02b128bf0879d9bb1d3940e40bcb14bdc6 3.14

@StanFromIreland

StanFromIreland commented Jun 19, 2025

Copy link
Copy Markdown
MemberAuthor

The original was "backported" to 3.13 but it seems it was modified:

self.write("Error: unknown command or invalid arguments:"
f' "{unknown}".\n')

@encukou Should we backport this (to 3.13) too so they are the same?

@bedevere-app

Copy link
Copy Markdown

GH-135708 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Jun 19, 2025
StanFromIreland added a commit to StanFromIreland/cpython that referenced this pull request Jun 19, 2025
…honGH-133807)
(cherry picked from commit ecd83e0)
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@StanFromIreland
StanFromIreland deleted the sqlite3-fixup branch June 19, 2025 12:33
@encukou

Copy link
Copy Markdown
Member

Now that I look at the diff, I don't see a bug to fix in 3.14.

@StanFromIreland

Copy link
Copy Markdown
MemberAuthor

There is no bug because the author modified the backport, the message is different though.

lkollar pushed a commit to lkollar/cpython that referenced this pull request Jun 19, 2025
…33807)
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
…33807)
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
…33807)
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
…33807)
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@StanFromIreland@picnixz@erlend-aasland@encukou@serhiy-storchaka