Disable line wrap #983
Disable line wrap #983
Conversation
src/ui/printer.rs
Outdated
| // Disable line wrapping | ||
| write!(self.writer, "\x1B[?7l")?; | ||
|
|
||
| write!(self.writer, "{buf}")?; | ||
|
|
||
| // Re-enable line wrapping | ||
| write!(self.writer, "\x1B[?7h")?; |
There was a problem hiding this comment.
I believe this approach is relatively safe. My main concern was to avoid leaving the user's terminal in a modified state if an error occurred, with line wrapping still disabled. However, based on this diff, the only situation where that might happen is if the program panics at line 141.
What are your thoughts on this?
Another option could be to implement the Drop trait to Printer<W> and include write!(self.writer, "\x1B[?7h")? in it. However, that might be excessive 🤔 . What do you think?
There was a problem hiding this comment.
No, this looks good to me! Since Drop::drop doesn't return a Result, the call to write! would have to unwrap, right? Probably best to keep potentially panicking code out of drop 🙂
Any reason not to make it one call to write! instead of 3?
There was a problem hiding this comment.
No, this looks good to me! Since Drop::drop doesn't return a Result, the call to write! would have to unwrap, right? Probably best to keep potentially panicking code out of drop 🙂
Very well, that reassures me 😩
Any reason not to make it one call to write! instead of 3?
You're right it's better if we merge them into one -> ddba00b
src/ui/printer.rs
Outdated
| // Disable line wrapping | ||
| write!(self.writer, "\x1B[?7l")?; | ||
|
|
||
| write!(self.writer, "{buf}")?; | ||
|
|
||
| // Re-enable line wrapping | ||
| write!(self.writer, "\x1B[?7h")?; |
There was a problem hiding this comment.
No, this looks good to me! Since Drop::drop doesn't return a Result, the call to write! would have to unwrap, right? Probably best to keep potentially panicking code out of drop 🙂
Any reason not to make it one call to write! instead of 3?
|
Didn't mean to duplicate comments. That's an odd behavior of GitHub 🤔 😆 |
|
Thanks for the review @spenserblack I didn't expect this issue to be resolved with a one-line PR. 😅 |
|
Just thought of something: how does Ctrl+C handle this? If onefetch is taking a long time (like in a massive repo) a user might try to terminate the process. Could that potentially leave the terminal in a "no wrap" state? |
|
That's a good point. But since we disable line wrapping only after generating the output, we should be fine. If the user decides to halt the program, it is likely to happen during the commit traversal or language stat computation stages. |
#980