Fix Namespace Bug#123
Conversation
Greptile SummaryThis PR fixes a namespace bug where hints widgets applied in components were evaluating
Confidence Score: 5/5The change is narrowly scoped to namespace context tracking for DefaultExpr evaluation; the logic is correct and handles all hint-layer combinations. Context is correctly initialised in cx_set_hints, propagated through cx_copy_hints and the applyto chain, merged safely in cx_merge_two_hints, and read back in cx_hints_setdefault. All hint-layer combinations are handled correctly and no pre-existing interfaces are broken. No files require special attention.
|
| Filename | Overview |
|---|---|
| centrallix-os/sys/js/ht_utils_hints.js | Adds namespace context tracking to hints expressions; logic in cx_merge_two_hints, cx_set_hints, cx_copy_hints, and cx_hints_setdefault is correct and handles all edge cases (null layers, single-layer, same-context merge, cross-context conflict). |
Reviews (2): Last reviewed commit: "Fix unsafe hint merge behavior (noticed ..." | Re-trigger Greptile
|
This PR is ready for human review. |
gbeeley
left a comment
There was a problem hiding this comment.
logic change needed for default expressions - thanks!
| if (h1.DefaultExpr && h2.DefaultExpr && h1.Context !== h2.Context) | ||
| { | ||
| nh.DefaultExpr = h1.DefaultExpr; | ||
| nh.Context = h1.Context; |
There was a problem hiding this comment.
This logic doesn't account for the situation where both hint sources have a valid DefaultExpr, but h1's expression returns null while h2's expression does not. In that case the value would be set to null and h2's expression ignored completely. We need some mechanism here to merge two hint expressions with different scopes and keep the scopes/contexts intact for both.
Incidentally this situation applies also to other expression based hints. We probably need a construct like cx_eval_in_scope(scope, expression) that can be placed within expression strings.
There was a problem hiding this comment.
I just barely understand how this code works at all, so I'm not sure how to design that in a way that works well with the rest of this system. We might need to have a design discussion.
This PR fixes #120, so hints widgets applied in components now use the correct namespace context. Thus, this PR closes #120.