Skip to content

Commit 9619dba

Browse files
committed
run(fix[trim]): Capture output faithfully
why: The runner trimmed each captured line and dropped blank lines, so whitespace-significant output was corrupted: captured diffs were rejected by `git apply`, `cat-file blob` lost indentation and blank lines, and multi-line stderr was concatenated into one run-on line. Single-token reads like `rev-parse HEAD` hid the defect. what: - Capture stdout/stderr verbatim; decode once; trim only the whole output (str.rstrip), preserving leading indentation, blank lines, and interior structure. - Add `trim` (default True). Pass `trim=False` for byte-faithful output -- diffs that feed `git apply`, exact blob contents. - Rejoin stderr without smashing lines together. - Correct the Svn.blame doctest, which had encoded the stripped output. - Cover trim=False fidelity, the default trim contract, and stderr line preservation with functional tests.
1 parent 967ca4b commit 9619dba

3 files changed

Lines changed: 109 additions & 18 deletions

File tree

src/libvcs/_internal/run.py

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def console_to_str(s: bytes) -> str:
3434
try:
3535
return s.decode(console_encoding)
3636
except UnicodeDecodeError:
37-
return s.decode("utf_8")
37+
return s.decode("utf_8", errors="backslashreplace")
3838
except AttributeError: # for tests, #13
3939
return str(s)
4040

