Xkeyboard use variant #2521
Xkeyboard use variant #2521
Conversation
|
Part of the configuration I used for validating warning and errors: Edit: indicator-icon-default -> layout-icon-default |
Codecov Report
@@ Coverage Diff @@
## master #2521 +/- ##
==========================================
+ Coverage 10.24% 10.44% +0.20%
==========================================
Files 145 147 +2
Lines 10096 10155 +59
==========================================
+ Hits 1034 1061 +27
- Misses 9062 9094 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
include/utils/string.hpp
Outdated
| @@ -62,6 +62,7 @@ namespace string_util { | |||
| using hash_type = unsigned long; | |||
|
|
|||
| bool contains(const string& haystack, const string& needle); | |||
| bool contains_nocase(const string& haystack, const string& needle); | |||
There was a problem hiding this comment.
Maybe rename to contains_ignore_case
|
Once reviewed, some commits may be merged to keep a nice history |
| variant = vec[1]; | ||
| icon = vec[2]; | ||
| } | ||
| const string& variant = size == 2 ? layouticonset::VARIANT_ANY : vec[1]; |
There was a problem hiding this comment.
I think there might be a problem with this solution because VARIANT_ANY is a static constexpr const char* and the compiler will need to create a string from it to assign this const reference but this string may be created on the stack and the reference may not live long enough to be used correctly once the ternary operator finishes.
I may be wrong as I'm not fluent in c++ const_expr
There was a problem hiding this comment.
I don't think that is an issue here. The way I understand it is that from VARIANT_ANY a string object is created, a reference to which is stored in variant. The lifetime of this object is prolonged to match the lifetime of the reference.
|
I rewrote the best-match function because it bothered me that we have so many flags that we have to check. I also slightly changed the semantics that once we have a match of a certain priority, we can't have another match of the same priority. In practice this means if we have multiple case-insensitive matches, the first one is used. |
* feat(string_util): add contains_nocase * feat(xkeyboard): Enable icon by variant * Cleanup * string_util: add some cases to string test * string_util: rename contains_nocase -> contains_ignore_case * layouticonset: use contains_ignore_case * layouticonset: apply renamings and remove dead code * remove VARIANT_NONE and use empty string instead * use emplace_back and add assert * layouticonset: precompute condition * xkeyboard: restore missing continue * Cleanup parse_icons * Always choose the first case-insensitive match * Cleanup layouticonset.get * update the changelog Co-authored-by: patrick96 <p.ziegler96@gmail.com>
What type of PR is this? (check all applicable)
Description
The new configuration can use variant.
Related Issues & Documents
Closes #2414
Documentation (check all applicable)