Skip to content
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

[Fix 954]: #955

Closed
wants to merge 16 commits into from
Closed

[Fix 954]: #955

wants to merge 16 commits into from

Conversation

fenrir-san
Copy link

@fenrir-san fenrir-san commented Dec 4, 2023

Describe your changes

  1. Modified split_args_and_sql to account for uppercase SQL commands.
  2. Modified split_args_and_sql to check for variable assignments and add it to sql_line instead of arg_line.
  3. Modified without_sql_comment to improve handling for cases in which queries include a -- within quotes with spaces. Example : SELECT TRIM (' --padded ').
  4. Added tests for the modifications.

Issue number

Closes #954

Checklist before requesting a review


📚 Documentation preview 📚: https://jupysql--955.org.readthedocs.build/en/955/

@fenrir-san fenrir-san marked this pull request as ready for review December 26, 2023 11:32
@@ -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)

Choose a reason for hiding this comment

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

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')

Choose a reason for hiding this comment

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

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:

Choose a reason for hiding this comment

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

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)

Choose a reason for hiding this comment

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

why are we running run_line_magic two times now?

@edublancas edublancas closed this Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ValueError: No closing quotation / Likely argument parsing issue.
2 participants