Format ledmap arrays to match keyboard layout structure#4
Format ledmap arrays to match keyboard layout structure#4jpds wants to merge 1 commit intorcorre:mainfrom
Conversation
rcorre
left a comment
rcorre
left a comment
There was a problem hiding this comment.
Thanks for the contribution! I think the main change is we should have a single tree-sitter query that captures LAYOUT or ledmap. Then our loop can iterate through ledmap/LAYOUT entries in order, and format each as it comes to it. That solves the problem of ordering, as well as re-parsing the document.
Maybe we could even have the same formatting logic for either (just format a list of Nodes), though I'm not sure about that.
I think a query like this might work?
[
(call_expression
(identifier) @id (#match? @id "^LAYOUT")
(argument_list) @args) @call
(init_declarator
(identifier) @id (#eq? @id "ledmap")
(initializer_list) @args) @call
]
Use a single combined tree-sitter query with two patterns to match both LAYOUT calls and ledmap identifiers in document order, so they are processed in a unified loop without re-parsing the document. The ledmap identifier is matched simply and the init_declarator located by walking up the tree, since tree-sitter-c mis-parses PROGMEM as the declarator name (putting "ledmap" in an ERROR node). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@rcorre Should be good to go now. |
rcorre
left a comment
rcorre
left a comment
There was a problem hiding this comment.
Sorry for the delay, thanks for your patience!
I was hoping it would be simpler, but now I see the issue you ran into with the treesitter error node :(
| indent: &str, | ||
| split_spaces: usize, | ||
| ) { | ||
| let column_count = rows.iter().map(|r| r.len()).max().unwrap_or(0); |
There was a problem hiding this comment.
I think you could just exit early here, right? A grid with 0 columns would be empty.
|
|
||
| last_byte = call_node.end_byte(); | ||
| } else { | ||
| // ledmap: walk up from the identifier to find init_declarator + initializer_list |
There was a problem hiding this comment.
these end up being pretty long branches, I might factor each out into a separate function for readability
| let mut tuple_cursor = inner_init.walk(); | ||
| let tuples: Vec<_> = inner_init.named_children(&mut tuple_cursor).collect(); | ||
|
|
||
| let layer_num: Option<usize> = designator |
There was a problem hiding this comment.
What happens if we use symbolic layer names instead of indices, e.g. LYR_BSE? Looks like we'll just not format it at all. Might be safer to assume the ledmap order matches the keymap order? Or just that the first LAYOUT order can be used for all of them?
| 0 | ||
| if let Some(rs) = row_structure { | ||
| log::warn!( | ||
| "Ledmap layer {} has {} tuples but layout has {} keys, using fixed formatting", |
There was a problem hiding this comment.
Is this a typical/supported setup in QMK, to have the number of ledmaps not match the number of keys? It might be safer to not touch it, than arbitrarily pick 6
| @@ -14,52 +14,52 @@ enum custom_keycodes { RGB_SLD = ML_SAFE_RANGE, }; | |||
|
|
|||
There was a problem hiding this comment.
You shouldn't need to touch the source test files here, just the snapshots.
2 participants
Add formatting for ledmap (RGB LED color) declarations so they visually match the physical keyboard layout, mirroring how LAYOUT macros are already formatted. Extract shared write_grid() for aligned column output and build_ledmap_rows() to map flat tuples onto the row structure.
Fixes #3
Formatted my keyboard layout nicely here too: jpds/qmk-keymap-jpds@9d2753d