Skip to content

Add --batch option as an alternative to STDIN#1723

Merged
rolandwalker merged 1 commit intomainfrom
RW/add-batch-option-for-scripting
Mar 20, 2026
Merged

Add --batch option as an alternative to STDIN#1723
rolandwalker merged 1 commit intomainfrom
RW/add-batch-option-for-scripting

Conversation

@rolandwalker
Copy link
Contributor

@rolandwalker rolandwalker commented Mar 17, 2026

Description

Add --batch option as an alternative to STDIN, factoring out a statements_from_filehandle() function which can also be used to improve the functionality of the source command.

The initial behavior when both STDIN is not-a-tty and --batch is in effect is to warn, and use --batch. Similarly, the priority of the --execute option was clarified with warnings. These may be revisited: we could consider different behavior, as well as exiting with an error in case of warning.

Motivation: display of a prompt_toolkit progress bar during batch execution. If the input is from STDIN, we can't know how many statements are the goal, at least without creating a tempfile. As mentioned above, this also lays the basis to improve the source command, which reads the entire file into memory.

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 .

@rolandwalker rolandwalker self-assigned this Mar 17, 2026
@github-actions
Copy link

No correctness or security regressions stood out in the PR diff.

Residual testing gaps (worth adding before merge):

  1. Missing coverage for --batch - (explicit STDIN path), including precedence behavior when STDIN is non-tty.

  2. Missing coverage for the new MAX_MULTILINE_BATCH_STATEMENT guard raising ValueError in the factored parser.

  3. No direct unit tests for statements_from_filehandle() splitting behavior across multi-statement input and trailing non-terminated statement.

I couldn’t run tests locally because pytest/uv aren’t installed in this environment.

@rolandwalker rolandwalker added the codex request Codex review for an updated PR label Mar 20, 2026
@github-actions
Copy link

  1. Regression: --batch does not expand ~ paths
    File: mycli/main.py:2570
    click.open_file(batch_file) uses the literal string, so --batch '~/script.sql' fails (especially when quoted), while source already does os.path.expanduser(...) in mycli/main.py:483.
    Action: expand user paths before open, e.g. click.open_file(os.path.expanduser(batch_file)).

  2. Missing edge-case coverage for new batch flow
    Files: test/test_main.py:1313, mycli/packages/batch_utils.py:8
    Current tests cover happy path and option precedence with --execute, but not key new edges:

  • --batch - should read stdin explicitly.
  • both piped stdin and --batch file should verify warning + file wins behavior.
  • MAX_MULTILINE_BATCH_STATEMENT overflow path (ValueError handling) should exit cleanly.
  • statements_from_filehandle() should be unit-tested directly for trailing-no-semicolon and multi-statement boundaries.

No security-specific issues stood out in this diff.

Assumption: I could not run pytest here (pytest not installed in this environment), so findings are from static review only.

@rolandwalker rolandwalker removed codex request Codex review for an updated PR labels Mar 20, 2026
factoring out a statements_from_filehandle() function which can also be
used to improve the functionality of the "source" command.

The initial behavior when both STDIN is not-a-tty and --batch is in
effect is to warn, and use --batch.  Similarly, the priority of the
--execute option was clarified with warnings.  These may be revisited:
we could consider different behavior, as well as exiting with an error
in case of warning.

Motivation: display of a prompt_toolkit progress bar during batch
execution.  If the input is from STDIN, we can't know how many
statements are the goal, at least without creating a tempfile.  As
mentioned above, this also lays the basis to improve the "source"
command, which reads the entire file into memory.
@rolandwalker rolandwalker force-pushed the RW/add-batch-option-for-scripting branch from 0cdcb15 to d239179 Compare March 20, 2026 19:32
@rolandwalker rolandwalker merged commit 8805d1b into main Mar 20, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/add-batch-option-for-scripting branch March 20, 2026 19:36
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.

2 participants