Skip to content

New Error type in Config module#21

Closed
MoistPyro wants to merge 3 commits intoeshrh:masterfrom
MoistPyro:master
Closed

New Error type in Config module#21
MoistPyro wants to merge 3 commits intoeshrh:masterfrom
MoistPyro:master

Conversation

@MoistPyro
Copy link
Copy Markdown
Contributor

Added the type ConfigError.
All methods on the Config struct has better error handeling.
moved a function to view.rs
Theme struct now implements From

@MoistPyro
Copy link
Copy Markdown
Contributor Author

Disregard the 3rd commit. I forgot to make a branch for my changes.

@eshrh
Copy link
Copy Markdown
Owner

eshrh commented Sep 19, 2025

Hi, thank you very much, sorry for the late response! I'm new to Rust so these kind of PRs help me a lot. I went ahead and cherry-picked your first two commits.

The third commit also looks really good and I'd love to have those changes as well, but I'll hold off on it as you ask. From a cursory look, it seems like the top-level dvorak/qwerty keybind options aren't being read in the new Config::from_toml().

Let me know when you're ready, feel free to open a new PR or update your fork. If you're interested, I would like to add you as a collaborator on this repo to make it easier for you to continue your great refactoring work.

eshrh pushed a commit that referenced this pull request Sep 19, 2025
@MoistPyro
Copy link
Copy Markdown
Contributor Author

The serde stuff is not very thought out, and relies on a Alpha build of Ratatui, but I'm glad you liked my changes. I am very inexperienced with git, so hope it's not too scuffed. :)

@MoistPyro
Copy link
Copy Markdown
Contributor Author

MoistPyro commented Sep 19, 2025

The qwerty / Dvorak stuff can be added back in with some option<bool> fields maybe 🤔

@MoistPyro
Copy link
Copy Markdown
Contributor Author

Do I close this now or? since you cherry picked it? This is my first pull request, so I don't know how this works.

@eshrh
Copy link
Copy Markdown
Owner

eshrh commented Sep 27, 2025

Sure, I have some free time now and I'm ready to work on improving the config parsing. i'll close this now.

@eshrh eshrh closed this Sep 27, 2025
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.

2 participants