Skip to content

should be able to edit/extrude sketch with no variable declaration #3238

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

Open
jessfraz opened this issue Aug 2, 2024 · 8 comments
Open

should be able to edit/extrude sketch with no variable declaration #3238

jessfraz opened this issue Aug 2, 2024 · 8 comments
Labels
bug Something isn't working cryptic-error Source code produces a confusing error

Comments

@jessfraz
Copy link
Contributor

jessfraz commented Aug 2, 2024

see video

Screenshare.-.2024-08-01.3_45_27.PM.mp4

more info here https://kittycadworkspace.slack.com/archives/C04KFV6NKL0/p1722552201055069

@jessfraz jessfraz added bug Something isn't working high-priority labels Aug 2, 2024
@jtran
Copy link
Contributor

jtran commented Aug 2, 2024

The app is trying to go through this path:

source range selection -> pathToNode -> call AST node -> variable declaration AST node -> variable name -> variable value in ProgramMemory (should be ExtrudeGroup or SketchGroup) -> face or plane

Unfortunately, that's impossible unless the KCL has assigned the pipeline to a variable. If you add let x = to the first line, all problems go away. The assumption that there is a variable is deeply ingrained.

To fix this right, we need @Irev-Dev's changes from #3140 so that we can find the face or plane from an arbitrary UUID that the user has clicked.

In the meantime, I thought we could require KCL authors to always use a variable. A parse error would hopefully guide users to fixing their code.

Is it easy to make an auto-fix that can migrate users' code? That would be ideal.

But also, the variable requirement would go away once the artifact graph was completed.

@jessfraz
Copy link
Contributor Author

jessfraz commented Aug 3, 2024

id rather just disable the button on these? is that possible or we wait for kurts change because changing kcl is not going to work that will break everyone more

@jessfraz
Copy link
Contributor Author

jessfraz commented Aug 3, 2024

@Irev-Dev can you get in your PR by our monday?

@jtran
Copy link
Contributor

jtran commented Aug 3, 2024

It should be possible to disable the button until Kurt's change is in.

@Irev-Dev
Copy link
Contributor

Irev-Dev commented Aug 5, 2024

The change is in!

@Irev-Dev
Copy link
Contributor

Irev-Dev commented Aug 6, 2024

Temp fix to stop user from getting into a bad state is here: #3292
Not closing this issue out yet though.
Quick explanation

Screenshare.-.2024-08-06.3_01_07.PM.mp4

@jessfraz jessfraz changed the title cannot edit sketch/whole ui breaks when placing cursor on circle/invalid sketch for edit should be able to edit/extrude sketch with no variable declaration Aug 25, 2024
@jessfraz jessfraz added this to the v1 Modeling App Launch milestone Sep 11, 2024
@jtran jtran removed their assignment Nov 20, 2024
@Irev-Dev
Copy link
Contributor

Maybe this needs to be a lint that adds a variable when users type this.

@franknoirot
Copy link
Contributor

franknoirot commented Apr 1, 2025

This affects users who:

  1. have hand-written some KCL manually for a sketch, which is not the primary way to interact with the app
  2. have a sketch that cannot be used anywhere else in their program, because it has no identifier assignment
  3. want to edit this nameless, unusable sketch via point-and-click

I think that's a vanishingly small group of users. I'm removing high-priority tag. I agree with @Irev-Dev that this would be helped most of all by a lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cryptic-error Source code produces a confusing error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants