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

Make sure stable artifact IDs are in all variants KclValue type #5186

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

Make sure stable artifact IDs are in all variants KclValue type #5186

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

Comments

@franknoirot
Copy link
Collaborator

#4442 requires more information from operations than just source ranges. After a discussion with @jtran, we decided that passing through the real KclValue for each of an operation's arguments should give us enough information to populate the command palette for most cases of edit flows for operations generically, thereby solving our "missing representation" problem for the time being.

The issue right now is that our KclValue has some instances where the original artifact ID for an operation is overwritten within the Rust code. @jtran and I couldn't figure out why this was the case, and had to guess that this is to uphold something about the inheritance of IDs in the engine from plane to sketch to sweep ("sweep" in the generic sense). This area of code is worth revisiting elsewhere, but this issue just asks that instead of overwriting the original artifact ID, that we shift it over to some new key on those KclValues that currently have code that mutates it. That way frontend code (like the feature tree's operation selection and edit code) can derive artifact IDs from the KclValues.

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.

2 participants