Skip to content

Commit f0866b4

Browse files
Unrevert #10305 with more stringent file watching guard (#10344)
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
1 parent 95f51d8 commit f0866b4

7 files changed

Lines changed: 142 additions & 74 deletions

File tree

e2e/playwright/debug-pane.spec.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
5252
await page.keyboard.press('ArrowDown')
5353
}
5454
})
55+
let lastSegmentText = await segment.innerText()
5556
// TODO: if you type all the code at once without delay (or paste it in)
5657
// the initial segment artifact ID is different. This appears to be niche bug
5758
// that is being sidestepped in this test until https://github.com/KittyCAD/modeling-app/issues/9609 is addressed.
@@ -60,20 +61,23 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
6061
// Wait for keyboard input debounce and updated artifact graph.
6162
await page.waitForTimeout(1000)
6263
})
63-
// Extract the artifact IDs from the debug artifact graph.
64-
const initialSegmentIds = await segment.innerText({ timeout: 5_000 })
64+
await expect(segment).not.toHaveText(lastSegmentText)
6565
// The artifact ID should include a UUID.
66-
expect(initialSegmentIds).toMatch(
66+
const uuidRegexp =
6767
/[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}/
68-
)
69-
await test.step('Enter a comment', async () => {
68+
await expect(segment).toHaveText(uuidRegexp)
69+
const uuid = (await segment.innerText()).match(uuidRegexp)!
70+
71+
await test.step('Enter another line', async () => {
72+
lastSegmentText = await segment.innerText()
7073
await page.keyboard.type('\n|> line(end = [2, 2])', { delay: 10 })
7174
// Wait for keyboard input debounce and updated artifact graph.
7275
await page.waitForTimeout(1000)
7376
})
74-
const newSegmentIds = await segment.innerText()
75-
// Strip off the closing bracket.
76-
const initialIds = initialSegmentIds.slice(0, initialSegmentIds.length - 1)
77-
expect(newSegmentIds.slice(0, initialIds.length)).toEqual(initialIds)
77+
78+
// Expect the artifact IDs to be changed (by adding another),
79+
await expect(segment).not.toHaveText(lastSegmentText)
80+
// but still contain the stable first ID.
81+
await expect(segment).toContainText(uuid)
7882
})
7983
})

src/components/CommandBar/CommandBarSelectionMixedInput.spec.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import CommandBarSelectionMixedInput from '@src/components/CommandBar/CommandBar
55
import type { CommandArgument } from '@src/lib/commandTypes'
66
import { App } from '@src/lib/app'
77
import { KclManager } from '@src/lang/KclManager'
8+
import { signal } from '@preact/signals-core'
89

