Skip to content
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

Return KclValue for operations' arguments, and artifactId or KclValue for operation itself #5187

Open
franknoirot opened this issue Jan 30, 2025 · 3 comments · May be fixed by #5206
Open

Return KclValue for operations' arguments, and artifactId or KclValue for operation itself #5187

franknoirot opened this issue Jan 30, 2025 · 3 comments · May be fixed by #5206
Assignees
Labels
dev Issues related to development of the app.

Comments

@franknoirot
Copy link
Collaborator

franknoirot commented Jan 30, 2025

Blocked by #5186.

Once our KclValue type can reliably be used to get appropriate artifact IDs from, replace the current behavior of operation generation to include KclValues for an operation's arguments instead of their source ranges (or in addition too them?).

In a similar vein, return the KclValue for the overall operation, or the artifact ID associated with the operation as well, depending on which is easiest. This will enable the feature tree UI to select the appropriate artifact on selection click, without needing to derive the appropriate artifact from source ranges.

@franknoirot franknoirot added the dev Issues related to development of the app. label Jan 30, 2025
@jtran
Copy link
Collaborator

jtran commented Jan 30, 2025

Did we ever decide whether we want the user to edit the expression or the computed value?

Say the argument is x + 2. When building the operations, we know that the computed value is 42. The intent of this issue was that we would capture 42 and include that in the operation.

However, if we do that and use it to edit in the command bar, and the user changes it to 43, we'd be changing the meaning. It would no longer depend on x.

The expression x + 2 is just an example. It could be an arbitrary expression like a function call. This also applies to objects in the scene also, like getOppositeEdge(seg001).

On the other hand, if we decide they're editing the expression, then you'll want to use the KCL source. That you can get from the source range.

@franknoirot
Copy link
Collaborator Author

Oh yeah for sure the expression in the case of arguments that can be represented as KCL values in the command palette. So the source range is perfectly fine for those cases.

@jtran jtran linked a pull request Jan 31, 2025 that will close this issue
@jtran
Copy link
Collaborator

jtran commented Jan 31, 2025

Clarification: We want the value when it's primitive. But for larger values in the scene, we don't care about most of the KclValue struct. The only thing we care about is its artifact ID.

@pierremtb pierremtb added this to the v1 Modeling App Launch milestone Feb 3, 2025
@jtran jtran self-assigned this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Issues related to development of the app.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants