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.

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

feat: refactor app.rs #21

Merged
merged 33 commits into from Feb 10, 2024
Merged

feat: refactor app.rs #21

merged 33 commits into from Feb 10, 2024

Conversation

joshka
Copy link
Member

@joshka joshka commented Feb 10, 2024

The purpose of this PR is to gradually refactor the app module to
split the search and summary functionality better. Right now app is a
module that has a bit of everything.

Don't merge this, I'll update this as I'm working through the refactor.


This PR refactors app.rs and adds a permanent statusbar.

src/widgets.rs Outdated Show resolved Hide resolved
@kdheepak kdheepak changed the title refactor app.rs feat: refactor app.rs Feb 10, 2024
@kdheepak kdheepak marked this pull request as ready for review February 10, 2024 11:30
@kdheepak
Copy link
Contributor

I think it is worth merging this at this point and continuing a refactor if needed in a separate PR. It'll be easier to track changes that way.

@joshka
Copy link
Member Author

joshka commented Feb 10, 2024

image

}

fn status(&self) -> Block {
let line = if self.mode.is_filter() {
Copy link
Member Author

Choose a reason for hiding this comment

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

When there's this much difference in the if statements, the differences seem to outweigh the value of treating this as common. I'd pull this into the search.

Keybindings needs a "First Keybinding" type method to simplify this.

@joshka
Copy link
Member Author

joshka commented Feb 10, 2024

I think it is worth merging this at this point and continuing a refactor if needed in a separate PR. It'll be easier to track changes that way.

Sounds good. I'll start a round 2 PR on it later though as I think this can still be simplified a lot.

@joshka joshka merged commit 095c5ca into main Feb 10, 2024
6 checks passed
@joshka joshka deleted the jm/refactor-app branch February 10, 2024 19:06
@kdheepak kdheepak mentioned this pull request Feb 9, 2024
@kdheepak
Copy link
Contributor

We should leave the command ignore in there so users can override defaults

kdheepak added a commit that referenced this pull request Feb 11, 2024
## 🤖 New release
* `crates-tui`: 0.1.5 -> 0.1.6

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

##
[0.1.6](v0.1.5...v0.1.6)
- 2024-02-11

### Added
- refactor app.rs
([#21](#21))
- Increase spacing between top and bottom in summary

### Fixed
- version string even without git
([#24](#24))

### Other
- Add AUR instructions
([#22](#22))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).

Signed-off-by: Dheepak <1813121+kdheepak@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants