Skip to content

Commit 6643724

Browse files
BlocksecPHDFunan Zhoupre-commit-ci[bot]
authored
fix: write binary graphviz output to temp file when stdout is tty (#564)
## Summary When using graphviz output formats like png, jpeg, pdf, etc., the output is binary and was previously written directly to stdout. This causes issues when stdout is a terminal (isatty), as binary data would be spewed to the console. This fix addresses the issue by: - Detecting when stdout is a tty and the output is binary - Writing to a temporary file with appropriate extension - Printing the file path to stderr - Opening the file with webbrowser.open() This matches the behavior suggested in the original issue. ## Changes - Modified `print_graphviz()` to handle tty case for binary output - Updated `render_graphviz()` to pass output format to `print_graphviz()` - Added test `test_print_graphviz_binary_tty_handling` to verify the fix ## Testing - All existing graphviz tests pass - New test verifies the tty handling behavior Closes #103 --------- Co-authored-by: Funan Zhou <matrix@blocksec.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 2e3d1f8 commit 6643724

File tree

3 files changed

+1268
-3
lines changed

3 files changed

+1268
-3
lines changed

src/pipdeptree/_render/graphviz.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
import math
44
import os
55
import sys
6+
import tempfile
7+
import webbrowser
68
from collections import deque
79
from typing import TYPE_CHECKING
810

@@ -16,7 +18,10 @@
1618

1719
def render_graphviz(tree: PackageDAG, *, output_format: str, reverse: bool, max_depth: float = math.inf) -> None:
1820
output = dump_graphviz(tree, output_format=output_format, is_reverse=reverse, max_depth=max_depth)
19-
print_graphviz(output)
21+
if isinstance(output, bytes):
22+
print_graphviz(output, output_format=output_format)
23+
else:
24+
print_graphviz(output)
2025

2126

2227
def dump_graphviz(
@@ -76,13 +81,28 @@ def dump_graphviz(
7681
return graph.pipe()
7782

7883

79-
def print_graphviz(dump_output: str | bytes) -> None:
84+
def print_graphviz(dump_output: str | bytes, *, output_format: str = "dot") -> None:
8085
"""
81-
Dump the data generated by GraphViz to stdout.
86+
Dump the data generated by GraphViz to stdout or open in browser/viewer.
8287
8388
:param dump_output: The output from dump_graphviz
89+
:param output_format: The output format (used to determine file extension)
8490
8591
"""
92+
if isinstance(dump_output, bytes) and sys.stdout.isatty():
93+
with tempfile.NamedTemporaryFile(
94+
suffix=f".{output_format}",
95+
delete=False,
96+
) as temp_file:
97+
temp_file.write(dump_output)
98+
temp_path = temp_file.name
99+
100+
print(f"Binary output file written to: {temp_path}", file=sys.stderr) # noqa: T201
101+
print("Opening file with default application...", file=sys.stderr) # noqa: T201
102+
if not webbrowser.open(temp_path):
103+
print("Could not open file with default application. Please open it manually.", file=sys.stderr) # noqa: T201
104+
return
105+
86106
if isinstance(dump_output, str):
87107
print(dump_output) # noqa: T201
88108
else:

tests/render/test_graphviz.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,66 @@ def test_render_dot_with_depth_zero(example_dag: PackageDAG) -> None:
126126
assert "a [label=" in output
127127
assert "g [label=" in output
128128
assert "->" not in output
129+
130+
131+
def test_print_graphviz_binary_tty_handling(mocker: MockerFixture, example_dag: PackageDAG) -> None:
132+
"""Test that binary output is written to a temp file when stdout is a tty."""
133+
output = dump_graphviz(example_dag, output_format="pdf")
134+
assert isinstance(output, bytes)
135+
136+
# Mock stdout.isatty() to return True
137+
mock_stdout = mocker.patch.object(sys, "stdout")
138+
mock_stdout.isatty.return_value = True
139+
mock_stdout.fileno.return_value = 1
140+
141+
# Mock webbrowser.open to avoid actually opening a browser
142+
mock_open = mocker.patch("webbrowser.open")
143+
144+
# Capture temp file creation
145+
class MockTempFile:
146+
def __init__(self, *_args: object, **_kwargs: object) -> None:
147+
self._content = b""
148+
self._name = "/tmp/pipdeptree_test_output.pdf" # noqa: S108 # Mock path for testing
149+
self._closed = False
150+
151+
def write(self, data: bytes) -> None:
152+
self._content += data
153+
154+
@property
155+
def name(self) -> str:
156+
return self._name
157+
158+
def __enter__(self) -> "MockTempFile": # noqa: PYI034, UP037
159+
return self
160+
161+
def __exit__(self, *_args: object) -> None:
162+
self._closed = True
163+
164+
mocker.patch("tempfile.NamedTemporaryFile", return_value=MockTempFile())
165+
166+
print_graphviz(output, output_format="pdf")
167+
168+
# Verify that webbrowser.open was called with the temp file path
169+
mock_open.assert_called_once_with("/tmp/pipdeptree_test_output.pdf") # noqa: S108 # Mock path for testing
170+
171+
172+
def test_print_graphviz_binary_non_tty_handling(mocker: MockerFixture, example_dag: PackageDAG) -> None:
173+
"""Test that binary output is written directly to stdout when stdout is not a tty."""
174+
output = dump_graphviz(example_dag, output_format="pdf")
175+
assert isinstance(output, bytes)
176+
177+
# Mock stdout.isatty() to return False (non-tty, e.g., piped output)
178+
mock_stdout = mocker.patch.object(sys, "stdout")
179+
mock_stdout.isatty.return_value = False
180+
mock_stdout.fileno.return_value = 1
181+
182+
# Mock fdopen to capture binary write
183+
mock_fdopen = mocker.patch("os.fdopen")
184+
mock_bytestream = mocker.MagicMock()
185+
mock_fdopen.return_value.__enter__.return_value = mock_bytestream
186+
187+
print_graphviz(output, output_format="pdf")
188+
189+
# Verify that binary data was written to stdout
190+
mock_fdopen.assert_called_once_with(1, "wb")
191+
mock_bytestream.write.assert_called_once_with(output)

0 commit comments

Comments
 (0)