-
Notifications
You must be signed in to change notification settings - Fork 680
Import MySQL completion candidates from pygments
#1447
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
Conversation
c077ff7 to
7fee5fd
Compare
I always prefer lowercase, but as long as it's consistent; i.e. all options, regardless of source, are uppercase or lowercase ideally. |
| for x in favorite_keywords | ||
| + list(MYSQL_DATATYPES) | ||
| + list(MYSQL_KEYWORDS) | ||
| + ['ALTER TABLE', 'CHANGE MASTER TO', 'CHARACTER SET', 'FOREIGN KEY'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind I realized it makes sense in the context of you technically don't know if they want just the keyword master, or the command change master to in this example. Was overthinking it since there is no valid "change master" command, but that is another layer of context.
Since the pygments library is already required, why not use its list of reserved words for completions? This fixes #767 and should keep us up-to-date. It's really great to have the JSON_* functions. Obsoletes #925. Compared to the previous code, this PR * removes LEN and TOP, which are not MySQL reserved words * preserves extra completion candidates containing space, such as "ORDER BY" Downsides and bugs: * pygments.lexers._mysql_builtins does contain a leading underscore, so we should be aware that the library reserves the right to break this usage. The library version has been more tightly defined to remediate issues here. * Similarly, certain tests might become more brittle with regard to library updates. * Certain reserved words are duplicated with special commands, so if the first word of a command-line is any of the following, duplicated completions will show, as both upper- and lower-case: exit, help, source, status, system, use. This is fixable, but should we prefer the upper- or lower-case flavor? * There are _many_ more completion candidates now, which may inspire us to do further work soon on prioritizing which completions are seen at the top.
7fee5fd to
7103137
Compare
I'll make lowercase duplicates preferred in a followup PR. |


Description
Since the
pygmentslibrary is already required, why not use its list of reserved words for completions?This fixes #767 and should keep us up-to-date. It's really great to have the
JSON_*functions.Obsoletes #925, and addresses the two points in #925 (comment) .
Compared to the previous code, this PR
LENandTOP, which are not MySQL reserved wordsORDER BYDownsides and bugs:
pygments.lexers._mysql_builtinsdoes contain a leading underscore, so we should be aware that the library reserves the right to break this usage. The library version has been more tightly defined to remediate issues here.exit,help,source,status,system,use. This is fixable, but should we prefer the upper- or lower-case flavor?Checklist
changelog.md.AUTHORSfile (or it's already there).uv run ruff check && uv run ruff format && uv run mypy --install-types .to lint and format the code.