@@ -149,6 +149,7 @@ def run(
149149
umask: int = -1,
150150
log_in_real_time: bool = False,
151151
check_returncode: bool = True,
152+
trim: bool = True,
152153
callback: ProgressCallbackProtocol | None = None,
153154
timeout: float | None = None,
154155
) -> str:
@@ -157,6 +158,13 @@ def run(
157158
Run 'args' in a shell and return the combined contents of stdout and
158159
stderr (Blocking). Throws an exception if the command exits non-zero.
159160
161+
Output is captured verbatim. When ``trim`` is True (the default)
162+
:meth:`str.rstrip` removes trailing whitespace from the whole output,
163+
preserving leading indentation, blank lines, and interior structure. Pass
164+
``trim=False`` for byte-faithful output -- for example a ``git diff``
165+
destined for ``git apply``, which requires the trailing newline, or
166+
``git cat-file blob`` whose contents must round-trip exactly.
167+
160168
Keyword arguments are passthrough to :class:`subprocess.Popen`.
161169
162170
Parameters
@@ -181,6 +189,14 @@ def run(
181189
Indicate whether a ``libvcs.exc.CommandError`` should be raised if return
182190
code is different from 0.
183191
192+
trim : bool
193+
When True (default), strip trailing whitespace from the whole output
194+
for the convenient "bare value" reads callers expect (e.g.
195+
``rev-parse HEAD``). When False, return the output verbatim, including
196+
any trailing newline, so whitespace-significant output (diffs, blob
197+
contents) round-trips exactly. On a non-zero exit the captured stderr
198+
used for the error output is trimmed the same way.
199+
184200
callback : ProgressCallbackProtocol
185201
callback to return output as a command executes, accepts a function signature
186202
of ``(output, timestamp)``. Example usage::
@@ -273,29 +289,25 @@ def progress_cb(output: t.AnyStr, timestamp: datetime.datetime) -> None:
273289
callback(output="\r", timestamp=datetime.datetime.now())
274290

275291
if proc.stdout is not None:
276-
stdout_lines: list[bytes] = (
277-
timeout_stdout.split(b"\n")
292+
raw_stdout: bytes = (
293+
timeout_stdout
278294
if timeout_stdout is not None
279-
else proc.stdout.readlines()
295+
else b"".join(proc.stdout.readlines())
280296
)
281-
lines: t.Iterable[bytes] = filter(
282-
None,
283-
(line.strip() for line in stdout_lines),
284-
)
285-
all_output = console_to_str(b"\n".join(lines))
297+
all_output = console_to_str(raw_stdout)
298+
if trim:
299+
all_output = all_output.rstrip()
286300
else:
287301
all_output = ""
288302
if code and proc.stderr is not None:
289-
stderr_raw: list[bytes] = (
290-
timeout_stderr.split(b"\n")
303+
raw_stderr: bytes = (
304+
timeout_stderr
291305
if timeout_stderr is not None
292-
else proc.stderr.readlines()
293-
)
294-
stderr_lines: t.Iterable[bytes] = filter(
295-
None,
296-
(line.strip() for line in stderr_raw),
306+
else b"".join(proc.stderr.readlines())
297307
)
298-
all_output = console_to_str(b"".join(stderr_lines))
308+
all_output = console_to_str(raw_stderr)
309+
if trim:
310+
all_output = all_output.rstrip()
299311
output = "".join(all_output)
300312
if code != 0 and check_returncode:
301313
raise exc.CommandError(

src/libvcs/cmd/svn.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ def blame(
387387
>>> svn.commit(path=new_file, message='My new commit')
388388
'...'
389389
>>> svn.blame('new.txt')
390-
'4 ... example text'
390+
' 4 ... example text'
391391
"""
392392
local_flags: list[str] = [str(target)]
393393

tests/cmd/test_git.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,3 +2704,82 @@ def test_rev_list_all_parameter(git_repo: GitSync) -> None:
27042704

27052705
# _all=True should return strictly more commits (the other-branch commit)
27062706
assert count_with_all > count_no_all
2707+
2708+
2709+
def test_run_trim_false_preserves_diff(git_repo: GitSync) -> None:
2710+
"""run(trim=False) returns byte-faithful output a patch tool can apply.
2711+
2712+
The default per-line trimming dropped leading indentation and blank
2713+
context lines and removed the trailing newline, so a captured
2714+
``git diff`` was rejected by ``git apply`` as a corrupt patch.
2715+
"""
2716+
base = (
2717+
"def greet(name):\n"
2718+
' message = "hello, " + name\n'
2719+
"\n"
2720+
" print(message)\n"
2721+
" return message\n"
2722+
)
2723+
target = git_repo.path / "greet.py"
2724+
target.write_text(base)
2725+
git_repo.cmd.run(["add", "greet.py"])
2726+
git_repo.cmd.run(["commit", "-m", "Add greet.py"])
2727+
target.write_text(base.replace("hello, ", "hi, "))
2728+
2729+
faithful = subprocess.run(
2730+
["git", "diff", "--no-color", "--", "greet.py"],
2731+
cwd=git_repo.path,
2732+
capture_output=True,
2733+
text=True,
2734+
check=True,
2735+
).stdout
2736+
captured = git_repo.cmd.run(["diff", "--no-color", "--", "greet.py"], trim=False)
2737+
assert captured == faithful
2738+
2739+
# Restore the clean pre-image so the captured patch can be validated.
2740+
target.write_text(base)
2741+
check = subprocess.run(
2742+
["git", "apply", "--check"],
2743+
cwd=git_repo.path,
2744+
input=captured,
2745+
capture_output=True,
2746+
text=True,
2747+
)
2748+
assert check.returncode == 0, check.stderr
2749+
2750+
2751+
def test_run_trim_false_preserves_blob(git_repo: GitSync) -> None:
2752+
"""run(trim=False) round-trips file contents byte-for-byte."""
2753+
base = "a\n indented\n\nb\n"
2754+
target = git_repo.path / "blob.txt"
2755+
target.write_text(base)
2756+
git_repo.cmd.run(["add", "blob.txt"])
2757+
git_repo.cmd.run(["commit", "-m", "Add blob.txt"])
2758+
2759+
blob = git_repo.cmd.run(["cat-file", "blob", "HEAD:blob.txt"], trim=False)
2760+
assert blob == base
2761+
2762+
2763+
def test_run_default_trims_trailing_newline(git_repo: GitSync) -> None:
2764+
"""Default run() keeps the no-trailing-newline contract callers rely on."""
2765+
sha = git_repo.cmd.run(["rev-parse", "HEAD"])
2766+
2767+
assert "\n" not in sha
2768+
assert sha == sha.strip()
2769+
2770+
2771+
def test_run_failure_preserves_stderr_lines(git_repo: GitSync) -> None:
2772+
"""A failed command keeps stderr line structure in CommandError.output.
2773+
2774+
Previously stderr lines were rejoined with no separator, smashing a
2775+
multi-line git error into a single run-on string.
2776+
"""
2777+
with pytest.raises(exc.CommandError) as excinfo:
2778+
git_repo.cmd.run(["push", "no-such-remote", "HEAD"])
2779+
2780+
output = excinfo.value.output
2781+
# Multi-line stderr keeps its line breaks (previously smashed into one
2782+
# run-on line). Assert on the echoed remote name, which git does not
2783+
# localize, rather than on translatable English error text.
2784+
assert "\n" in output
2785+
assert "no-such-remote" in output

0 commit comments

Comments
 (0)