Repository navigation
Conversation
e54b016 to
db1f4e7
Compare
igoropaniuk
left a comment
igoropaniuk
left a comment
There was a problem hiding this comment.
Thanks for the PR. I've added some inline comments
Could you document the new key next to the existing layout selector text?
| if (selector) | ||
| ux_err_tag(UX_TAG_FLASHMAP, "layout \"%s\" not found\n", selector); | ||
| else | ||
| ux_err_tag(UX_TAG_FLASHMAP, "no default layout found\n"); |
There was a problem hiding this comment.
could we add a hint for a user for this case, so he knows what to do? Something like multiple layouts found and none is marked default, select one with ::<layout>
| ret = json_get_bool(layout, "default", &default_layout); | ||
| if (!selector && !ret && default_layout) { | ||
| ux_debug_tag(UX_TAG_FLASHMAP, | ||
| "flashmap: selected default layout: %s\n", |
There was a problem hiding this comment.
no need for "flashmap:", UX_TAG_FLASHMAP already prints the FLASHMAP: prefix
| layout = json_get_element_object(layouts, i); | ||
| name = json_get_string(layout, "name"); | ||
| if (name && !strcmp(name, selector)) { | ||
| ret = json_get_bool(layout, "default", &default_layout); |
There was a problem hiding this comment.
If several layouts are marked default, we silently take the first.
That is almost certainly a mistake in the flashmap, and the user would flash a layout they may not expect. I'd rather treat it as an error (or at least add ux_warn_tag) and list the layouts. Rejecting it now is easier than tightening it later
| return NULL; | ||
| } | ||
|
|
||
| int json_get_bool(struct json_value *object, const char *key, bool *value) |
There was a problem hiding this comment.
since json_get_child() already does the key lookup, it can be just:
it = json_get_child(object, key);
if (!it || (it->type != JSON_TYPE_TRUE && it->type != JSON_TYPE_FALSE))
return -1;
I'd also re-work json_get_number() if possible
Use json_get_child() instead of duplicating its lookup loop. Signed-off-by: Hans Månsson <hans.mansson@sony.com>
Add a helper to read boolean values from JSON objects. Signed-off-by: Hans Månsson <hans.mansson@sony.com>
When no selector is given and multiple layouts exist, select the one marked with "default": true. A single layout is still selected automatically. If several layouts are marked default, error out. Signed-off-by: Hans Månsson <hans.mansson@sony.com>
db1f4e7 to
5e0b268
Compare
When no selector is given and multiple layouts exist, select the one
marked with "default": true. The single-layout fallback is retained.
If several layouts are marked default, the first is chosen.