Don't offer completions within strings

[rolandwalker]

Description

• Don't offer completions when the cursor is within a string.
• Move the similar implementation for numbers to a better location, next to the string check.

It would be great if suggest_based_on_last_token() was refactored to use named parameters.

Example of the behavior solved by this PR:

Checklist

• I added this contribution to the changelog.md file.
• I added my name to the AUTHORS file (or it's already there).
• To lint and format the code, I ran

  uv run ruff check && uv run ruff format && uv run mypy --install-types .

[scottnemes]

Verified completions are no longer given when proceeded by a single or double quote.

[github-actions]

Findings

1. Correctness: string detection is incomplete (suggestions can still appear inside strings).
   The new guard checks only word_before_cursor[0], which fails when the cursor is inside a quoted string but the last word no longer includes the opening quote (e.g. spaces, commas, or cursor after whitespace). Example: select "foo bar or select 'a,b at end — last_word returns bar or b, so suggestions still show. This is a regression against the PR intent.
   File: mycli/packages/completion_engine.py:188
   Suggested fix: Use a quote-aware check on the full buffer, e.g. _is_inside_quotes(text_before_cursor, len(text_before_cursor)), or equivalent state tracking that handles escapes.

2. Missing tests for string-with-space / punctuation cases.
   The new test only covers a string starting with a quote and no spaces. Please add at least one case like select "foo bar or select 'foo,bar (cursor at end), and one with trailing space inside quotes (select "foo ) to assert no completions.
   File: test/test_smart_completion_public_schema_only.py

Notes

• No security concerns found.

Questions/Assumptions

• If you want to keep double quotes as “string” in MySQL default mode, the above fix is still correct; if ANSI_QUOTES is in play, you may want different behavior for double quotes.

Residual risk

• Without the broader quote-state check, users can still get completions inside many real-world string literals.

[rolandwalker]

Codex is right about _is_inside_quotes() and the feature being incomplete. That way might not be too performant. It's a shame, because somewhere there is the state of the Pygments lexer, which we presumably can't access.

[github-actions]

Findings

1. Correctness: cursor position ignored in string detection.
   In mycli/packages/completion_engine.py, _is_inside_quotes(full_text, -1) checks the entire document, not the cursor location. If the cursor is not at the end, a quote later in the buffer can incorrectly suppress completions. Use text_before_cursor (or pass the actual cursor position) instead.
   File: mycli/packages/completion_engine.py

2. Regression: numeric guard no longer applies when smart completion is off.
   The numeric early return was removed from SQLCompleter.get_completions, and the new check only runs in suggest_based_on_last_token (smart completion path). With smart_completion=False, numeric prefixes will now suggest completions. If the old behavior should remain, add the guard back in the non-smart path.
   File: mycli/sqlcompleter.py

Missing tests / edge cases

1. Add a test where the cursor is not at the end and there is a quote after the cursor, to ensure completions are still offered (covers the cursor-position bug).
   File: test/test_smart_completion_public_schema_only.py

2. If numeric suppression is still desired for non-smart completion, add a test for smart_completion=False with numeric input.
   File: test/test_smart_completion_public_schema_only.py

I didn’t run tests. If you want, I can add the failing test for the cursor-position issue to demonstrate the bug quickly.

[github-actions]

Findings

1. Regression when smart_completion is off
   The number/string suppression moved into suggest_based_on_last_token(), which is only used when smart_completion is enabled. With smart_completion=False, completions will now show inside numbers/strings, whereas previously they were suppressed for all modes.
   Impact: users who disable smart completion (performance or preference) will get regressions.
   Fix: add a lightweight guard back in get_completions() before the smart_completion branch or apply the new word_before_cursor checks there too.
   File: mycli/sqlcompleter.py#L1069.

