Skip to content

Commit a66c669

Browse files
committed
Revert "Move responsibility of disk watching for active editor into KclManager, don't watch while writing (#10305)"
This reverts commit 7ac36d3.
1 parent e7c3f56 commit a66c669

7 files changed

Lines changed: 74 additions & 130 deletions

File tree

e2e/playwright/debug-pane.spec.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
5252
await page.keyboard.press('ArrowDown')
5353
}
5454
})
55-
let lastSegmentText = await segment.innerText()
5655
// TODO: if you type all the code at once without delay (or paste it in)
5756
// the initial segment artifact ID is different. This appears to be niche bug
5857
// that is being sidestepped in this test until https://github.com/KittyCAD/modeling-app/issues/9609 is addressed.
@@ -61,23 +60,20 @@ test.describe('Debug pane', { tag: '@desktop' }, () => {
6160
// Wait for keyboard input debounce and updated artifact graph.
6261
await page.waitForTimeout(1000)
6362
})
64-
await expect(segment).not.toHaveText(lastSegmentText)
63+
// Extract the artifact IDs from the debug artifact graph.
64+
const initialSegmentIds = await segment.innerText({ timeout: 5_000 })
6565
// The artifact ID should include a UUID.
66-
const uuidRegexp =
66+
expect(initialSegmentIds).toMatch(
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-
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()
68+
)
69+
await test.step('Enter a comment', async () => {
7370
await page.keyboard.type('\n|> line(end = [2, 2])', { delay: 10 })
7471
// Wait for keyboard input debounce and updated artifact graph.
7572
await page.waitForTimeout(1000)
7673
})
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)
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)
8278
})
8379
})

src/components/CommandBar/CommandBarSelectionMixedInput.spec.tsx

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,10 @@ describe('CommandBarSelectionMixedInput', () => {
5555
describe('clearSelectionFirst behavior', () => {
5656
it('should send clear selection command when clearSelectionFirst is true', async () => {
5757
const app = App.getDefaultSystems()
58-
const executingEditor = new KclManager('some-path', {
58+
const executingEditor = new KclManager({
5959
commandBar: app.commands.actor,
6060
settings: app.settings.actor,
6161
wasmInstancePromise: app.wasmPromise,
62-
projectPath: 'some-project',
6362
})
6463
const mockModelingSend = vi.spyOn(
6564
executingEditor.engineCommandManager,
@@ -86,11 +85,10 @@ describe('CommandBarSelectionMixedInput', () => {
8685

8786
it('should NOT send clear selection command when clearSelectionFirst is false', async () => {
8887
const app = App.getDefaultSystems()
89-
const executingEditor = new KclManager('some-path', {
88+
const executingEditor = new KclManager({
9089
commandBar: app.commands.actor,
9190
settings: app.settings.actor,
9291
wasmInstancePromise: app.wasmPromise,
93-
projectPath: 'some-project',
9492
})
9593
const mockModelingSend = vi.spyOn(
9694
executingEditor.engineCommandManager,
@@ -114,11 +112,10 @@ describe('CommandBarSelectionMixedInput', () => {
114112

115113
it('should NOT send clear selection command when clearSelectionFirst is undefined', async () => {
116114
const app = App.getDefaultSystems()
117-
const executingEditor = new KclManager('some-path', {
115+
const executingEditor = new KclManager({
118116
commandBar: app.commands.actor,
119117
settings: app.settings.actor,
120118
wasmInstancePromise: app.wasmPromise,
121-
projectPath: 'some-project',
122119
})
123120
const mockModelingSend = vi.spyOn(
124121
executingEditor.engineCommandManager,
@@ -142,11 +139,10 @@ describe('CommandBarSelectionMixedInput', () => {
142139

143140
it('should send clear selection command only once on mount', async () => {
144141
const app = App.getDefaultSystems()
145-
const executingEditor = new KclManager('some-path', {
142+
const executingEditor = new KclManager({
146143
commandBar: app.commands.actor,
147144
settings: app.settings.actor,
148145
wasmInstancePromise: app.wasmPromise,
149-
projectPath: 'some-project',
150146
})
151147
const mockModelingSend = vi.spyOn(
152148
executingEditor.engineCommandManager,
@@ -185,11 +181,10 @@ describe('CommandBarSelectionMixedInput', () => {
185181

186182
it('should set hasClearedSelection state after clearing', async () => {
187183
const app = App.getDefaultSystems()
188-
const executingEditor = new KclManager('some-path', {
184+
const executingEditor = new KclManager({
189185
commandBar: app.commands.actor,
190186
settings: app.settings.actor,
191187
wasmInstancePromise: app.wasmPromise,
192-
projectPath: 'some-project',
193188
})
194189
const mockModelingSend = vi.spyOn(
195190
executingEditor.engineCommandManager,

src/components/RouteProvider.tsx

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

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) {
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 {
79107
const fileNameWithExtension = getStringAfterLastSeparator(path)
80108
// Is the file from the change event type imported into the currently opened file
81109
const isImportedInCurrentFile = kclManager.ast.body.some(

0 commit comments

Comments
 (0)