Skip to content

Fix Namespace Bug#123

Open
Lightning11wins wants to merge 2 commits into
masterfrom
fix-namespace-in-cmp
Open

Fix Namespace Bug#123
Lightning11wins wants to merge 2 commits into
masterfrom
fix-namespace-in-cmp

Conversation

@Lightning11wins

Copy link
Copy Markdown
Contributor

This PR fixes #120, so hints widgets applied in components now use the correct namespace context. Thus, this PR closes #120.

@Lightning11wins Lightning11wins self-assigned this Jun 10, 2026
@Lightning11wins Lightning11wins added bug ai-review Request AI review for PRs. size: trivial Easy to review, probably ~100 lines or fewer. labels Jun 10, 2026
@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a namespace bug where hints widgets applied in components were evaluating DefaultExpr under the wrong widget-tree root, causing silent resolution failures in cross-namespace component scenarios.

  • Adds a context parameter to cx_set_hints (falling back to wgtrGetRoot(element)) and stores it as Context on each parsed hints layer; propagates through cx_hints_applyto and cx_copy_hints.
  • Rewrites the DefaultExpr merge in cx_merge_two_hints: when both layers carry a DefaultExpr from different namespaces (h1.Context !== h2.Context), the merge now keeps only h1's expression (consistent with cx_FIRST priority) and drops h2's, preventing cross-namespace eval contamination.
  • Updates cx_hints_setdefault to read _context from the stored Context on the merged hints object rather than always deriving it from wgtrGetRoot(e).

Confidence Score: 5/5

The 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.

Important Files Changed

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

Comment thread centrallix-os/sys/js/ht_utils_hints.js Outdated
@Lightning11wins

Copy link
Copy Markdown
Contributor Author

This PR is ready for human review.

@Lightning11wins Lightning11wins requested a review from gbeeley June 10, 2026 18:05

@gbeeley gbeeley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic change needed for default expressions - thanks!

if (h1.DefaultExpr && h2.DefaultExpr && h1.Context !== h2.Context)
{
nh.DefaultExpr = h1.DefaultExpr;
nh.Context = h1.Context;

@gbeeley gbeeley Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Lightning11wins Lightning11wins Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request AI review for PRs. bug size: trivial Easy to review, probably ~100 lines or fewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hints Widgets Applied to component Widgets Use Incorrect Namespace

2 participants