Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-133390: Support SQL keyword completion for sqlite3 CLI#133393

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
erlend-aasland merged 54 commits intopython:mainfromtanloong:sqlite3-cli-completion
Jun 6, 2025

Conversation

tanloong
Copy link
Contributor

@tanloongtanloong commentedMay 4, 2025
edited by bedevere-appbot
Loading

This adds tab-completion for the147 SQLite Keywords. A whitespace is appended to completion candidates for users' convenience and to mimic the behavior ofsqlite3 tool.

@python-cla-bot
Copy link

python-cla-botbot commentedMay 4, 2025
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

Copy link
Member

@StanFromIrelandStanFromIreland left a comment

Choose a reason for hiding this comment

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

This needs a whatsnew entry, (maybe a note in the docs?), a NEWS entry and tests.

This is quite limited completion so I guess it could work.

@picnixz
Copy link
Member

I think we need a separate nodule for this because if we want to have a smarter completion later the code is better to be isolated, so something like sqlite3._completer.

If possible, can we have anautogenerated list of keywords? and those keywords could also be stored in a (private) module-level list.

@picnixz
Copy link
Member

Please add tests

@tanloong
Copy link
ContributorAuthor

Yes, tests is coming. I will try to find a way to autogenerate list of keywords.

picnixz reacted with thumbs up emoji

@StanFromIreland
Copy link
Member

You can mark the pr as a draft.

@tanloongtanloong marked this pull request as draftMay 4, 2025 21:23
@tanloongtanloongforce-pushed thesqlite3-cli-completion branch 8 times, most recently from782a599 to5005d85CompareMay 5, 2025 04:47
@tanloong
Copy link
ContributorAuthor

tanloong commentedMay 5, 2025
edited
Loading

Added tests and moved keyword list to module level.

Keywords seems not easy to auto-generate, theSQLite document says they can be accessed bysqlite3_keyword_count() andsqlite3_keyword_name() using C, but I am bad at C and didn't figure out how to call these two at Python level. I think these two functions are not exposed to Python?

@tanloongtanloong marked this pull request as ready for reviewMay 5, 2025 05:08
Copy link
Member

@StanFromIrelandStanFromIreland left a comment

Choose a reason for hiding this comment

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

With#133447 in mind the keywords list would be quite handy (I think hardcoding is fine, it doesn't change often anyway). Maybe it could be moved elsewhere, what do you think Benedikt?

@tanloongtanloongforce-pushed thesqlite3-cli-completion branch from15019ea to22652aeCompareMay 5, 2025 18:20
@tanloong
Copy link
ContributorAuthor

tanloong commentedMay 5, 2025
edited
Loading

Hi@picnixz, as Stan mentioned, beta freeze is around the corner, but I really hope this can be shipped in 3.14, do you think there is a chance to merge it before beta freeze?

for (int i = 0; i < count; i++) {
const char *keyword;
int size;
int result = sqlite3_keyword_name(i, &keyword, &size);
Copy link
Contributor

Choose a reason for hiding this comment

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

For the record:
Wecould justassert that the result isSQLITE_OK. This is just an out-of-bounds check, and we know that we are within bounds here. OTOH, we don't know how the SQLite internals may change, so let's keep it like it is.

@erlend-aasland
Copy link
Contributor

FTR, CI fails bco.#134077. Let's await a revert or a fix for that.

Otherwise, this looks great. Thank you so much for improving the sqlite3 module,@tanloong. Highly appreciated!

tanloong reacted with heart emoji

@erlend-aaslanderlend-aasland changed the titlegh-133390: Support basic completion for sqlite3 command-line interfacegh-133390: Support SQL keyword completion for sqlite3 CLIJun 6, 2025
@erlend-aasland
Copy link
Contributor

FTR, I've tested this with new SQLite variants, as well as SQLite 3.23.0 and 3.24.0. Everything works as expected for all versions. I should really get to setting up a build-bot with a SQLite library matrix.

stonebig reacted with hooray emojiencukou reacted with heart emoji

@erlend-aasland
Copy link
Contributor

I only get the"Display all 147 possibilities? (y or n)" prompt with editline, and never on readline. Probably a bug in readline? Or are we missing a configuration option in order to get this to work?

@tanloong
Copy link
ContributorAuthor

tanloong commentedJun 6, 2025
edited
Loading

Readline has a completion-query-items (default value 100) option to determine whether or when this prompt should be shown.https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html#index-completion_002dquery_002ditems

With a default readline config the prompt should have be shown, I suspect there is a config that sets the option to 0 on the testing machine?

@erlend-aasland
Copy link
Contributor

erlend-aasland commentedJun 6, 2025
edited
Loading

With a default readline config the prompt should have be shown, I suspect there is a config that sets the option to 0 on the testing machine?

Nope. I was a little bit vague in my previous comment: I see the text"Display all 147 possibilities? (y or n)", but it is not shown as a prompt after the first tab. It is only displayed together with the resulting completion list, after the second tab.

IOW: For editline, I get the prompt on the first tab; for readline, I get no prompt, but I see the prompt text after the second tab.

FTR, I already experimented with thecompletion-query-items option, and it has no impact.

@tanloong
Copy link
ContributorAuthor

Tryset show-all-if-unmodified on in readline and see if it make the prompt shown on just the first tab?https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File-Syntax.html#index-show_002dall_002dif_002dunmodified

@erlend-aaslanderlend-aasland merged commit62b3d2d intopython:mainJun 6, 2025
39 checks passed
@tanloong
Copy link
ContributorAuthor

tanloong commentedJun 6, 2025
edited
Loading

Thank you very much for the careful review and thorough test! There are so many corners and difficulties that I would have never considered let alone solved, thanks for everyone's help!

encukou added a commit to encukou/cpython that referenced this pull requestJun 7, 2025
encukou pushed a commit to encukou/cpython that referenced this pull requestJun 7, 2025
encukou added a commit that referenced this pull requestJun 7, 2025
encukou pushed a commit to encukou/cpython that referenced this pull requestJun 7, 2025
encukou added a commit that referenced this pull requestJun 12, 2025
lkollar pushed a commit to lkollar/cpython that referenced this pull requestJun 19, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull requestJul 12, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull requestJul 12, 2025
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull requestJul 12, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@picnixzpicnixzpicnixz approved these changes

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes

@StanFromIrelandStanFromIrelandStanFromIreland left review comments

@encukouencukouencukou left review comments

Assignees

@erlend-aaslanderlend-aasland

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

8 participants
@tanloong@picnixz@StanFromIreland@bedevere-bot@encukou@erlend-aasland@serhiy-storchaka@ZeroIntensity

[8]ページ先頭

©2009-2025 Movatter.jp