Repository navigation
fix: keep piped output in order and drop italics without colors - #369
Open
mirfalltnixein wants to merge 1 commit into
Open
mirfalltnixein wants to merge 1 commit into
mirfalltnixein wants to merge 1 commit into
Conversation
output() wrote page lines with sys.stdout.buffer.write() but the newlines with print(). The text layer has its own buffer, which is only flushed per line on a terminal, so when stdout is a pipe all the newlines came out at the end, after the joined page lines. Write everything in output() as bytes (including the --markdown lines, which would otherwise reorder against the final newline) and flush pending text output first. The italics around backticked text in example descriptions were raw ANSI escapes, so they were printed even when termcolor disables colors (e.g. when piped). Only add them when colored() actually styled the text. Add a test that renders the test pages through a real pipe, with and without FORCE_COLOR. Fixes tldr-pages#367 Fixes tldr-pages#368 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
I, the human behind this, have run into this problem. I've reviewed this code and am willing to put my name on it.
Fixes #367
Fixes #368
Problem
When stdout is not a terminal (e.g.
tldr tar | cat):hexdumppadding the odd-length output;od -cshows none.)Cause
output()writes the page lines withsys.stdout.buffer.write()(raw bytes, added in improve handling of different languages #112 so pages are always UTF-8) but the newlines withprint().print()goes through the text layer, which has its own buffer. On a terminal that buffer is flushed every line, so the order looks right; on a pipe it is block-buffered, so all theprint()newlines arrive after the byte writes.\x1B[3m/\x1B[23mescapes, added aroundcolored(), so they are emitted even when termcolor has decided not to color the output.The existing
test_whole_pagereplacessys.stdout.bufferwith a stub that writes into the sameStringIOasprint(), which is why it never showed up in the tests.Fix
output()writes everything as bytes, including the--markdownlines (otherwise they would be reordered against the final newline), and flushes pending text output first so messages printed before the page (e.g. theTLDR_PLATFORMwarning) stay in front of it.emphasise_exampleonly adds the italics whencolored()actually styled the text. (termcolor has anitalicattribute only since 3.3.0, so I didn't rely on it.)test_whole_page_pipedrenders the gem and jq pages in a subprocess through a real pipe, with and withoutFORCE_COLOR. Without colors, the expected output is the rendered file with all ANSI codes stripped. It skips the no-color case on termcolor < 2.1, which doesn't disable colors when not on a TTY.One behavior change: on Windows, the newlines written by
output()are now\ninstead ofprint()'s\r\n. The page lines themselves already bypassed the text layer.Testing
pytest: 35 passed on Python 3.10 and 3.14; with termcolor 1.1.0 and 2.0.0, 33 passed and the 2 no-color cases skipped; 2.1.0, 2.4.0 and 3.3.0 all pass.flake8clean.main, and also fail if thesys.stdout.flush()is removed.tldr tar | cat -Anow prints the page line by line with no escape codes;FORCE_COLOR=1 tldr echo | cat -Akeeps the colors and italics.I used an AI assistant (Claude Code) to write this change. I reviewed the code and the tests before submitting.
🤖 Generated with Claude Code