[Fix 954]: #955
Closed
[Fix 954]: #955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nooodle-soup
force-pushed
the
issue#954
branch
from
d5c8de3
to
6e50ecb
Compare
nooodle-soup
force-pushed
the
issue#954
branch
from
da8d32a
to
ab819ad
Compare
edublancas
requested changes
Dec 26, 2023
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| ## 0.10.8dev | |||
|
|
|||
| * [Fix] Fixed bug where the `%sql` line magic would not parse properly due to expectations from shlex(#954) | |||
There was a problem hiding this comment.
the changelog is for end users, so there's no need to disclose internal stuff like shlex. it's better to phrase the entry in a way that end users can understand
| @@ -228,11 +228,14 @@ def without_sql_comment(parser, line): | |||
| """ | |||
|
|
|||
| args = _option_strings_from_parser(parser) | |||
| pattern = re.compile(r'([\'"])(.*?)\1') | |||
There was a problem hiding this comment.
please add some comment to explain this code
| # Split line into args and sql, beginning at sql keyword | ||
| if split_idx != -1: | ||
| arg_line, sql_line = line[:split_idx], line[split_idx:] | ||
| if "<<" in line: |
There was a problem hiding this comment.
add some comments to explain this code
| @@ -12,7 +12,8 @@ def __init__(self, connectstr): | |||
| self.connectstr = connectstr | |||
|
|
|||
| def query(self, txt): | |||
| return ip.run_line_magic("sql", "%s %s" % (self.connectstr, txt)) | |||
| ip.run_line_magic("sql", self.connectstr) | |||
There was a problem hiding this comment.
why are we running run_line_magic two times now?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
split_args_and_sqlto account for uppercase SQL commands.split_args_and_sqlto check for variable assignments and add it tosql_lineinstead ofarg_line.without_sql_commentto improve handling for cases in which queries include a--within quotes with spaces. Example :SELECT TRIM (' --padded ').Issue number
Closes #954
Checklist before requesting a review
pkgmt format馃摎 Documentation preview 馃摎: https://jupysql--955.org.readthedocs.build/en/955/