2. False negatives after a closed quoted token
   The new early return checks word_before_cursor[0] in ('"', "'") without verifying the quote is still open. If the cursor is immediately after a closed quoted token (e.g. select "col" with no trailing space), this will suppress completions even though the cursor is not inside a string.
   Impact: completions are unexpectedly disabled right after a closed quoted identifier/literal.
   Fix: either remove the direct quote check and rely on _is_inside_quotes(...), or gate it on _is_inside_quotes returning true.
   File: mycli/packages/completion_engine.py#L180-L199.

Missing tests / edge cases

1. Add a test for single-quoted strings (e.g. select 'json) to ensure the new guard works for single quotes.
   File: test/test_smart_completion_public_schema_only.py.

2. Add a test for a closed-quote case to ensure completions are still offered after a complete quoted token (e.g. select "col" with cursor at end).
   File: test/test_smart_completion_public_schema_only.py.

Notes

• I didn’t run tests locally.

If you want, I can draft the small guard in get_completions() and add the two tests.

[github-actions]

Findings

1. Regression: numeric suppression now only runs in the smart-completion path. With smart_completion=False, get_completions will return matches for numbers again because the guard was removed from mycli/sqlcompleter.py and moved into suggest_based_on_last_token in mycli/packages/completion_engine.py. This is a behavior change from before. Consider keeping the early number guard in get_completions or adding a shared guard that applies to both modes. mycli/sqlcompleter.py, mycli/packages/completion_engine.py.
2. Requirement gap: the “don’t offer completions within strings” behavior is implemented only via suggest_type/suggest_based_on_last_token, so it is bypassed when smart_completion=False. If the intent is global, add a cheap string check in get_completions that applies regardless of smart completion. mycli/sqlcompleter.py, mycli/packages/completion_engine.py.

Missing tests

1. Add coverage for smart_completion=False so both strings and numeric prefixes produce no completions. This prevents the regressions above from slipping in again. test/test_smart_completion_public_schema_only.py.

[github-actions]

Findings

1. Likely regression: completions suppressed immediately after a closing quote.
   suggest_based_on_last_token() now calls _is_inside_quotes(text_before_cursor, -1). In _is_inside_quotes(), text[:pos] with pos=-1 excludes the last character. If the cursor is directly after a closing quote (no trailing space), the closing quote is excluded, leaving an unmatched opening quote and returning True, which suppresses completions even though the cursor is not inside a string.
   File: mycli/packages/completion_engine.py#L182 (calls at #L200 and later).
   Fix: pass len(text_before_cursor) (or the actual cursor position) instead of -1.
   Missing test: add a case like text = 'select "foo"' (and single-quote variant) where completions should still be offered. Current tests only cover the “inside string” case.

Questions / Assumptions

• I’m assuming completions should be available immediately after a quoted identifier/string is closed (e.g., select "foo"). If that’s intended to be suppressed, the new behavior is fine, but it would be a change from typical SQL completion UX.

Suggested next steps

1. Decide desired behavior for cursor directly after a closing quote; if completions should appear, update _is_inside_quotes call to use len(text_before_cursor) and add the test.

[github-actions]

Findings (ordered by severity)

1. Potential regression: completions suppressed immediately after a closed quoted literal or identifier (no trailing space).
   In suggest_based_on_last_token, the early guard returns [] whenever word_before_cursor starts with ' or ", regardless of whether the quote is closed. So SELECT "foo"| or SELECT 'foo'| will suppress completions even though the cursor is outside the string. This is new behavior.
   If the intent is only “inside string”, consider guarding on _is_inside_quotes(...) instead of word_before_cursor[0] (or combine them).
   File: mycli/packages/completion_engine.py (new early return near suggest_based_on_last_token)

Missing tests / edge cases

1. Add a test that completions resume after a closing quote with no trailing space, if that’s the intended behavior.
   Suggested cases:
   • select "json"|
   • select 'json'|
     File: test/test_smart_completion_public_schema_only.py

No security concerns found.

Open questions / assumptions

1. Do you want completions immediately after a closing quote without requiring a space? Current behavior is “no”. If that’s intended, a test should lock it in.