-
-
Notifications
You must be signed in to change notification settings - Fork 593
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.
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
Add support for options handling in log and stashes views #1661 #1675
Add support for options handling in log and stashes views #1661 #1675
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for taking this! One small comment
5256db6
to
46b4e10
Compare
@pm100 mind giving this a spin? I am currently on holiday and can’t test it |
sure, I can do a PR to increase min rust version to 1.65 too (since all CI are failing now) |
@pm100 ok let’s do that. Search for all occupancies of 1.64 please especially also in CD for releases so we do not see surprises in a release |
@extrawurst Not quite sure how this has happened (I am also away but have my macbook and can remote into my home windows dev system) something has pulled crossterm 0.26 into gitui. This has a serious breaking change for windows. I already ran across this for tiu-textarea - see rhysd/tui-textarea#17 because , if you recall, I am working on integrating that into gitui (I have pending PR too, hopefully they get to that) I supposed that its not this PR but something earlier BUT my initial testing suggests its this change (I pulled gitui master branch and it was fine) The fix is simple if you know where the keyboard event conversion core is. You now need to explictly test that the crossterm key event is a keydown (ct 0.26 on windows also generates a key release event - grr). Look at the PR i linked above , its tiny If you tell me where to tweak that I can edit it in and test it (remote debugging / exploring of code I dont know is very hard - only one monitor, small screen, and v slow) EDIT: Its OK i found how to fix it. I will do a separate PR for that. I suggest you sit on whatever is causing the crossterm upgrade till then (Dont quite understand what is doing that) BTW. This PR works fine on Mac and Ubuntu (via WSL). |
Let me know if you need any further changes for this PR. |
@kamillo I was testing this on windows and hit a blocking issue pulled in from crossterm 0.26, this is now fixed in master and you have synced with that. I will finish testing tomorrow or the next day |
@extrawurst tested on macos 13.3.1 ubuntu 20.2 (WSL) and windows 11. Works fine |
Thanks @kamillo |
This Pull Request fixes/closes #1661.
It changes the following:
SharedOptions
are now passed toInspectCommitComponent
andCompareCommitsComponent
and used instead ofDiffOptions::default()
I followed the checklist:
make check
without errors