910
vi.mock(`@rust/kcl-wasm-lib/pkg/kcl_wasm_lib`)
1011
vi.mock('@src/lang/wasmUtils', async () => {
@@ -55,10 +56,11 @@ describe('CommandBarSelectionMixedInput', () => {
5556
describe('clearSelectionFirst behavior', () => {
5657
it('should send clear selection command when clearSelectionFirst is true', async () => {
5758
const app = App.getDefaultSystems()
58-
const executingEditor = new KclManager({
59+
const executingEditor = new KclManager('some-path', {
5960
commandBar: app.commands.actor,
6061
settings: app.settings.actor,
6162
wasmInstancePromise: app.wasmPromise,
63+
projectPath: signal('some-project'),
6264
})
6365
const mockModelingSend = vi.spyOn(
6466
executingEditor.engineCommandManager,
@@ -85,10 +87,11 @@ describe('CommandBarSelectionMixedInput', () => {
8587

8688
it('should NOT send clear selection command when clearSelectionFirst is false', async () => {
8789
const app = App.getDefaultSystems()
88-
const executingEditor = new KclManager({
90+
const executingEditor = new KclManager('some-path', {
8991
commandBar: app.commands.actor,
9092
settings: app.settings.actor,
9193
wasmInstancePromise: app.wasmPromise,
94+
projectPath: signal('some-project'),
9295
})
9396
const mockModelingSend = vi.spyOn(
9497
executingEditor.engineCommandManager,
@@ -112,10 +115,11 @@ describe('CommandBarSelectionMixedInput', () => {
112115

113116
it('should NOT send clear selection command when clearSelectionFirst is undefined', async () => {
114117
const app = App.getDefaultSystems()
115-
const executingEditor = new KclManager({
118+
const executingEditor = new KclManager('some-path', {
116119
commandBar: app.commands.actor,
117120
settings: app.settings.actor,
118121
wasmInstancePromise: app.wasmPromise,
122+
projectPath: signal('some-project'),
119123
})
120124
const mockModelingSend = vi.spyOn(
121125
executingEditor.engineCommandManager,
@@ -139,10 +143,11 @@ describe('CommandBarSelectionMixedInput', () => {
139143

140144
it('should send clear selection command only once on mount', async () => {
141145
const app = App.getDefaultSystems()
142-
const executingEditor = new KclManager({
146+
const executingEditor = new KclManager('some-path', {
143147
commandBar: app.commands.actor,
144148
settings: app.settings.actor,
145149
wasmInstancePromise: app.wasmPromise,
150+
projectPath: signal('some-project'),
146151
})
147152
const mockModelingSend = vi.spyOn(
148153
executingEditor.engineCommandManager,
@@ -181,10 +186,11 @@ describe('CommandBarSelectionMixedInput', () => {
181186

182187
it('should set hasClearedSelection state after clearing', async () => {
183188
const app = App.getDefaultSystems()
184-
const executingEditor = new KclManager({
189+
const executingEditor = new KclManager('some-path', {
185190
commandBar: app.commands.actor,
186191
settings: app.settings.actor,
187192
wasmInstancePromise: app.wasmPromise,
193+
projectPath: signal('some-project'),
188194
})
189195
const mockModelingSend = vi.spyOn(
190196
executingEditor.engineCommandManager,

src/components/RouteProvider.tsx

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import { isCodeTheSame } from '@src/lib/codeEditor'
21
import fsZds from '@src/lib/fs-zds'
32
import type { ReactNode } from 'react'
43
import { createContext, useEffect, useState } from 'react'
54
import { useLocation, useNavigate, useNavigation } from 'react-router-dom'
6-
import toast from 'react-hot-toast'
7-
85
import { useAuthNavigation } from '@src/hooks/useAuthNavigation'
96
import { useFileSystemWatcher } from '@src/hooks/useFileSystemWatcher'
107
import { getAppSettingsFilePath } from '@src/lib/desktop'
@@ -75,35 +72,10 @@ export function RouteProvider({ children }: { children: ReactNode }) {
7572
// is very high in the context tree, higher than mlEphant's.
7673
if (kclManager.mlEphantManagerMachineBulkManipulatingFileSystem) return
7774

78-
const isCurrentFile = loadedFile?.path === path
79-
if (isCurrentFile) {
80-
if (window.electron) {
81-
// Your current file is changed, read it from disk and write it into the code manager and execute the AST,
82-
// unless the change was initiated by us (the currently running instance).
83-
const code = await window.electron.readFile(path, {
84-
encoding: 'utf-8',
85-
})
86-
87-
const lastWrittenCode = kclManager.lastWrite?.code
88-
if (!lastWrittenCode || !isCodeTheSame(lastWrittenCode, code)) {
89-
const isInSketchMode =
90-
kclManager.modelingState?.matches('Sketch') ||
91-
kclManager.modelingState?.matches('sketchSolveMode')
92-
93-
// Nothing written out yet by ourselves, or it's not the same as the current file content
94-
// -> this must be an external change -> re-execute.
95-
kclManager.updateCodeEditor(code, {
96-
shouldExecute: !isInSketchMode,
97-
shouldResetCamera: !isInSketchMode,
98-
// We explicitly do not write to the file here since we are loading from
99-
// the file system and not the editor.
100-
shouldWriteToDisk: false,
101-
})
102-
103-
toast('Reloading file from disk', { icon: '📁' })
104-
}
105-
}
106-
} else {
75+
// We only react on files other than the currently-executing one here
76+
// because the currently-executing one is handled with its own watcher in
77+
// KclManager. In future, all files and folders will watch themselves.
78+
if (loadedFile?.path !== path) {
10779
const fileNameWithExtension = getStringAfterLastSeparator(path)
10880
// Is the file from the change event type imported into the currently opened file
10981
const isImportedInCurrentFile = kclManager.ast.body.some(

0 commit comments

Comments
 (0)