Skip to content

shell-like completion support in clickhouse keeper-client#99312

Merged
azat merged 3 commits intoClickHouse:masterfrom
azat:keeper-improve-completion
Mar 17, 2026
Merged

shell-like completion support in clickhouse keeper-client#99312
azat merged 3 commits intoClickHouse:masterfrom
azat:keeper-improve-completion

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Mar 12, 2026

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

shell-like completion support in clickhouse keeper-client (handle completions of arguments in quotes, i.e. 'foo ba', handle escaped arguments, i.e. foo\ ba, make ls print quoted nodes, if they have whitespaces)

Also remove on_complete_modify_callback hack, replace it with pure getCompletions implementation

Follow-up for: #97828


Note

Medium Risk
Touches keeper-client parsing and interactive tab-completion logic, which can change how commands/paths are interpreted and may introduce edge-case regressions (escaping/quoting, multi-statement handling). Changes are CLI-scoped with added regression tests.

Overview
Adds shell-like path handling to clickhouse-keeper-client: tab completion and ls output now round-trip node names that include spaces, backslashes, quotes, and other special characters via new formatKeeperNodeName plus unescape/quote-aware completion logic.

Reworks query processing to tokenize once and parse multiple statements directly (supporting escaped spaces and rejecting trailing garbage), and removes the ReplxxLineReader on_complete_modify_callback TAB-binding hook across clients. Updates/extends stateless tests to cover the new escaping/quoting and completion behaviors.

Written by Cursor Bugbot for commit bc15f8e. This will update automatically on new commits. Configure here.

@azat azat requested review from pufit and thevar1able March 12, 2026 10:03
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 12, 2026

Workflow [PR], commit [9d5ef4b]

Summary:

job_name test_name status info comment
Stateless tests (arm_asan, azure, parallel) failure
01910_view_dictionary_check_refresh FAIL cidb

AI Review

Summary

This PR adds shell-like argument parsing and tab completion in clickhouse-keeper-client, improves ls token round-tripping with formatKeeperNodeName, and removes the old ReplxxLineReader completion-modification hook in favor of parser-driven completion. Based on the current diff, I did not find new high-confidence blocker/major issues to add beyond already-posted inline bot findings.

Missing context

  • ⚠️ No CI run results/logs were provided in this review context, so runtime coverage is inferred from added tests only.

ClickHouse Rules

Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsHistory.cpp
Safe rollout
Compilation time

Final Verdict

  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Mar 12, 2026
@azat azat force-pushed the keeper-improve-completion branch from ff5c108 to 9fb2418 Compare March 12, 2026 11:59
@pufit pufit self-assigned this Mar 12, 2026
@azat azat force-pushed the keeper-improve-completion branch 3 times, most recently from dba7d1b to 8cbdf0e Compare March 12, 2026 18:29
@azat azat force-pushed the keeper-improve-completion branch from 8cbdf0e to d64c961 Compare March 12, 2026 18:59
@azat azat force-pushed the keeper-improve-completion branch from d64c961 to 8b865b2 Compare March 13, 2026 00:18
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

@azat azat force-pushed the keeper-improve-completion branch 2 times, most recently from a09bec6 to bc15f8e Compare March 13, 2026 10:12
azat and others added 2 commits March 16, 2026 15:20
…g for ls)

shell-like completion support in clickhouse keeper-client (handle
completions of arguments in quotes, i.e. 'foo ba', handle escaped
arguments, i.e. foo\ ba, make ls print quoted nodes, if they have
whitespaces)

Also remove on_complete_modify_callback hack, replace it with pure
getCompletions implementation

Co-Authored-By: Claude <noreply@anthropic.com>
@azat azat force-pushed the keeper-improve-completion branch from bc15f8e to 9d5ef4b Compare March 16, 2026 14:23
@azat azat enabled auto-merge March 16, 2026 14:30
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 17, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.80% 83.70% -0.10%
Functions 23.90% 23.90% +0.00%
Branches 76.30% 76.30% +0.00%

PR changed lines: PR changed-lines coverage: 41.38% (132/319, 0 noise lines excluded)
Diff coverage report
Uncovered code

@azat azat added this pull request to the merge queue Mar 17, 2026
Merged via the queue into ClickHouse:master with commit 807a0c3 Mar 17, 2026
161 of 163 checks passed
@azat azat deleted the keeper-improve-completion branch March 17, 2026 02:57
@robot-ch-test-poll robot-ch-test-poll added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants