Skip to content

Commit 0954cda

Browse files
authored
Merge pull request #117 from Maxteabag/feature/execute-selection-by-default
Execute selection by default, fix comment execution, add gcS
2 parents c1f3fcc + 7af2e05 commit 0954cda

File tree

10 files changed

+211
-29
lines changed

10 files changed

+211
-29
lines changed

sqlit/core/keymap.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ def _build_leader_commands(self) -> list[LeaderCommandDef]:
282282
LeaderCommandDef("k", "up", "Comment line up", "Comment", menu="gc"),
283283
LeaderCommandDef("G", "to_end", "Comment to end", "Comment", menu="gc"),
284284
LeaderCommandDef("s", "selection", "Toggle selection", "Comment", menu="gc"),
285+
LeaderCommandDef("S", "statement", "Toggle statement", "Comment", menu="gc"),
285286
# ry results yank menu
286287
LeaderCommandDef("c", "cell", "Copy cell", "Copy", menu="ry"),
287288
LeaderCommandDef("y", "row", "Copy row", "Copy", menu="ry"),

sqlit/domains/query/app/alerts.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ class AlertSeverity(IntEnum):
4646
_DOUBLE_QUOTE_RE = re.compile(r'"[^"]*"')
4747
_BACKTICK_RE = re.compile(r"`[^`]*`")
4848
_BRACKET_RE = re.compile(r"\[[^\]]*]")
49-
_LINE_COMMENT_RE = re.compile(r"--[^\n]*")
50-
_BLOCK_COMMENT_RE = re.compile(r"/\*.*?\*/", re.DOTALL)
5149

5250

5351
def parse_alert_mode(value: str | int | None) -> AlertMode | None:
@@ -113,8 +111,9 @@ def _classify_statement(statement: str) -> AlertSeverity:
113111

114112
def _strip_comments_and_literals(sql: str) -> str:
115113
"""Remove comments and quoted literals/identifiers for keyword scanning."""
116-
cleaned = _LINE_COMMENT_RE.sub("", sql)
117-
cleaned = _BLOCK_COMMENT_RE.sub("", cleaned)
114+
from sqlit.domains.query.editing.comments import strip_all_comments
115+
116+
cleaned = strip_all_comments(sql)
118117
cleaned = _SINGLE_QUOTE_RE.sub("''", cleaned)
119118
cleaned = _DOUBLE_QUOTE_RE.sub('""', cleaned)
120119
cleaned = _BACKTICK_RE.sub("``", cleaned)

