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

Group All Fillet Tags Under a Single Fillet Expression for Multiple Selections #4039

Closed
Tracked by #2606
max-mrgrsk opened this issue Sep 30, 2024 · 0 comments · Fixed by #4058
Closed
Tracked by #2606

Group All Fillet Tags Under a Single Fillet Expression for Multiple Selections #4039

max-mrgrsk opened this issue Sep 30, 2024 · 0 comments · Fixed by #4058
Assignees
Labels
ast Issues / features relevant to ast and parser. desktop-app Issues from the desktop app.

Comments

@max-mrgrsk
Copy link
Collaborator

max-mrgrsk commented Sep 30, 2024

this issue is part of Fillet project: #2606
continues AST mod: #3680

Issue:

When selecting multiple edges and applying a fillet in one operation, the AST mod currently creates a new fillet expression for each selected edge. This results in multiple fillet expressions instead of grouping all fillets under a single expression.

const sketch001 = startSketchOn('XY')
  |> startProfileAt([-20, -5], %)
  |> line([0, 10], %, $seg01)
  |> line([10, 0], %)
  |> line([0, -10], %, $seg02)
  |> lineTo([profileStartX(%), profileStartY(%)], %)
  |> close(%)

const extrude001 = extrude(-10, sketch001)
  |> fillet({ radius: 5, tags: [seg01] }, %)
  |> fillet({ radius: 5, tags: [seg02] }, %)

Background:

Originally, the fillet AST mod was designed to handle a single edge. In a later iteration, the functionality was extended to support multiple selections by looping through the same function for each edge. This approach was simple and made it easy to extend the code from handling single to multiple selections. Additionally, it worked well for cases where edges in different bodies were selected.

Improvement Proposal:

To improve both efficiency and code clarity, all fillet tags should be grouped under the same fillet expression when multiple edges are selected and filleted in a single operation. This will streamline the operation and result in cleaner, more maintainable code.

const sketch001 = startSketchOn('XY')
  |> startProfileAt([-20, -5], %)
  |> line([0, 10], %, $seg01)
  |> line([10, 0], %)
  |> line([0, -10], %, $seg02)
  |> lineTo([profileStartX(%), profileStartY(%)], %)
  |> close(%)

const extrude001 = extrude(-10, sketch001)
  |> fillet({ radius: 5, tags: [seg01, seg02] }, %)

related comments:

@max-mrgrsk max-mrgrsk mentioned this issue Sep 30, 2024
37 tasks
@max-mrgrsk max-mrgrsk self-assigned this Sep 30, 2024
@max-mrgrsk max-mrgrsk added ast Issues / features relevant to ast and parser. desktop-app Issues from the desktop app. labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ast Issues / features relevant to ast and parser. desktop-app Issues from the desktop app.
Projects
None yet
1 participant