jj
jj copied to clipboard
martinvonz
Use `minus` as a builtin pager
Added a built-in pager since the solution in #2928 felt hacky. Also should address #2044
Checklist
If applicable:
- [X ] I have updated
CHANGELOG.md - [X ] I have updated the documentation (README.md, docs/, demos/)
- [X ] I have updated the config schema (cli/src/config-schema.json)
This is currently using a branch of mine so that I could see if I could keep the -X flag behavior which was the default before. https://github.com/arijit79/minus/pull/128
One downside is that minus does not support the less -F equivalent in dynamic mode (not even starting the pager if everything will fit on the screen). I did try using the static mode but it's noticeably slower (not actually measured, just user perception based).
The minus docs say this will never be supported due to the impossibility of knowing when the dynamic output is going to end, we have more information on when that will occur as the caller of the library, maybe we can get that information propagated through?
I've been using jj with this for a little bit, there is definitely fine tuning left to do. jj show is a noticeably degraded experience right now since it winds up taking over the whole window.
Very supportive of not pinning git repos :+1:. I think if for some reason (which I don't think will happen) we can't work with minus upstream the right think to do is just republish the crate or vendor it. I think we should land this patch with upstream and iterate.
Also, after actively using it for a couple hours I think it's actually better without my patch until I figure out how to do early exit with minus. Losing the whole scroll back for things like jj status is pretty annoying.
Do you wanna give it a try on Windows first or should we merge as is?
Hey guys! So I hope I am not being an unwanted guest here. @benbrittain actually mentioned about this in a PR. So I came here to take a look at the actual code being written. While reading your comments, I found this.
The
minusdocs say this will never be supported due to the impossibility of knowing when the dynamic output is going to end, we have more information on when that will occur as the caller of the library, maybe we can get that information propagated through?
I don't think it's any true as of today. I had written that comment for <=v4.x.x releases which used a completely different model for text data that didn't allow this functionality for dynamic mode. As for v5.x.x releases this should be possible. I didn't implement this because it never crossed my mind earlier. So if you guys want this, just throw out a issue atleast.
@arijit79 you are more than welcome here! Thank you for writing minus! I'm really excited to hear that about 5.x I'll file an issue later this evening 😄