Don't equip Line tool if sketch has a circle

* dont jump to lints

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* updates

Signed-off-by: Jess Frazelle <github@jessfraz.com>

* fix

Signed-off-by: Jess Frazelle <github@jessfraz.com>

---------

Signed-off-by: Jess Frazelle <github@jessfraz.com>

e2e/playwright/flow-tests.spec.ts

@@ -1020,7 +1020,7 @@ test.describe('Editor tests', () => {
   |> line([0, -10], %, $revolveAxis)
   |> close(%)
   |> extrude(10, %)
-  
+
   const sketch001 = startSketchOn(box, revolveAxis)
   |> startProfileAt([5, 10], %)
   |> line([0, -10], %)
@@ -3894,6 +3894,39 @@ const extrude001 = extrude(distance001, sketch001)`.replace(
 
 test.describe('Regression tests', () => {
   // bugs we found that don't fit neatly into other categories
+  test('bad model has inline error #3251', async ({ page }) => {
+    // because the model has `line([0,0]..` it is valid code, but the model is invalid
+    // regression test for https://github.com/KittyCAD/modeling-app/issues/3251
+    // Since the bad model also found as issue with the artifact graph, which in tern blocked the editor diognostics
+    const u = await getUtils(page)
+    await page.addInitScript(async () => {
+      localStorage.setItem(
+        'persistCode',
+        `const sketch2 = startSketchOn("XY")
+const sketch001 = startSketchAt([-0, -0])
+  |> line([0, 0], %)
+  |> line([-4.84, -5.29], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)`
+      )
+    })
+
+    await page.setViewportSize({ width: 1000, height: 500 })
+
+    await u.waitForAuthSkipAppStart()
+
+    // error in guter
+    await expect(page.locator('.cm-lint-marker-error')).toBeVisible()
+
+    // error text on hover
+    await page.hover('.cm-lint-marker-error')
+    // this is a cryptic error message, fact that all the lines are co-linear from the `line([0,0])` is the issue why
+    // the close doesn't work
+    // when https://github.com/KittyCAD/modeling-app/issues/3268 is closed
+    // this test will need updating
+    const crypticErrorText = `ApiError`
+    await expect(page.getByText(crypticErrorText).first()).toBeVisible()
+  })
   test('executes on load', async ({ page }) => {
     const u = await getUtils(page)
     await page.addInitScript(async () => {
@@ -8333,6 +8366,83 @@ test.describe('Code pane and errors', () => {
     await badge.click()
 
     // Ensure we have an error diagnostic.
-    await expect(page.locator('.cm-lint-marker-error')).toBeVisible()
+    await expect(page.locator('.cm-lint-marker-error').first()).toBeVisible()
+
+    // Hover over the error to see the error message
+    await page.hover('.cm-lint-marker-error')
+    await expect(
+      page
+        .getByText(
+          'sketch profile must lie entirely on one side of the revolution axis'
+        )
+        .first()
+    ).toBeVisible()
+  })
+
+  test('When error is not in view WITH LINTS you can click the badge to scroll to it', async ({
+    page,
+  }) => {
+    const u = await getUtils(page)
+
+    // Load the app with the working starter code
+    await page.addInitScript((code) => {
+      localStorage.setItem('persistCode', code)
+    }, TEST_CODE_LONG_WITH_ERROR_OUT_OF_VIEW)
+
+    await page.setViewportSize({ width: 1200, height: 500 })
+    await u.waitForAuthSkipAppStart()
+
+    await page.waitForTimeout(1000)
+
+    // Ensure badge is present
+    const codePaneButtonHolder = page.locator('#code-button-holder')
+    await expect(codePaneButtonHolder).toContainText('notification')
+
+    // Ensure we have no errors in the gutter, since error out of view.
+    await expect(page.locator('.cm-lint-marker-error')).not.toBeVisible()
+
+    // click in the editor to focus it
+    await page.locator('.cm-content').click()
+
+    await page.waitForTimeout(500)
+
+    // go to the start of the editor and enter more text which will trigger
+    // a lint error.
+    // GO to the start of the editor.
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('ArrowUp')
+    await page.keyboard.press('Home')
+    await page.keyboard.type('const foo_bar = 1')
+    await page.waitForTimeout(500)
+    await page.keyboard.press('Enter')
+
+    // ensure we have a lint error
+    await expect(page.locator('.cm-lint-marker-info').first()).toBeVisible()
+
+    // Click the badge.
+    const badge = page.locator('#code-badge')
+    await expect(badge).toBeVisible()
+    await badge.click()
+
+    // Ensure we have an error diagnostic.
+    await expect(page.locator('.cm-lint-marker-error').first()).toBeVisible()
+
+    // Hover over the error to see the error message
+    await page.hover('.cm-lint-marker-error')
+    await expect(
+      page
+        .getByText(
+          'sketch profile must lie entirely on one side of the revolution axis'
+        )
+        .first()
+    ).toBeVisible()
   })
 })

e2e/playwright/storageStates.ts

@@ -337,7 +337,24 @@ fn svg = (surface, origin, depth) => {
     |> close(%)
     |> extrude(depth, %)
 
-  "thing";kajsnd;akjsnd
+const box = startSketchOn('XY')
+  |> startProfileAt([0, 0], %)
+  |> line([0, 10], %)
+  |> line([10, 0], %)
+  |> line([0, -10], %, $revolveAxis)
+  |> close(%)
+  |> extrude(10, %)
+
+  const sketch001 = startSketchOn(box, revolveAxis)
+  |> startProfileAt([5, 10], %)
+  |> line([0, -10], %)
+  |> line([2, 0], %)
+  |> line([0, -10], %)
+  |> close(%)
+  |> revolve({
+  axis: revolveAxis,
+  angle: 90
+  }, %)
   return 0
 }
 

src/clientSideScene/sceneEntities.ts

@@ -98,6 +98,7 @@ import {
 import { getThemeColorForThreeJs } from 'lib/theme'
 import { err, trap } from 'lib/trap'
 import { CSS2DObject } from 'three/examples/jsm/renderers/CSS2DRenderer'
+import { addCircleToSketchAst } from 'lib/circleTool'
 
 type DraftSegment = 'line' | 'tangentialArcTo'
 
@@ -714,11 +715,8 @@ export class SceneEntities {
     })
   }
   setupDraftRectangle = async (
-    sketchPathToNode: PathToNode,
-    forward: [number, number, number],
-    up: [number, number, number],
-    sketchOrigin: [number, number, number],
-    rectangleOrigin: [x: number, y: number]
+    rectangleOrigin: [number, number],
+    { sketchPathToNode, origin, zAxis, yAxis }: SketchDetails
   ) => {
     let _ast = structuredClone(kclManager.ast)
 
@@ -750,9 +748,9 @@ export class SceneEntities {
 
     const { programMemoryOverride, truncatedAst } = await this.setupSketch({
       sketchPathToNode,
-      forward,
-      up,
-      position: sketchOrigin,
+      forward: yAxis,
+      up: zAxis,
+      position: origin,
       maybeModdedAst: _ast,
       draftExpressionsIndices: { start: 0, end: 3 },
     })
@@ -862,6 +860,52 @@ export class SceneEntities {
       },
     })
   }
+  setupCircleOriginListener = () => {
+    sceneInfra.setCallbacks({
+      onClick: (args) => {
+        const twoD = args.intersectionPoint?.twoD
+        if (!twoD) {
+          console.warn(`This click didn't have a 2D intersection`, args)
+          return
+        }
+        sceneInfra.modelingSend({
+          type: 'Add circle origin',
+          data: [twoD.x, twoD.y],
+        })
+      },
+    })
+  }
+  setupDraftCircle = async (
+    circleOrigin: [number, number],
+    { sketchPathToNode, origin, zAxis, yAxis }: SketchDetails
+  ) => {
+    const astWithCircle = await addCircleToSketchAst({
+      sourceAst: kclManager.ast,
+      sketchPathToNode,
+      circleOrigin,
+    })
+
+    const { programMemoryOverride, truncatedAst } = await this.setupSketch({
+      sketchPathToNode,
+      forward: yAxis,
+      up: zAxis,
+      position: origin,
+      maybeModdedAst: astWithCircle,
+      draftExpressionsIndices: { start: 0, end: 3 },
+    })
+
+    // TODO: Move this into the onclick handler to get the real shit
+    // Update the primary AST and unequip the rectangle tool
+    await kclManager.executeAstMock(astWithCircle)
+    sceneInfra.modelingSend({ type: 'CancelSketch' })
+
+    const { programMemory } = await executeAst({
+      ast: astWithCircle,
+      useFakeExecutor: true,
+      engineCommandManager: this.engineCommandManager,
+      programMemoryOverride,
+    })
+  }
   setupSketchIdleCallbacks = ({
     pathToNode,
     up,
@@ -1176,6 +1220,9 @@ export class SceneEntities {
         ? orthoFactor
         : perspScale(sceneInfra.camControls.camera, group)) /
       sceneInfra._baseUnitMultiplier
+
+    console.log('segment type', type)
+
     if (type === TANGENTIAL_ARC_TO_SEGMENT) {
       return this.updateTangentialArcToSegment({
         prevSegment: sgPaths[index - 1],

src/components/ModelingSidebar/ModelingPanes/KclEditorPane.tsx

@@ -22,7 +22,7 @@ import {
   historyKeymap,
   history,
 } from '@codemirror/commands'
-import { lintGutter, lintKeymap } from '@codemirror/lint'
+import { diagnosticCount, lintGutter, lintKeymap } from '@codemirror/lint'
 import {
   foldGutter,
   foldKeymap,
@@ -196,7 +196,10 @@ export const KclEditorPane = () => {
 
           // On first load of this component, ensure we show the current errors
           // in the editor.
-          kclManager.setDiagnosticsForCurrentErrors()
+          // Make sure we don't add them twice.
+          if (diagnosticCount(_editorView.state) === 0) {
+            kclManager.setDiagnosticsForCurrentErrors()
+          }
         }}
       />
     </div>

src/components/ModelingSidebar/ModelingPanes/index.ts

@@ -63,7 +63,7 @@ export const sidebarPanes: SidebarPane[] = [
       },
       onClick: (e) => {
         e.preventDefault()
-        editorManager.scrollToFirstDiagnosticIfExists()
+        editorManager.scrollToFirstErrorDiagnosticIfExists()
       },
     },
   },

src/editor/manager.ts

@@ -141,14 +141,14 @@ export default class EditorManager {
     })
   }
 
-  scrollToFirstDiagnosticIfExists() {
+  scrollToFirstErrorDiagnosticIfExists() {
     if (!this._editorView) return
 
     let firstDiagnosticPos: [number, number] | null = null
     forEachDiagnostic(
       this._editorView.state,
       (d: Diagnostic, from: number, to: number) => {
-        if (!firstDiagnosticPos) {
+        if (!firstDiagnosticPos && d.severity === 'error') {
           firstDiagnosticPos = [from, to]
         }
       }
@@ -161,7 +161,11 @@ export default class EditorManager {
       selection: EditorSelection.create([
         EditorSelection.cursor(firstDiagnosticPos[0]),
       ]),
-      effects: [EditorView.scrollIntoView(firstDiagnosticPos[0])],
+      effects: [
+        EditorView.scrollIntoView(
+          EditorSelection.range(firstDiagnosticPos[0], firstDiagnosticPos[1])
+        ),
+      ],
       annotations: [
         updateOutsideEditorEvent,
         Transaction.addToHistory.of(false),

src/lang/std/artifactGraph.ts

@@ -240,6 +240,7 @@ export function getArtifactsToUpdate({
   const response = responseMap[id]
   const cmd = command.cmd
   const returnArr: ReturnType<typeof getArtifactsToUpdate> = []
+  if (!response) return returnArr
   if (cmd.type === 'enable_sketch_mode') {
     const plane = getArtifact(currentPlaneId)
     const pathIds = plane?.type === 'plane' ? plane?.pathIds : []
@@ -316,7 +317,7 @@ export function getArtifactsToUpdate({
         artifact: { ...path, segIds: [id] },
       })
     if (
-      response.type === 'modeling' &&
+      response?.type === 'modeling' &&
       response.data.modeling_response.type === 'close_path'
     ) {
       returnArr.push({

src/lang/std/engineConnection.ts

@@ -2,7 +2,7 @@ import { Program, SourceRange } from 'lang/wasm'
 import { VITE_KC_API_WS_MODELING_URL } from 'env'
 import { Models } from '@kittycad/lib'
 import { exportSave } from 'lib/exportSave'
-import { deferExecution, uuidv4 } from 'lib/utils'
+import { deferExecution, isOverlap, uuidv4 } from 'lib/utils'
 import { Themes, getThemeColorForEngine, getOppositeTheme } from 'lib/theme'
 import { DefaultPlanes } from 'wasm-lib/kcl/bindings/DefaultPlanes'
 import {
@@ -1899,15 +1899,10 @@ export class EngineCommandManager extends EventTarget {
     range: SourceRange,
     commandTypeToTarget: string
   ): string | undefined {
-    const values = Object.entries(this.artifactGraph)
-    for (const [id, data] of values) {
-      // // Our range selection seems to just select the cursor position, so either
-      // // of these can be right...
-      if (
-        (data.range[0] === range[0] || data.range[1] === range[1]) &&
-        data.type === commandTypeToTarget
-      )
-        return id
+    for (const [artifactId, artifact] of this.artifactGraph) {
+      if ('codeRef' in artifact && isOverlap(range, artifact.codeRef.range)) {
+        if (commandTypeToTarget === artifact.type) return artifactId
+      }
     }
     return undefined
   }

src/lib/circleTool.ts

@@ -0,0 +1,92 @@
+import {
+  createArrayExpression,
+  createCallExpressionStdLib,
+  createLiteral,
+  createPipeExpression,
+  createPipeSubstitution,
+  createTagDeclarator,
+  findUniqueName,
+} from 'lang/modifyAst'
+import { roundOff } from './utils'
+import {
+  PathToNode,
+  Program,
+  VariableDeclaration,
+  parse,
+  recast,
+} from 'lang/wasm'
+import { getNodeFromPath } from 'lang/queryAst'
+import { trap } from './trap'
+
+/**
+ * Hide away the working with the AST
+ */
+export async function addCircleToSketchAst({
+  sourceAst,
+  sketchPathToNode,
+  circleOrigin,
+}: {
+  sourceAst: Program
+  sketchPathToNode?: PathToNode
+  circleOrigin?: [number, number]
+}) {
+  let _ast = JSON.parse(JSON.stringify(sourceAst))
+
+  const _node1 = getNodeFromPath<VariableDeclaration>(
+    _ast,
+    sketchPathToNode || [],
+    'VariableDeclaration'
+  )
+  if (trap(_node1)) return Promise.reject(_node1)
+
+  const tag = findUniqueName(_ast, 'circle')
+
+  const _node2 = getNodeFromPath<VariableDeclaration>(
+    _ast,
+    sketchPathToNode || [],
+    'VariableDeclaration'
+  )
+  if (trap(_node2)) return Promise.reject(_node2)
+  const startSketchOn = _node2.node?.declarations
+
+  const startSketchOnInit = startSketchOn?.[0]?.init
+  startSketchOn[0].init = createPipeExpression([
+    startSketchOnInit,
+    ...getCircleCallExpressions({
+      center: circleOrigin,
+      tag,
+    }),
+  ])
+
+  const maybeModdedAst = parse(recast(_ast))
+  if (trap(maybeModdedAst)) return Promise.reject(maybeModdedAst)
+  return Promise.resolve(maybeModdedAst)
+}
+
+/**
+ * Returns AST expressions for this KCL code:
+ * const yo = startSketchOn('XY')
+ *   |> startProfileAt([0, 0], %)
+ *   |> circle([0, 0], 0, %) <- this line
+ */
+export function getCircleCallExpressions({
+  center = [0, 0],
+  radius = 10,
+  tag,
+}: {
+  center?: [number, number]
+  radius?: number
+  tag: string
+}) {
+  return [
+    createCallExpressionStdLib('circle', [
+      createArrayExpression([
+        createLiteral(roundOff(center[0])),
+        createLiteral(roundOff(center[1])),
+      ]),
+      createLiteral(roundOff(radius)),
+      createPipeSubstitution(),
+      createTagDeclarator(tag),
+    ]),
+  ]
+}

src/lib/singletons.ts

@@ -4,6 +4,7 @@ import EditorManager from 'editor/manager'
 import { KclManager } from 'lang/KclSingleton'
 import CodeManager from 'lang/codeManager'
 import { EngineCommandManager } from 'lang/std/engineConnection'
+import { uuidv4 } from './utils'
 
 export const codeManager = new CodeManager()
 
@@ -40,4 +41,14 @@ if (typeof window !== 'undefined') {
   ;(window as any).enableFillet = () => {
     ;(window as any)._enableFillet = true
   }
+  ;(window as any).zoomToFit = () =>
+    engineCommandManager.sendSceneCommand({
+      type: 'modeling_cmd_req',
+      cmd_id: uuidv4(),
+      cmd: {
+        type: 'zoom_to_fit',
+        object_ids: [], // leave empty to zoom to all objects
+        padding: 0.2, // padding around the objects
+      },
+    })
 }

src/lib/toolbar.ts

@@ -293,7 +293,8 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
         status: 'available',
         disabled: (state) =>
           state.matches('Sketch no face') ||
-          state.matches('Sketch.Rectangle tool.Awaiting second corner'),
+          state.matches('Sketch.Rectangle tool.Awaiting second corner') ||
+          state.matches('Sketch.Circle tool.Awaiting perimeter click'),
         title: 'Line',
         hotkey: (state) =>
           state.matches('Sketch.Line tool') ? ['Esc', 'L'] : 'L',
@@ -355,9 +356,20 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
       [
         {
           id: 'circle-center',
-          onClick: () => console.error('Center circle not yet implemented'),
+          onClick: ({ modelingStateMatches, modelingSend }) =>
+            modelingSend({
+              type: 'change tool',
+              data: {
+                tool: !modelingStateMatches('Sketch.Circle tool')
+                  ? 'circle'
+                  : 'none',
+              },
+            }),
           icon: 'circle',
-          status: 'unavailable',
+          status: 'available',
+          disabled: (state) =>
+            !canRectangleTool(state.context) &&
+            !state.matches('Sketch.Circle tool'),
           title: 'Center circle',
           showTitle: false,
           description: 'Start drawing a circle from its center',
@@ -367,6 +379,9 @@ export const toolbarConfig: Record<ToolbarModeName, ToolbarMode> = {
               url: 'https://github.com/KittyCAD/modeling-app/issues/1501',
             },
           ],
+          hotkey: (state) =>
+            state.matches('Sketch.Circle tool') ? ['Esc', 'C'] : 'C',
+          isActive: (state) => state.matches('Sketch.Circle tool'),
         },
         {
           id: 'circle-three-points',

src/machines/modelingMachine.ts

@@ -141,7 +141,12 @@ interface Store {
   openPanes: SidebarType[]
 }
 
-export type SketchTool = 'line' | 'tangentialArc' | 'rectangle' | 'none'
+export type SketchTool =
+  | 'line'
+  | 'tangentialArc'
+  | 'rectangle'
+  | 'circle'
+  | 'none'
 
 export type ModelingMachineEvent =
   | {
@@ -209,6 +214,10 @@ export type ModelingMachineEvent =
       type: 'Add rectangle origin'
       data: [x: number, y: number]
     }
+  | {
+      type: 'Add circle origin'
+      data: [x: number, y: number]
+    }
   | {
       type: 'done.invoke.animate-to-face' | 'done.invoke.animate-to-sketch'
       data: SketchDetails
@@ -591,6 +600,8 @@ export const modelingMachine = createMachine(
                   Cancel: '#Modeling.Sketch.undo startSketchOn',
                 },
               },
+
+              'new state 1': {},
             },
 
             initial: 'Init',
@@ -796,8 +807,31 @@ export const modelingMachine = createMachine(
                 target: 'Tangential arc to',
                 cond: 'next is tangential arc',
               },
+              {
+                target: 'Circle tool',
+                cond: 'next is circle',
+              },
             ],
           },
+
+          'Circle tool': {
+            entry: 'listen for circle origin',
+
+            states: {
+              'Awaiting origin': {
+                on: {
+                  'Add circle origin': {
+                    target: 'Awaiting perimeter click',
+                    actions: 'set up draft circle',
+                  },
+                },
+              },
+
+              'Awaiting perimeter click': {},
+            },
+
+            initial: 'Awaiting origin',
+          },
         },
 
         initial: 'Init',
@@ -1041,6 +1075,14 @@ export const modelingMachine = createMachine(
         if ((state?.event as any).data.tool !== 'rectangle') return false
         return canRectangleTool({ sketchDetails })
       },
+      'next is circle': ({ sketchDetails }, _, { state }) => {
+        if ((state?.event as any).data.tool !== 'circle') return false
+        // TODO: Both this an rectangle can currently only be used
+        // if the sketch is empty. They share limited implementations,
+        // so I'm using the same cond until we have
+        // multi-profile sketch support in.
+        return canRectangleTool({ sketchDetails })
+      },
       'next is line': (_, __, { state }) =>
         (state?.event as any).data.tool === 'line',
       'next is none': (_, __, { state }) =>
@@ -1101,11 +1143,13 @@ export const modelingMachine = createMachine(
         store.videoElement?.pause()
         const updatedAst = await kclManager.updateAst(modifiedAst, true, {
           focusPath: pathToExtrudeArg,
-          zoomToFit: true,
-          zoomOnRangeAndType: {
-            range: selection.codeBasedSelections[0].range,
-            type: 'start_path',
-          },
+          // commented out as a part of https://github.com/KittyCAD/modeling-app/issues/3270
+          // looking to add back in the future
+          // zoomToFit: true,
+          // zoomOnRangeAndType: {
+          //   range: selection.codeBasedSelections[0].range,
+          //   type: 'path',
+          // },
         })
         if (!engineCommandManager.engineConnection?.idleMode) {
           store.videoElement?.play().catch((e) => {
@@ -1279,13 +1323,18 @@ export const modelingMachine = createMachine(
       },
       'set up draft rectangle': ({ sketchDetails }, { data }) => {
         if (!sketchDetails || !data) return
-        sceneEntitiesManager.setupDraftRectangle(
-          sketchDetails.sketchPathToNode,
-          sketchDetails.zAxis,
-          sketchDetails.yAxis,
-          sketchDetails.origin,
-          data
-        )
+
+        sceneEntitiesManager.setupDraftRectangle(data, sketchDetails)
+      },
+      'listen for circle origin': ({ sketchDetails }) => {
+        if (!sketchDetails) return
+        sceneEntitiesManager.setupCircleOriginListener()
+      },
+      'set up draft circle': ({ sketchDetails }, { data }) => {
+        if (!sketchDetails || !data) return
+        console.log('setting up draft circle', data)
+
+        sceneEntitiesManager.setupDraftCircle(data, sketchDetails)
       },
       'set up draft line without teardown': ({ sketchDetails }) => {
         if (!sketchDetails) return
@@ -1656,7 +1705,10 @@ export function isEditingExistingSketch({
     (item) =>
       item.type === 'CallExpression' && item.callee.name === 'startProfileAt'
   )
-  return hasStartProfileAt && pipeExpression.body.length > 2
+  const hasCircle = pipeExpression.body.some(
+    (item) => item.type === 'CallExpression' && item.callee.name === 'circle'
+  )
+  return (hasStartProfileAt && pipeExpression.body.length > 2) || hasCircle
 }
 
 export function canRectangleTool({