PoC: KeyC and keyItemC shorter secondary constructors #2
Conversation
There was a problem hiding this comment.
I still think this is a solution without a problem, because people are still going to have to copy-paste to create keyboards, this doesn't change that in the slightest. It just makes it a tiny more compact, while placing additional burden on me, not you, to support this alternate definition.
Overall it just makes more work for me, when we inevitably change fields in KeyC in the future.
Why not just go with my original suggestion: define your own format in your own repo, and create a script to convert them to thumbkey keyboard files. Then you're not making more work for me. I'll even link to it in the thumbkey readme (as long as you do the work to maintain it).
| color = ColorVariant.MUTED, | ||
| ), | ||
| ), | ||
| DUMMY_KEY, DUMMY_KEY, DUMMY_KEY, |
There was a problem hiding this comment.
Now you've lost all the swipetype direction limiters by adding dummy keys.
There was a problem hiding this comment.
I've introduced filtering dummy-keys out in second commit of this branch
But I don't know whether this approach isn't too liberal for your compile-time checking requirements
| @@ -48,15 +48,46 @@ data class KeyItemC( | |||
| val swipeType: SwipeNWay = SwipeNWay.EIGHT_WAY, | |||
| val slideType: SlideType = SlideType.NONE, | |||
| val longPress: KeyAction? = null, | |||
| ) | |||
| ) { | |||
There was a problem hiding this comment.
This is a much worse constructor than the default. None of these are optional, and forces you to use dummy keys. I'd prefer you got rid of this one.
There was a problem hiding this comment.
Yes, that was the idea. It forces exactly 9 args, which split into 3 lines represent key actions in human-friendly way
KeyItemC(
KeyC('f'), KeyC('&'), KeyC('°'),
KeyC('ś'), KeyC('s'), KeyC('>'),
KeyC(';'), DUMMY_KEY, DUMMY_KEY
)|
|
||
| data class KeyC( | ||
| val display: KeyDisplay?, | ||
| val capsModeDisplay: KeyDisplay? = null, | ||
| val action: KeyAction, | ||
| val color: ColorVariant = ColorVariant.SECONDARY, | ||
| val size: FontSizeVariant = FontSizeVariant.SMALL, | ||
| ) | ||
| ) { | ||
| constructor(character: Char) : this( |
There was a problem hiding this comment.
This one I could see being useful. Still scares me, because people might start defining keyboards like this, and this constructor logic could get complicated. Overall, still much worse than just the default data class constructor.
|
@dessalines ok, I think I understand your PoV. Thank you for your time 🙇 |
|
Its no probs, if I wasn't extremely overburdened with other duties I'd consider it. ping me if you end up creating that script, and I'll add it to the thumbkey readme. |
Maybe it should use helper functions instead of secondary constructors?
Is dummy keys filtering breaking compile-time checking?