-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not use :
to separate linux devices
#121
Comments
The idea of using |
As an aside, some communication advice: avoid unsubstantiated statements like "X is a bad idea". A better way to phrase this type of issue is something like: The current approach has X, Y, Z weaknesses. The new approach A fixes these weaknesses and also has improvements B, C. Approach A has downsides D, E, but to me, these are worth the advantages. |
Yes, but linux environment variables have no "types", everything is a string. By contrast, we are using a structured configuration file in the form of S-expressions. We should take advantage of that structure! It doesn't make sense to have one special case here when we use lists everywhere else. Most paths in To me, there are no disadvantages to using lists except breaking some existing configurations. We can always document this somewhere so I don't see this as a big issue. |
Sounds sensible. We can bump the major version to 2 to signal a breaking change if/when this is implemented. |
I suggest just adding an alternative option Since my dev list has 5 long ugly things in it (because so many keyboards get connected to my laptop at various times), I like the sound of having a list that I can break up into multiple lines. |
That sounds like a decent idea. For anyone that may want to implement this (including my future self), do be aware that currently, the entirety of
A consideration: should the two methods merge their items together, or should one take priority over the other? |
1. Move validation of cfg values to `parse_defcfg` instead of validating them all over the codebase. Reason: before kanata didn't show location when parsing of a cfg value failed, now it does. 2. Replace `HashMap<String, String>` with `CfgOptions`, which holds only validated cfg values. Reason: Allows to easily parse lists as values, since now the actual value parsing is no longer delayed after parser function exited. This was done with #121 in mind. 3. Move cfg options defaults to a `CfgOptions::default`. Reason: it's good to have similar code in one place.
#624 have been merged, now we got the tech to easily parse lists in defcfg |
One thing to note, that we now have not only |
Interpreting
:
as separating devices is a bad idea. We should just allow a list to be passed tolinux-dev
. For exampleThe text was updated successfully, but these errors were encountered: