Skip to content

gh-133439: simplify how errors are reported by sqlite3 CLI #133807

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

StanFromIreland
Copy link
Contributor

@StanFromIreland StanFromIreland commented May 10, 2025

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

Request: @erlend-aasland

Copy link
Member

@picnixz picnixz left a comment

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

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
Copy link
Member

picnixz commented May 10, 2025

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?

@picnixz picnixz changed the title gh-133439: Fixup sqlite3 changes in 133440 gh-133439: simplify how errors are reported by sqlite3 CLI May 10, 2025
Co-authored-by: Bénédikt Tran <[email protected]>
@StanFromIreland StanFromIreland requested a review from picnixz May 10, 2025 08:34
@picnixz picnixz added the needs backport to 3.14 bugs and security fixes label May 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants