fix(cmd): prevent ANSI escape sequences in piped output#16859
Closed
xbrxr03 wants to merge 1 commit into
Closed
Conversation
When stdout is not a terminal (e.g. `ollama run llama3 'explain quantum computing' > output.txt`), displayResponse() would emit cursor-control escape sequences like \x1b[%dD (cursor back) and \x1b[K (clear line). These are part of the word-wrap logic and only make sense on a real terminal. In piped output they show up as garbage characters. The fix adds a plainText bool parameter to displayResponse(). When true (stdout is not a TTY), the word-wrap path is skipped entirely and content is printed as plain text. The existing plainText variable in generate() already tracks this — we now thread it through to displayResponse(). Chat mode calls pass plainText=false since chat is always interactive. Generate mode uses the already-computed plainText = !term.IsTerminal(). This is a more explicit approach than checking term.GetSize() errors (which could fail for other reasons) and makes the intent clear in the function signature. Closes ollama#16785
Author
|
Closing this PR due to lack of maintainer engagement. I'm happy to reopen if there's interest. Thanks for the great project. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When piping
ollama runoutput to a file or another command, the output contains ANSI escape sequences like\x1b[14D(cursor back) and\x1b[K(clear line). These are part of the word-wrap logic and only make sense on a real terminal. In redirected output they appear as garbage:Reported in #16785.
Fix
Add a
plainText boolparameter todisplayResponse(). Whentrue, the word-wrap path (which emits escape sequences) is skipped entirely and content is printed as plain text.The
generate()function already computesplainText := !term.IsTerminal(int(os.Stdout.Fd()))— this PR threads it through todisplayResponse()so the function knows whether it's writing to a terminal or not.Chat mode calls pass
plainText=falsesince chat is always interactive. Generate mode uses the already-computedplainTextvalue.Why not PR #16840's approach?
PR #16840 checks
term.GetSize()errors to detect non-TTY output. That works but has a subtle issue:term.GetSize()can fail for reasons other than "not a TTY" (e.g., the TTY exists but its size can't be determined). In those cases, word-wrap should still be disabled since we don't know the terminal width, but the approach conflates "can't get size" with "not a terminal."This PR is more explicit:
plainText bool)falsegenerate()function already had theplainTextvariable ready to thread throughTest Plan
3 new sub-tests under
TestDisplayResponseNoEscapesWhenPlainText:plain_text_no_escapes: verifies no\x1bbytes in output whenplainText=truewith long content that would normally wrapinteractive_with_escapes: verifiesplainText=falsedoesn't crash and produces outputplain_text_short_content: verifies short content passes through unchanged whenplainText=trueFull cmd test suite passes.
Closes #16785