add paging mode option #109
Conversation
There was a problem hiding this comment.
Awesome, thanks very much for doing this! I'm going to have to check out lazygit and what it looks like to have delta running in it.
Since github allows it, I went ahead and pushed a couple of small tweaks to your branch.
There are a couple of issues with delta that I noticed in the course of this, but none of them need concern you... Firstly, it seems like the behavior you wanted should have been achievable by setting the two relevant environment variables to empty string, but that's not true currently:
python -c "for i in range(999999): print(i)" | BAT_PAGER= PAGER= delta
Secondly, delta should only spawn the pager when its output is an interactive terminal (as opposed to a pipe or file redirect). This is how bat behaves, but that's also not true for delta currently. I've left the "auto" option vaguely documented so that it can acquire that behavior in a future PR.
| /// default pager is `less`: this can be altered by setting the environment variables BAT_PAGER | ||
| /// or PAGER (BAT_PAGER has priority). | ||
| #[structopt(long = "paging", default_value = "auto")] | ||
| pub paging_mode: String, |
There was a problem hiding this comment.
Since delta is in some ways similar to bat, I've made it a design guideline to be exactly the same as bat wherever it makes sense. bat has basically exactly the option that you're adding, so I've pushed commits to your PR making the interface identical. Here's a section from bat --help for reference.
There was a problem hiding this comment.
Sorry I should be totally explicit here, since lazy git is going to pass this option: I have changed the name of the option in your PR so that it will now be --paging=never.
| process::exit(1); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
I made a small change here to demand that if the user supplies a value, it is valid.
| @@ -624,6 +624,7 @@ mod tests { | |||
| hunk_style: cli::SectionStyle::Box, | |||
| hunk_color: "blue".to_string(), | |||
| width: Some("variable".to_string()), | |||
| paging_mode: "auto".to_string(), | |||
There was a problem hiding this comment.
I had to add an entry here to make delta's tests pass.
|
Thanks for the quick response :) |
fixes #108
This is my first ever PR in rust, hopefully the first of many!
The use case here is to allow me to use delta in lazygit without having to deal with
lessgetting in the way