Repository navigation
fix(read): --max-lines N returns N lines, not N/2 - #3678
Closed
dwin-gharibi wants to merge 1 commit into
Closed
dwin-gharibi wants to merge 1 commit into
dwin-gharibi wants to merge 1 commit into
Conversation
The hook rewrites head -N file to rtk read file --max-lines N, and that path called filter::smart_truncate, whose budget for ordinary lines is max_lines / 2. For any file without 'important' lines -- plain text, logs, CSV, data -- the result was exactly half the requested size with nothing to say the rest was dropped: head -20 f.txt returned 10 lines. Source files hid it, because braces, pub, export and imports count as important and are kept past the half budget, and the existing fixtures are code. Slice the first N lines instead, symmetric with --tail-lines N slicing the last N. No truncation marker: this window is one the caller asked for, and head prints none, so a marker would make the rewrite differ from the command it replaces. smart_truncate had exactly one production caller and the halving is its documented behaviour, so it goes with it rather than waiting for the next caller to inherit the same surprise.
Collaborator
|
Thanks for the detailed writeup and the comparison table against real head. #3941, merged on 2026-09-13, fixed the reported behaviour a different way: head -N now rewrites to a new --head-lines N window that slices at newline byte offsets and returns exactly N lines, while --max-lines is kept on purpose as a structural preview and is documented as such in src/main.rs. Because apply_line_window now takes a head_lines argument and the byte windows sit beside it, the change to --max-lines here and the removal of smart_truncate no longer apply, and the zero-length guard you added is already present. If a case from your table is still wrong on the current develop, please comment here or reopen. |
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.
Closes #3677.
Summary
--max-lines Nnow returns the first N lines, symmetric with--tail-lines Nreturning thelast N. It previously routed through
filter::smart_truncate, whose ordinary-line budget ismax_lines / 2, sohead -20 file— which rewrites to--max-lines 20— returned 10 linesof a plain text file.
filter::smart_truncateand its four tests. This was its only production caller, andthe halving is its documented behaviour ("Non-important lines beyond max_lines/2 are silently
skipped"), so leaving it would leave a loaded gun for the next caller.
Test plan
cargo fmt --all && cargo clippy --all-targets && cargo testrtk <command>output inspectedUnit tests (added, TDD — red before green)
src/cmds/system/read.rs:max_lines_returns_the_first_n_lines— for N in 5/10/20/50 the window is exactly N lines,starting at line 1 and contiguous. Red on
develop:--max-lines 5 must return 5 lines, got 3.max_lines_under_the_limit_is_untouched.tail_lines_still_returns_the_last_n_lines— control; passes before and after.One existing test updated
test_apply_line_window_max_lines_still_worksassertedoutput.contains("more lines"), pinningsmart_truncate's marker. It now asserts theheadcontract exactly ("a\nb\n", no marker) —see the note below.
Behavioural check against real head
head -n Nrtk read --max-lines NbeforeControls:
--tail-lines 20still equalstail -n 20; a fullrtk readstill equalscat.Suite
cargo test --all: 2624 passed, 0 failed. (2625 ondevelop; net −1 is +3 added, −4 removed withsmart_truncate.)cargo clippy --all-targets: clean.cargo fmt --all: clean.