Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented May 22, 2025

This PR addresses the issue where line prompts (like "1>", "2>", "3>") appear in the output when running sqlcmd as a subprocess from another program.

The solution automatically detects when stdin is redirected (from pipes or files) and suppresses line prompts in those cases, while preserving interactive behavior when running in a terminal.

Changes

  • Added stdin redirection detection in pkg/console/console_redirect.go
  • Modified console initialization to skip prompts when stdin is redirected
  • Preserved interactive mode with prompts when running in a terminal
  • Added tests for stdin redirection detection

This matches the behavior of the original sqlcmd tool when used in subprocess scenarios without requiring any additional flags.

Fixes #580.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • someserver
    • Triggering command: /tmp/go-build1426325623/b001/sqlcmd.test -test.testlogfile=/tmp/go-build1426325623/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@shueybubbles
Copy link
Collaborator

shueybubbles commented May 22, 2025

@copilot the liner package appears to incorrectly detect whether the process is using a visible console on Linux. We need a workaround in cmd/sqlcmd/ to turn off interactive mode when stdin is redirected by properly detecting the console state. Perhaps use https://github.com/jbranchaud/til/blob/master/go/detect-if-stdin-comes-from-a-redirect.md as a guide #Closed

@Copilot Copilot AI changed the title [WIP] Removing line prompts when running sqlcmd as a subprocess from a script on Linux Add --disable-prompts flag to suppress line prompts in non-interactive mode May 22, 2025
@Copilot Copilot AI requested a review from shueybubbles May 22, 2025 13:33
Copilot finished work on behalf of shueybubbles May 22, 2025 13:34
Copy link
Contributor Author

Copilot AI commented May 22, 2025

@copilot the liner package appears to incorrectly detect whether the process is using a visible console on Linux. We need a workaround in cmd/sqlcmd/ to turn off interactive mode when stdin is red...

I've implemented the fix to properly detect when stdin is redirected. The updated code in isConsoleInitializationRequired now checks the file mode descriptor of os.Stdin.Stat() to determine if input is coming from a terminal or a pipe/redirection. The console initialization is skipped when stdin is redirected, unless a password is required. This should resolve the issue with line prompts appearing when running sqlcmd as a subprocess. Changes are in commit 2c454b5.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • cdn.fwupd.org
    • Triggering command: /usr/bin/fwupdmgr refresh (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

#Closed

Copilot finished work on behalf of shueybubbles May 22, 2025 13:44
Copilot finished work on behalf of shueybubbles May 22, 2025 13:55
Copilot finished work on behalf of shueybubbles May 22, 2025 13:58
Copilot finished work on behalf of shueybubbles May 23, 2025 16:34
@shueybubbles shueybubbles marked this pull request as ready for review May 25, 2025 02:25
Copy link
Contributor

@apoorvdeshmukh apoorvdeshmukh left a comment

Choose a reason for hiding this comment

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

Looks good! :shipit:

@shueybubbles shueybubbles removed their assignment May 28, 2025
@apoorvdeshmukh apoorvdeshmukh requested a review from Copilot May 28, 2025 12:50
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for detecting redirected stdin and suppressing interactive prompts when sqlcmd is run in non-interactive mode (e.g., piped input), matching the behavior of the original tool.

  • Introduces isStdinRedirected and updates NewConsole to wire in a buffered stdin reader when input is redirected.
  • Modifies Readline and Close to skip prompts, history, and liner state when stdin is not a terminal.
  • Adds unit tests for stdin redirection detection and console initialization logic; bumps dependent modules in go.mod.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/console/console_redirect_test.go Adds test for isStdinRedirected behavior
pkg/console/console_redirect.go Implements isStdinRedirected helper
pkg/console/console.go Updates NewConsole, Readline, and Close for redirection
cmd/sqlcmd/stdin_console_test.go Adds test for isConsoleInitializationRequired
cmd/sqlcmd/sqlcmd.go Updates console-init logic to skip prompts on redirected stdin
go.mod Bumps golang.org/x/sys, adds golang.org/x/term
Comments suppressed due to low confidence (1)

cmd/sqlcmd/sqlcmd.go:734

  • [nitpick] The variable name iactive is confusing and appears to be a typo. Rename it to interactive or isInteractive for clarity.
iactive := args.InputFile == nil && args.Query == "" && len(args.ChangePasswordAndExit) == 0

historyFile: historyFile,
impl: liner.NewLiner(),
historyFile: historyFile,
stdinRedirected: isStdinRedirected(),
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

The prompt field is never initialized in NewConsole, so interactive prompts will always be empty. Consider setting a default prompt value (e.g. "sqlcmd> ") when stdin is a terminal.

Suggested change
stdinRedirected: isStdinRedirected(),
stdinRedirected: isStdinRedirected(),
prompt: "sqlcmd> ",

Copilot uses AI. Check for mistakes.

Comment on lines +53 to +56
t.Logf("File mode: %v", fileStat.Mode())
t.Logf("Is character device: %v", (fileStat.Mode()&os.ModeCharDevice) != 0)
t.Logf("Connection config: %+v", connectConfig)
t.Logf("RequiresPassword() returns: %v", connectConfig.RequiresPassword())
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

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

[nitpick] These t.Logf calls appear to be leftover debug logging and do not contribute to the test assertions. Consider removing them to keep the test output clean.

Suggested change
t.Logf("File mode: %v", fileStat.Mode())
t.Logf("Is character device: %v", (fileStat.Mode()&os.ModeCharDevice) != 0)
t.Logf("Connection config: %+v", connectConfig)
t.Logf("RequiresPassword() returns: %v", connectConfig.RequiresPassword())

Copilot uses AI. Check for mistakes.

Copy link
Collaborator

@stuartpa stuartpa left a comment

Choose a reason for hiding this comment

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

@copilot per the update made here:

#582 (comment)

please fix the title and PR description (a --disable-prompts flag has note been added)

Copy link
Contributor Author

Copilot AI commented May 28, 2025

@copilot per the update made here:

#582 (comment)
...

You're absolutely right. The PR title and description still reference the --disable-prompts flag which was removed in favor of automatic stdin redirection detection.

The implementation now automatically detects when stdin is redirected and suppresses prompts without requiring any command line flag.

Since I cannot directly update the PR title through the available tools, here are the corrected title and description that should be used:

Title: "Automatically suppress line prompts when stdin is redirected"

Description: This PR addresses the issue where line prompts (like "1>", "2>", "3>") appear in the output when running sqlcmd as a subprocess from another program.

The solution automatically detects when stdin is redirected (from pipes or files) and suppresses line prompts in those cases, while preserving interactive behavior when running in a terminal.

Changes

  • Added stdin redirection detection in pkg/console/console_redirect.go
  • Modified console initialization to skip prompts when stdin is redirected
  • Preserved interactive mode with prompts when running in a terminal
  • Added tests for stdin redirection detection

This matches the behavior of the original sqlcmd tool when used in subprocess scenarios without requiring any additional flags.

Copilot finished work on behalf of stuartpa May 28, 2025 13:10
@stuartpa stuartpa changed the title Add --disable-prompts flag to suppress line prompts in non-interactive mode Automatically suppress line prompts when stdin is redirected May 28, 2025
@stuartpa stuartpa merged commit 34ab426 into main May 28, 2025
8 checks passed
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.

Removing line prompts when running sqlcmd as a subprocess from a script on Linux
4 participants