sqlit/domains/query/app/multi_statement.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,28 @@ def find_statement_at_cursor(sql: str, row: int, col: int) -> tuple[str, int, in
257257
return ranges[0] if ranges else None
258258

259259

260+
def get_executable_sql(sql: str) -> str:
261+
"""Get the SQL that will actually be executed.
262+
263+
Filters out comment-only statements and returns the remaining SQL
264+
joined with semicolons. This should be used for display in confirm
265+
dialogs to show exactly what will execute.
266+
267+
Args:
268+
sql: Raw SQL that may contain comment-only statements.
269+
270+
Returns:
271+
SQL string with comment-only statements removed.
272+
"""
273+
from sqlit.domains.query.editing.comments import is_comment_only_statement
274+
275+
statements = split_statements(sql)
276+
executable = [s for s in statements if not is_comment_only_statement(s)]
277+
if not executable:
278+
return ""
279+
return "; ".join(executable)
280+
281+
260282
def split_statements(sql: str) -> list[str]:
261283
"""Split SQL into individual statements.
262284
@@ -400,8 +422,14 @@ def execute(self, sql: str, max_rows: int | None = None) -> MultiStatementResult
400422
Returns:
401423
MultiStatementResult containing results from all executed statements.
402424
"""
425+
from sqlit.domains.query.editing.comments import is_comment_only_statement
426+
403427
statements = split_statements(sql)
404428

429+
# Filter out comment-only statements - they cause "empty query" errors
430+
# in some database drivers that strip comments before execution
431+
statements = [s for s in statements if not is_comment_only_statement(s)]
432+
405433
if not statements:
406434
return MultiStatementResult(results=[], completed=True, error_index=None)
407435

sqlit/domains/query/app/query_service.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,20 +72,25 @@ def classify(self, query: str) -> QueryKind:
7272
we check the last statement to determine if results should be returned.
7373
Uses the same splitting logic as multi_statement.split_statements.
7474
"""
75+
from sqlit.domains.query.editing.comments import (
76+
is_comment_line,
77+
is_comment_only_statement,
78+
)
79+
7580
from .multi_statement import split_statements
7681

7782
statements = split_statements(query)
7883
if not statements:
7984
return QueryKind.NON_QUERY
8085

81-
# Filter out comment-only statements (lines starting with --)
82-
# and find the last actual SQL statement
86+
# Filter out comment-only statements and find the last actual SQL statement
8387
for stmt in reversed(statements):
84-
# Skip comment-only lines
88+
if is_comment_only_statement(stmt):
89+
continue
90+
# Found a statement with actual SQL - get first non-comment line
8591
lines = [line.strip() for line in stmt.split("\n") if line.strip()]
86-
non_comment_lines = [line for line in lines if not line.startswith("--")]
92+
non_comment_lines = [line for line in lines if not is_comment_line(line)]
8793
if non_comment_lines:
88-
# Found a statement with actual SQL
8994
first_line = non_comment_lines[0].upper()
9095
first_word = first_line.split()[0] if first_line else ""
9196
return QueryKind.RETURNS_ROWS if first_word in SELECT_KEYWORDS else QueryKind.NON_QUERY

sqlit/domains/query/app/transaction.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,8 @@ def atomic_execute(
274274
Raises:
275275
Exception: If any statement fails (after rollback).
276276
"""
277+
from sqlit.domains.query.editing.comments import is_comment_only_statement
278+
277279
from .multi_statement import (
278280
MultiStatementResult,
279281
StatementResult,
@@ -285,6 +287,9 @@ def atomic_execute(
285287
sql = normalize_for_execution(sql)
286288
statements = split_statements(sql)
287289

290+
# Filter out comment-only statements
291+
statements = [s for s in statements if not is_comment_only_statement(s)]
292+
288293
# Create a dedicated connection for this atomic operation
289294
conn = self.provider.connection_factory.connect(self.config)
290295
try:

sqlit/domains/query/editing/comments.py

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,94 @@
1-
"""Comment toggle helpers for SQL editing."""
1+
"""SQL comment handling - detection, toggling, and stripping.
2+
3+
This module is the single source of truth for SQL comment logic.
4+
All comment-related operations should use these functions.
5+
"""
26

37
from __future__ import annotations
48

9+
import re
10+
511
SQL_COMMENT_PREFIX = "-- "
612

13+
# Regex patterns for comment stripping
14+
_LINE_COMMENT_RE = re.compile(r"--[^\n]*")
15+
_BLOCK_COMMENT_RE = re.compile(r"/\*.*?\*/", re.DOTALL)
16+
17+
18+
def is_comment_line(line: str) -> bool:
19+
"""Check if a line is a SQL line comment.
20+
21+
Args:
22+
line: A single line of text.
23+
24+
Returns:
25+
True if the line (after stripping whitespace) starts with --.
26+
"""
27+
return line.strip().startswith("--")
28+
29+
30+
def is_comment_only_statement(statement: str) -> bool:
31+
"""Check if a SQL statement contains only comments (no actual SQL).
32+
33+
A statement is comment-only if every non-empty line starts with --.
34+
This is used to filter out statements that would cause "empty query"
35+
errors in database drivers that strip comments before execution.
36+
37+
Args:
38+
statement: A SQL statement (may be multi-line).
39+
40+
Returns:
41+
True if the statement contains only comments.
42+
"""
43+
lines = statement.strip().split("\n")
44+
for line in lines:
45+
stripped = line.strip()
46+
if stripped and not stripped.startswith("--"):
47+
return False
48+
return True
49+
50+
51+
def strip_line_comments(sql: str) -> str:
52+
"""Remove all line comments (-- ...) from SQL.
53+
54+
Note: This is a simple regex-based approach that doesn't account for
55+
-- inside string literals. For keyword detection this is usually fine,
56+
but for actual SQL transformation, use with caution.
57+
58+
Args:
59+
sql: SQL text that may contain line comments.
60+
61+
Returns:
62+
SQL with line comments removed.
63+
"""
64+
return _LINE_COMMENT_RE.sub("", sql)
65+
66+
67+
def strip_block_comments(sql: str) -> str:
68+
"""Remove all block comments (/* ... */) from SQL.
69+
70+
Args:
71+
sql: SQL text that may contain block comments.
72+
73+
Returns:
74+
SQL with block comments removed.
75+
"""
76+
return _BLOCK_COMMENT_RE.sub("", sql)
77+
78+
79+
def strip_all_comments(sql: str) -> str:
80+
"""Remove all comments (both line and block) from SQL.
81+
82+
Args:
83+
sql: SQL text that may contain comments.
84+
85+
Returns:
86+
SQL with all comments removed.
87+
"""
88+
result = strip_block_comments(sql)
89+
result = strip_line_comments(result)
90+
return result
91+
792

893
def toggle_comment_lines(text: str, start_row: int, end_row: int) -> tuple[str, int]:
994
"""Toggle SQL comments on a range of lines.
@@ -30,9 +115,8 @@ def toggle_comment_lines(text: str, start_row: int, end_row: int) -> tuple[str,
30115
# Determine if we should comment or uncomment based on first non-empty line
31116
should_comment = True
32117
for row in range(start_row, end_row + 1):
33-
stripped = lines[row].lstrip()
34-
if stripped:
35-
should_comment = not stripped.startswith("--")
118+
if lines[row].strip():
119+
should_comment = not is_comment_line(lines[row])
36120
break
37121

38122
# Apply toggle to each line

sqlit/domains/query/ui/mixins/query_editing_comments.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,30 @@ def action_gc_selection(self: QueryMixinHost) -> None:
9090
new_text, new_col = toggle_comment_lines(text, start_row, end_row)
9191
self.query_input.text = new_text
9292
self.query_input.cursor_location = (start_row, new_col)
93+
94+
def action_gc_statement(self: QueryMixinHost) -> None:
95+
"""Toggle comment on statement at cursor (gcS)."""
96+
self._clear_leader_pending()
97+
from sqlit.domains.query.app.multi_statement import find_statement_at_cursor
98+
from sqlit.domains.query.editing.comments import toggle_comment_lines
99+
100+
text = self.query_input.text
101+
if not text or not text.strip():
102+
return
103+
104+
row, col = self.query_input.cursor_location
105+
result = find_statement_at_cursor(text, row, col)
106+
107+
if result is None:
108+
return
109+
110+
_, start_offset, end_offset = result
111+
112+
# Convert character offsets to line numbers
113+
start_row = text[:start_offset].count("\n")
114+
end_row = text[:end_offset].count("\n")
115+
116+
self._push_undo_state()
117+
new_text, new_col = toggle_comment_lines(text, start_row, end_row)
118+
self.query_input.text = new_text
119+
self.query_input.cursor_location = (start_row, new_col)

sqlit/domains/query/ui/mixins/query_execution.py

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def action_execute_query_atomic(self: QueryMixinHost) -> None:
5252
self.notify("Connect to a server to execute queries", severity="warning")
5353
return
5454

55-
query = self.query_input.text.strip()
55+
query = self._get_query_to_execute()
5656

5757
if not query:
5858
self.notify("No query to execute", severity="warning")
@@ -113,13 +113,36 @@ def _proceed() -> None:
113113

114114
self._maybe_confirm_query(statement, _proceed)
115115

116+
def _get_query_to_execute(self: QueryMixinHost) -> str:
117+
"""Return selected text if present, otherwise the full query."""
118+
selection = self.query_input.selection
119+
if selection.start != selection.end:
120+
from sqlit.domains.query.editing import get_selection_text
121+
122+
start = selection.start
123+
end = selection.end
124+
if start > end:
125+
start, end = end, start
126+
127+
selected_sql = get_selection_text(
128+
self.query_input.text,
129+
start[0],
130+
start[1],
131+
end[0],
132+
end[1],
133+
)
134+
if selected_sql and selected_sql.strip():
135+
return selected_sql.strip()
136+
137+
return self.query_input.text.strip()
138+
116139
def _execute_query_common(self: QueryMixinHost, keep_insert_mode: bool) -> None:
117140
"""Common query execution logic."""
118141
if self.current_connection is None or self.current_provider is None:
119142
self.notify("Connect to a server to execute queries", severity="warning")
120143
return
121144

122-
query = self.query_input.text.strip()
145+
query = self._get_query_to_execute()
123146

124147
if not query:
125148
self.notify("No query to execute", severity="warning")
@@ -148,6 +171,7 @@ def _maybe_confirm_query(self: QueryMixinHost, query: str, proceed: Callable[[],
148171
format_alert_mode,
149172
should_confirm,
150173
)
174+
from sqlit.domains.query.app.multi_statement import get_executable_sql
151175
from sqlit.shared.ui.screens.confirm import ConfirmScreen
152176

153177
raw_mode = getattr(self.services.runtime, "query_alert_mode", 0) or 0
@@ -160,7 +184,14 @@ def _maybe_confirm_query(self: QueryMixinHost, query: str, proceed: Callable[[],
160184
proceed()
161185
return
162186

163-
severity = classify_query_alert(query)
187+
# Get the SQL that will actually execute (without comment-only statements)
188+
executable_sql = get_executable_sql(query)
189+
if not executable_sql:
190+
# Nothing to execute after filtering comments
191+
proceed()
192+
return
193+
194+
severity = classify_query_alert(executable_sql)
164195
if severity == AlertSeverity.NONE or not should_confirm(mode, severity):
165196
proceed()
166197
return
@@ -171,13 +202,6 @@ def _maybe_confirm_query(self: QueryMixinHost, query: str, proceed: Callable[[],
171202
elif severity == AlertSeverity.WRITE:
172203
title = "Confirm write query"
173204

174-
description = None
175-
snippet = query.strip().splitlines()[0] if query.strip() else ""
176-
if snippet:
177-
if len(snippet) > 120:
178-
snippet = snippet[:117] + "..."
179-
description = snippet
180-
181205
def _on_result(confirmed: bool | None) -> None:
182206
if confirmed:
183207
proceed()
@@ -188,7 +212,7 @@ def _on_result(confirmed: bool | None) -> None:
188212
)
189213

190214
self.push_screen(
191-
ConfirmScreen(title, description, yes_label="Run", no_label="Cancel"),
215+
ConfirmScreen(title, executable_sql, yes_label="Yes", no_label="No"),
192216
_on_result,
193217
)
194218

@@ -386,17 +410,20 @@ async def _run_query_async(self: QueryMixinHost, query: str, keep_insert_mode: b
386410

387411
# Use TransactionExecutor for transaction-aware query execution
388412
executor = self._get_transaction_executor(config, provider)
389-
# Check if this is a multi-statement query
413+
# Check if this is a multi-statement query (after filtering comment-only statements)
414+
from sqlit.domains.query.editing.comments import is_comment_only_statement
415+
390416
statements = split_statements(query)
391-
is_multi_statement = len(statements) > 1
417+
executable_statements = [s for s in statements if not is_comment_only_statement(s)]
418+
is_multi_statement = len(executable_statements) > 1
392419

393420
try:
394421
start_time = time.perf_counter()
395422
max_rows = self.services.runtime.max_rows or MAX_FETCH_ROWS
396423

397424
use_process_worker = self._use_process_worker(provider)
398-
if use_process_worker and statements:
399-
statement = statements[0].strip()
425+
if use_process_worker and executable_statements:
426+
statement = executable_statements[0].strip()
400427
if self.in_transaction or is_transaction_start(statement) or is_transaction_end(statement):
401428
use_process_worker = False
402429

sqlit/shared/ui/screens/confirm.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,16 @@ class ConfirmScreen(ModalScreen):
2828
}
2929
3030
#confirm-dialog {
31-
width: 36;
31+
width: auto;
32+
min-width: 36;
33+
max-width: 80%;
3234
}
3335
3436
#confirm-description {
3537
margin-bottom: 1;
3638
color: $text-muted;
39+
max-height: 20;
40+
overflow-y: auto;
3741
}
3842
3943
#confirm-list {

0 commit comments

Comments
 (0)