Feature: Configurable indent width / tab size#941
Conversation
christippett
force-pushed
the
feature/configurable-indent-width
branch
from
2f9fd02 to
7b5a9d3
Compare
There was a problem hiding this comment.
Pull request overview
Adds a configurable indent_width (tab size) for Harlequin’s query editor by plumbing a new option from config/CLI into the app and applying it to the underlying Textual TextArea used by TextEditor.
Changes:
- Add
indent_widthsupport via config (indent_width) and CLI (--indent-width) and pass it into the app/editor collection. - Apply
indent_widthto each newly-created editor’stext_input. - Add unit + functional tests covering config loading, CLI default behavior, and tab insertion behavior.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/harlequin/components/code_editor.py |
Adds indent_width parameter to EditorCollection and applies it to each editor’s text_input. |
src/harlequin/app.py |
Accepts/parses indent_width and passes it into EditorCollection. |
src/harlequin/cli.py |
Introduces --indent-width option and threads the value into Harlequin(...). |
src/harlequin/config.py |
Extends the Profile TypedDict to include indent_width. |
tests/unit_tests/test_config.py |
Asserts indent_width is loaded from config fixtures. |
tests/unit_tests/test_cli.py |
Updates expected Harlequin(...) invocation to include default indent_width=4. |
tests/functional_tests/test_query_editor.py |
Adds an async functional test verifying indent_width is applied and affects Tab insertion. |
tests/data/unit_tests/config/pyproject.toml |
Adds indent_width to test profile config fixture. |
tests/data/unit_tests/config/good_config.toml |
Adds indent_width to test profile config fixture. |
pyproject.toml |
Updates multiple dependency pins/ranges (click/rich-click/questionary/tomlkit/boto3/etc.). |
uv.lock |
Regenerates lockfile reflecting the dependency updates (including new transitive deps). |
.python-version |
Updates the dev/runtime version used by CI static checks. |
.github/workflows/test.yml |
Updates the setup-uv action version used in test workflow. |
.github/workflows/test-build.yml |
Updates the setup-uv action version used in build test workflow. |
.github/workflows/static.yml |
Updates the setup-uv action version used in static workflow. |
.github/workflows/release.yml |
Updates uv version and switches the configured Python version used for release branch automation. |
.github/workflows/publish.yml |
Updates uv version and switches the configured Python version used for publish automation. |
Comments suppressed due to low confidence (1)
pyproject.toml:44
- This PR is titled/described as adding configurable editor indent width, but it also bumps multiple core dependencies (click/rich-click/questionary/tomlkit/boto3/etc.) and updates test/build tooling. If these upgrades aren’t required for
indent_width, consider splitting them into a separate PR (or explicitly calling out why they’re needed) to keep the change focused and reduce review/rollback risk.
dependencies = [
# textual and component libraries
"textual==6.4.0",
"textual-fastdatatable==0.14.0",
"textual-textarea==0.17.2",
# click
"click==8.3.1",
"rich-click==1.9.7",
# other deps
"duckdb>=0.8.0; python_version < '3.14'",
"shandy-sqlfmt>=0.28.2",
"platformdirs>=3.10,<5.0",
"tomlkit>=0.14.0,<0.15.0",
"questionary==2.1.1",
# ensure we install from wheels
"duckdb>=1.4.2; python_version >= '3.14'",
"pandas>=2.3; python_version >= '3.14'",
]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "but is not a valid integer." | ||
| ) | ||
| ), | ||
| ) |
There was a problem hiding this comment.
indent_width is cast to int but not validated against the CLI’s documented range (1–16). Since config files bypass Click’s IntRange, invalid values (e.g., 0 or very large numbers) can slip through and lead to unexpected editor behavior. Consider enforcing the same bounds here (and exiting with a config error) after parsing.
| ) | |
| ) | |
| if not 1 <= self.indent_width <= 16: | |
| self.exit( | |
| return_code=2, | |
| message=pretty_error_message( | |
| HarlequinConfigError( | |
| "indent_width must be between 1 and 16 (inclusive); " | |
| f"got {self.indent_width!r} from config file." | |
| ) | |
| ), | |
| ) |
| try: | ||
| self.indent_width = int(indent_width) | ||
| except ValueError: | ||
| self.exit( |
There was a problem hiding this comment.
The int(indent_width) conversion can raise TypeError (e.g., if a config sets indent_width to a non-scalar TOML type). Currently only ValueError is handled, which would crash the app instead of producing a HarlequinConfigError. Catch TypeError as well (consistent with robust config parsing).
| show_s3: str | None | ||
| locale: str | ||
| no_download_tzdata: bool | ||
| indent_width: int |
There was a problem hiding this comment.
The Profile TypedDict declares indent_width as int, but the runtime code path accepts str | int (see CLI/app parsing) to support TOML values that may be quoted. Update the type here to match the accepted inputs to avoid mypy/type drift as this option evolves.
| indent_width: int | |
| indent_width: str | int |
| async with app.run_test() as pilot: | ||
| await wait_for_workers(app) | ||
| while app.editor is None: | ||
| await pilot.pause() | ||
|
|
||
| assert app.editor.text_input is not None | ||
| assert app.editor.text_input.indent_width == 2 | ||
|
|
There was a problem hiding this comment.
This new test block doesn’t appear to be formatted per the repo’s ruff format check (there are blank lines containing whitespace in this hunk). Please run ruff format (or remove trailing whitespace) so the static analysis workflow’s ruff format . --diff step doesn’t fail.
| - name: Set up Python 3.10 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.10" | ||
| python-version: "3.14" | ||
|
|
There was a problem hiding this comment.
The step name says “Set up Python 3.10” but the workflow now installs Python 3.14. This mismatch is confusing when reading CI logs/debugging release failures; please update the step name (or the version) so they agree.
| - name: Set up Python 3.10 | ||
| uses: actions/setup-python@v6 | ||
| with: | ||
| python-version: "3.10" | ||
| python-version: "3.14" | ||
|
|
There was a problem hiding this comment.
The step name says “Set up Python 3.10” but the workflow is configured to install Python 3.14. Please align the step name with the actual version to reduce confusion when troubleshooting publish runs.
Originally posted as a discussion by @joshtemple in #542.
I'm not sure when they added support for it, but Textual now has the ability to set custom tab sizes for their
TextAreacomponent (see here). Understanding that @tconbeer uses a variation of this component for Harlequin, I went about trying to piggyback on Textual's implementation so it could be used in the query editor. The attribute Textual uses for tab size isindent_width, which is a reactive property... so the approach I ended up going with was setting a value for this property oneditor.text_inputand becauseTextAreaPlusinherits from a recent enough version of Textual'sTextArea, it works.I haven't updated
CHANGELOG.mdyet. I'll wait for feedback on this PR and I can add a separate commit for it to the PR later.Does this PR require a change to Harlequin's docs?
Did you add or update tests for this change?
Please complete the following checklist:
CHANGELOG.md, under the[Unreleased]section heading. That entry references the issue closed by this PR.