darrenburns / dunk

Prettier git diffs in the terminal 🎨
758 stars 15 forks source link

Broken Pipe error when piping a long diff to less and quitting #15

Open nmay231 opened 2 years ago

nmay231 commented 2 years ago

To reproduce: get a very long diff, pipe the output of dunk to less or another pager, and quit without reading the entire thing.

python -c "print('line\n' * 1000)" > tmp
git add tmp
git diff HEAD | dunk | less -R  # Exit pager without scrolling

# ...
# BrokenPipeError: [Errno 32] Broken pipe

The desired behavior would be to either 1) completely ignore broken pipe errors or 2) catch broken pipe errors and print a one-liner to stderr. I would much prefer the first one.

Thank you so much for the project btw :smile:

darrenburns commented 2 years ago

I looked into this a while back and I think the only way to properly handle it was to use a SIGPIPE signal handler - catching the BrokenPipeError wasn't enough. I never got around to implementing though. I may try at some point but also open to PRs from yourself or anyone else who may be interested.

nmay231 commented 2 years ago

@darrenburns Catching BrokenPipeError seems to work fine for me (on Ubuntu 20.04). I did have to move the try-except from the if __name__ == "__main__" block in dunk.py into the entry function so that the poetry installed executable would have the try-except (perhaps that was the issue).

Would you try it out in my PR #16?

I am temporarily calling main() from main_wrapper() to make the commit smaller, but we can have it inside of main if you wish.

If this solution doesn't work, there's also a SO answer that suggests we can forcibly close stderr in the except block to prevent the output sys.stderr.close()