Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disable line wrap #983

Merged
merged 10 commits into from
Mar 17, 2023
Merged

Disable line wrap #983

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
060cde8
disable line-wrap
o2sh Mar 15, 2023
ee9396e
add panic hook
o2sh Mar 15, 2023
34dbe7c
revert change
o2sh Mar 15, 2023
e193fd3
setup panic hook
o2sh Mar 16, 2023
8a3e6e9
remove panic
o2sh Mar 16, 2023
e3158fe
simplify logic
o2sh Mar 17, 2023
060b458
remove generic constraint
o2sh Mar 17, 2023
8ff3063
revert change on Cargo.lock
o2sh Mar 17, 2023
e8ac0c3
revert change on main.rs
o2sh Mar 17, 2023
ddba00b
merge the three writes into one
o2sh Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 41 additions & 21 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use human_panic::setup_panic;
use onefetch::cli;
use onefetch::info::Info;
use onefetch::ui::printer::Printer;
use std::io;
use std::{env, io};

fn main() -> Result<()> {
setup_panic!();

#[cfg(windows)]
let _ = enable_ansi_support::enable_ansi_support();

setup_panic!();

let config = cli::Config::parse();

if config.languages {
Expand Down
6 changes: 6 additions & 0 deletions src/ui/printer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ impl<W: Write> Printer<W> {
}
}

// Disable line wrapping
write!(self.writer, "\x1B[?7l")?;

write!(self.writer, "{buf}")?;

// Re-enable line wrapping
write!(self.writer, "\x1B[?7h")?;
Copy link
Owner Author

@o2sh o2sh Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spenserblack

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
Ok(())
Expand Down