refactor: Moved all layouts into their own files

[Nick-Munnich]

As discussed on Discord. Also adjusted the docs for consistency.

I am not entirely sold on the matrix transform assignments happening in the separate file as well, thoughts?

[caksoylar]

Maybe I missed the aforementioned conversation -- I don't think pulling out layouts that simply import and modify the common layouts are necessary, is it? It adds a lot of noise for a few lines of config.

[Nick-Munnich]

Maybe I missed the aforementioned conversation -- I don't think pulling out layouts that simply import and modify the common layouts are necessary, is it? It adds a lot of noise for a few lines of config.

Yeah, that's the part I wasn't certain on myself. I think it would make sense to leave that kind of thing in, but then that raises the question of how to handle situations like the Kyria correctly. I like the way you defined it originally, but I'm not sure how best to extend that to a convention.

[caksoylar]

My thinking would be that if a keys property is to be defined then you should pull it off to a separate file. You'd do that even if the keyboard only has one physical layout, which we seem to have left in the dtsi/overlay before this PR.

[joelspadin]

The presence of a keys property seems like a weird condition for switching to use a separate file to me. Part of the reason I started the discussion on Discord that led to this was because I was updating the ZMK CLI new keyboard templates for Studio, and our current documentation says to use a layouts.dtsi file only if you use keys, so then I'd need the template to either

1. Create a -layouts.dtsi file with a keys property and add a comment that you should delete keys, move the contents back to the main file, and delete -layouts.dtsi if you don't intend to support Studio.
2. Put the position map into the main file with no keys property by default and add a comment that you should create a -layouts.dtsi file and move the position map there before adding keys if you intend to support Studio.

Both of those options seemed needlessly complicated.

I don't really have strong feelings on what to do with matrix transforms. We could either

1. Define the physical layouts and position map in -layouts.dtsi and assign their matrix transforms in the main file, which is a bit verbose.
2. Define the physical layouts and position map and assign their matrix transforms in the -layouts.dtsi file, since you can reference nodes before they're actually defined.
3. Move the matrix transforms into the -layouts.dtsi file too? Then it's easier to compare them with the matching position maps, but harder to compare them with the kscan definitions (unless we also moved those).

I also don't really have strong feelings on whether -layouts.dtsi is needed if you're just pulling in common layouts. I think I'm leaning towards no.

[caksoylar]

Nick pointed me to the previous discussion on Discord offline, where there seemed to be a consensus on always splitting off to a dtsi, similar to pinctrl. I don't have strong opinions against that, so I'd be fine with the current state of the PR. I am also OK with keeping the separate file even if it is just using a common layout, for consistency.

Re: matrix transforms, 2. might be the most practical right now, however 3. might be the most consistent.

[Nick-Munnich]

Any thoughts on how to handle the kyria case? It's failing compilation because despite being disabled the 5 col node still exists and references an undefined 5 col matrix transform in the rev3 case. I could delete-node it. Could also rearrange a bunch of things to avoid needing to resort to that, but I'm not sure what the best way to rearrange would be.

[joelspadin]

Maybe you could mirror the structure of the common layouts with one physical layout + position map child per file, and then have each of the rev2 and rev3 files include just the relevant ones?

[Nick-Munnich]

Maybe you could mirror the structure of the common layouts with one physical layout + position map child per file, and then have each of the rev2 and rev3 files include just the relevant ones?

I've ended up going with this, at least for the time being. I think I'd be in favor of assigning matrix transform names to the common layouts, and then importing those directly. The layouts file would then appear if there was a need to change the name used for the matrix transform for the common layouts, something I would imagine mostly would only occur if there are other non-common layouts being defined as well. I think that approach would be just as consistent, but much less noisy.

I did try the /delete-node/ approach, but then we'd also need to delete node the position map child node and that just feels messy.

[petejohanson]

I am really stuck on this aspect of the refactor. In particular, this makes this import still highly coupled with the importing environment, and makes it harder to refactor without surprising side effects. It also makes it so that future refactors that might identify "Hey, this layout is actually useful, and should be shared!" requires extracting this coupling later anyways.

It also leads to a "two ways to do things" situation, between "using shared layouts" and "using local layouts" that would potentially lead to more confusion. IMHO, I favor Joel's first proposed option.

[Nick-Munnich]

I think I'd like consistency between the shared and the local approaches on this point, which way around it goes I'm not too fussy about. That option is a bit ugly, but justifiable imo. What would your thoughts be on applying option 2 to all the shared layouts?

I do think that option 1 should be used for binding kscans to physical transforms when different kscans are used.

[petejohanson]

IMHO matrix transforms are just as tightly coupled to the "host" that is going to inject them, and shouldn't be set either.