Send ruff's stderr to os.devnull
#2
Conversation
WalkthroughThe changes involve a modification to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FormatFunction
participant Ruff
User->>FormatFunction: Call format_python()
FormatFunction->>Ruff: Execute formatting command
Ruff-->>FormatFunction: Return formatted output
FormatFunction-->>User: Return formatted output
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Unrelated to this PR, but since there is no issue tracker in this repository: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
mdformat_ruff/__init__.py (1)
20-22: Consider documenting the stderr suppression.Since this is an intentional UX decision to suppress stderr, it would be helpful to document this behavior in the docstring.
Here's a suggested update to the docstring:
def format_python(unformatted: str, _info_str: str) -> str: r"""Format python. :param unformatted: :type unformatted: str :param _info_str: :type _info_str: str :rtype: str + + Note: stderr from the ruff formatter is suppressed to maintain clean output. + Formatting errors are still raised through CalledProcessError. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
mdformat_ruff/__init__.py(1 hunks)
🔇 Additional comments (1)
mdformat_ruff/__init__.py (1)
20-22: LGTM! Verify error handling remains intact.
The change to suppress stderr aligns with the goal of providing a cleaner output experience. The error handling through subprocess.check_output remains intact as it will still raise CalledProcessError on formatting failures.
Let's verify the error handling behavior:
✅ Verification successful
Error handling is properly implemented through subprocess.check_output
The code correctly relies on subprocess.check_output's built-in error handling, which raises CalledProcessError if the command returns a non-zero exit code. Suppressing stderr with subprocess.DEVNULL only affects the visibility of error messages, not the actual error propagation. This is appropriate for a formatting tool where errors should be handled by the parent mdformat interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the code properly handles formatting errors
# Look for try/except blocks or error handling around subprocess calls
# Search for error handling patterns
rg -A 5 "try:|except:|raise|error|Exception" mdformat_ruff/
Length of output: 61
Script:
#!/bin/bash
# Let's check the entire content of the file to understand the error handling context
cat mdformat_ruff/__init__.py
Length of output: 559
Inspired by discussion here hukkin/mdformat#435
IMO error output from commands mdformat calls in a subprocess should not be shown to the user.
Summary by CodeRabbit