Create .envrc and fix Cargo.lock, added h/l nav controls #53
Conversation
| @@ -0,0 +1 @@ | |||
| use flake | |||
There was a problem hiding this comment.
direnv is a way of automatically sourcing a file when entering a directory. .envrc is what it looks for (think of it as a .env file, but much more powerful). In this case, I am using it to enter a nix devshell (really useful for reproducible runs)
| KeyCode::Char('h') => match app.focused_block { | ||
| FocusedBlock::Adapter => { | ||
| if let Some(selected_controller) = app.controller_state.selected() { | ||
| let controller = &app.controllers[selected_controller]; | ||
| if controller.new_devices.is_empty() { | ||
| app.focused_block = FocusedBlock::PairedDevices; | ||
| } else { | ||
| app.focused_block = FocusedBlock::NewDevices; | ||
| } | ||
| } | ||
| } | ||
| FocusedBlock::PairedDevices => { | ||
| app.focused_block = FocusedBlock::Adapter; | ||
| } | ||
| FocusedBlock::NewDevices => { | ||
| app.focused_block = FocusedBlock::PairedDevices; | ||
| app.reset_devices_state(); | ||
| } | ||
| _ => {} | ||
| }, | ||
|
|
||
| KeyCode::Char('l') => match app.focused_block { | ||
| FocusedBlock::Adapter => { | ||
| app.focused_block = FocusedBlock::PairedDevices; | ||
| app.reset_devices_state(); | ||
| } | ||
| FocusedBlock::PairedDevices => { | ||
| if let Some(selected_controller) = app.controller_state.selected() { | ||
| let controller = &app.controllers[selected_controller]; | ||
| if controller.new_devices.is_empty() { | ||
| app.focused_block = FocusedBlock::Adapter; | ||
| } else { | ||
| app.focused_block = FocusedBlock::NewDevices; | ||
| } | ||
| } | ||
| } | ||
| FocusedBlock::NewDevices => app.focused_block = FocusedBlock::Adapter, | ||
| _ => {} | ||
| }, | ||
|
|
||
|
|
There was a problem hiding this comment.
Your Cargo.toml didn't seem to work right. I ran a reconfig on it, and it gave me this. I can't see any breaking changes. The reason it was broken is probably that your local cargo install didn't match what you set declaratively. For this reason I recommend nix for development.
Edit: misunderstood the question. These changes are what initially made me PR. I added navigation code to your app because I wanted it to feel more like lazygit. Can't see any reason for not merging these.
There was a problem hiding this comment.
Those changes have nothing to do with the PR. You are introducing changes without explaining why are doing them.
I don't mind merging this PR as long as it is in the scope of .envrc. if you wanna make other changes, feel free to open another PR
There was a problem hiding this comment.
I'm sorry. I'm unsure of how to help you. I feel like I've explained the changes to the best of my ability in both the comment and the original PR. Could you please explain what you are referring to? If you're talking about the added cases in handler.rs, I explain those above in the edit. I made the PR because it seemed unintuitive not to have h/l controls for navigating between subwindows.
There was a problem hiding this comment.
Sorry. Just noticed. Must have forgotten to update the title. It now also mentions the navigation changes, which are also talked about in the PR description
There was a problem hiding this comment.
I personally find the h/l controls much more convenient. I see no reason why this couldn't be integrated as it doesn't overlap any other features or functionality. This brings the app in line with how other tuis handle navigation and I believe it would lead to people finding the app much more usable.
There was a problem hiding this comment.
I also find the h/l controls more convenient and usable than using tab and shift tab, since then all navigation keys are next to eachother. Having to do keybinds to navigate is not bad, but still annoying knowing there are better alternatives.
There was a problem hiding this comment.
To move on with this PR, can you just keep the changes related to the keymap h/l for navigation and remove the rest ?
There was a problem hiding this comment.
Cherrypick the last 3 commits if you don't want any of the direnv stuff. I don't see why you wouldn't merge them though it's just improved DevEx especially for nix users.
There was a problem hiding this comment.
Also last time I checked the codebase the cargo stuff was still active weird so I think you should update Cargo.lock anyway
valyntyler
changed the title
|
@valyntyler can you plz fix the conflicts and I ll merge it |
valyntyler
force-pushed
the
master
branch
2 times, most recently
from
9071705 to
5382814
Compare
|
branch is up to date with main. reliable cargo builds from the declarative devshell. cleaned up nix flake. bluetui package flake output now compiles without re-doing much of the work it had been before the refactor. |
|
Thanks for the PR and your patience as well 😄 |
|
no problem :))) |
|
Yippee ki yay! |
3 participants
Wanted to PR some QoL features and noticed the devex could be improved with a simple direnv integration.
Edit: added QoL features