Separate vial feature #520
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary Size Reportuse_config/nrf52832_bleDiffuse_config/nrf52840_bleDiffuse_config/nrf52840_ble_splitDiffCentral DiffPeripheral Diffuse_config/pi_pico_w_bleDiffuse_config/pi_pico_w_ble_splitDiffCentral DiffPeripheral Diffuse_config/rp2040Diffuse_config/rp2040_splitDiffCentral DiffPeripheral Diffuse_config/stm32f1Diffuse_config/stm32f4Diffuse_config/stm32h7Diff |
|
Ah, didn't take the toml config into account. I'll fix it tomorrow. |
HaoboGu
reviewed
Aug 11, 2025
rmk/src/via/vial.rs
Outdated
| @@ -1,3 +1,5 @@ | |||
| #![cfg(feature = "vial")] | |||
There was a problem hiding this comment.
It's unnecessary because this file is already behind vial feature gate in via/mod.rs
rmk/src/via/protocol.rs
Outdated
| @@ -1,3 +1,5 @@ | |||
| #![cfg(feature = "vial")] | |||
There was a problem hiding this comment.
It's unnecessary because this file is already behind vial feature gate in via/mod.rs
|
Thanks for the work! Is it possible to move vial related code to separated sub-mods? For example, adding a And, I have to admit the current implementation is not very clear, all those features aren't decoupled very well, making it hard to extract/disable a single functionality. Some refactoring is needed 🥲 |
True, I'll try to isolate vial a bit better this week, and I'll see what I can come up with ✌️. Marking this as draft again until I fix CI and do some refactoring to reduce the |
Contributors who work on Windows might accidentally commit newly created files with CRLF line endings, later causing noisy diffs when Linux developers work on the same files and they do not have `core.autocrlf = true`. However nowadays, most text editors work with LF line endings just fine on windows, so there's no need for this incompatibility. Add an `.editorconfig` file which causes the text editor to create new files using LF from the get-go instead of CRLF (VSCode needs https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig in order to conform, neovim has support for this built-in). Add a `.gitattributes` file to tell git to always checkout files using LF as line ending. https://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/ https://docs.github.com/en/get-started/git-basics/configuring-git-to-handle-line-endings
this allows users who do not need vial to use more complex combinations of features that cannot be expressed in the VIA binary format, such as e.g. non-modifier tap-hold
previously, the "storage" feature was not being tested, which lead to test code for the storage feature to become out of date
this makes sure that we don't have dead code left over when it is not needed for certain features
9464017 to
3a58a02
Compare
3a58a02 to
baeec04
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've tested these changes by running the
cargo test+cargo checkwith and without thevialfeature.I haven't tested directly that a firmware built with vial in mind still works as expected, but I've tested that some of the examples build and flash. Would need some help here from other people to test these changes, if they use vial.
Made it so we test multiple combinations of features on CI, since the storage tests have not been working for a while.
Also add
cargo checkwith different feature sets. This way, any PR will be alerted if they have leftover code that's not needed for one of the feature combinations.I've tested that I can use non-modifier tap-hold with the "vial" feature removed (although there seem to be some key repeating bugs, which I'm still investigating. I don't think they are related to this PR though). So this closes #470 .
Also added a
.gitattributesand.editorconfigfile to force everything to have LF endings. I'm on windows, and neovim createsCRLFby default, if there's no.editorconfigfile telling it otherwise.NOTE: this is a breaking change for users who have
no-default-featuresin theirCargo.toml. They will have to add back thevialfeature, unless they use thematrix_testerorvial_lockfeatures, since they automatically bring in thevialfeature.EDIT: would also close #272