Revert "Deflake project settings override on desktop (#4370)" (#4450)

* Revert "Deflake project settings override on desktop (#4370)"

This reverts commit ad1cd56891.

* Part of the revert
This commit is contained in:
49fl
2024-11-08 17:16:46 -05:00
committed by 49lf
parent 347a6ef15a
commit fcfecf702b
8 changed files with 34 additions and 49 deletions

View File

@ -318,7 +318,6 @@ test.describe('Testing settings', () => {
timeout: 5_000, timeout: 5_000,
}) })
.toContain(`themeColor = "${userThemeColor}"`) .toContain(`themeColor = "${userThemeColor}"`)
// Only close the button after we've confirmed
}) })
await test.step('Set project theme color', async () => { await test.step('Set project theme color', async () => {
@ -345,13 +344,14 @@ test.describe('Testing settings', () => {
await test.step('Refresh the application and see project setting applied', async () => { await test.step('Refresh the application and see project setting applied', async () => {
// Make sure we're done navigating before we reload // Make sure we're done navigating before we reload
await expect(settingsCloseButton).not.toBeVisible() await expect(settingsCloseButton).not.toBeVisible()
await page.reload({ waitUntil: 'domcontentloaded' }) await page.reload({ waitUntil: 'domcontentloaded' })
await expect(logoLink).toHaveCSS('--primary-hue', projectThemeColor) await expect(logoLink).toHaveCSS('--primary-hue', projectThemeColor)
}) })
await test.step(`Navigate back to the home view and see user setting applied`, async () => { await test.step(`Navigate back to the home view and see user setting applied`, async () => {
await logoLink.click() await logoLink.click()
await page.screenshot({ path: 'out.png' })
await expect(logoLink).toHaveCSS('--primary-hue', userThemeColor) await expect(logoLink).toHaveCSS('--primary-hue', userThemeColor)
}) })

View File

@ -189,15 +189,15 @@ const FileTreeItem = ({
// the ReactNodes are destroyed, so is this listener :) // the ReactNodes are destroyed, so is this listener :)
useFileSystemWatcher( useFileSystemWatcher(
async (eventType, path) => { async (eventType, path) => {
// Prevents a cyclic read / write causing editor problems such as
// misplaced cursor positions.
if (codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher) {
codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher = false
return
}
// Don't try to read a file that was removed. // Don't try to read a file that was removed.
if (isCurrentFile && eventType !== 'unlink') { if (isCurrentFile && eventType !== 'unlink') {
// Prevents a cyclic read / write causing editor problems such as
// misplaced cursor positions.
if (codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher) {
codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher = false
return
}
let code = await window.electron.readFile(path, { encoding: 'utf-8' }) let code = await window.electron.readFile(path, { encoding: 'utf-8' })
code = normalizeLineEndings(code) code = normalizeLineEndings(code)
codeManager.updateCodeStateEditor(code) codeManager.updateCodeStateEditor(code)
@ -242,11 +242,11 @@ const FileTreeItem = ({
// Show the renaming form // Show the renaming form
addCurrentItemToRenaming() addCurrentItemToRenaming()
} else if (e.code === 'Space') { } else if (e.code === 'Space') {
void handleClick() handleClick()
} }
} }
async function handleClick() { function handleClick() {
setTreeSelection(fileOrDir) setTreeSelection(fileOrDir)
if (fileOrDir.children !== null) return // Don't open directories if (fileOrDir.children !== null) return // Don't open directories
@ -258,10 +258,12 @@ const FileTreeItem = ({
`import("${fileOrDir.path.replace(project.path, '.')}")\n` + `import("${fileOrDir.path.replace(project.path, '.')}")\n` +
codeManager.code codeManager.code
) )
await codeManager.writeToFile() // eslint-disable-next-line @typescript-eslint/no-floating-promises
codeManager.writeToFile()
// Prevent seeing the model built one piece at a time when changing files // Prevent seeing the model built one piece at a time when changing files
await kclManager.executeCode(true) // eslint-disable-next-line @typescript-eslint/no-floating-promises
kclManager.executeCode(true)
} else { } else {
// Let the lsp servers know we closed a file. // Let the lsp servers know we closed a file.
onFileClose(currentFile?.path || null, project?.path || null) onFileClose(currentFile?.path || null, project?.path || null)
@ -293,7 +295,7 @@ const FileTreeItem = ({
style={{ paddingInlineStart: getIndentationCSS(level) }} style={{ paddingInlineStart: getIndentationCSS(level) }}
onClick={(e) => { onClick={(e) => {
e.currentTarget.focus() e.currentTarget.focus()
void handleClick() handleClick()
}} }}
onKeyUp={handleKeyUp} onKeyUp={handleKeyUp}
> >
@ -653,13 +655,6 @@ export const FileTreeInner = ({
const isCurrentFile = loaderData.file?.path === path const isCurrentFile = loaderData.file?.path === path
const hasChanged = eventType === 'change' const hasChanged = eventType === 'change'
if (isCurrentFile && hasChanged) return if (isCurrentFile && hasChanged) return
// If it's a settings file we wrote to already from the app ignore it.
if (codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher) {
codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher = false
return
}
fileSend({ type: 'Refresh' }) fileSend({ type: 'Refresh' })
}, },
[loaderData?.project?.path, fileContext.selectedDirectory.path].filter( [loaderData?.project?.path, fileContext.selectedDirectory.path].filter(

View File

@ -41,7 +41,6 @@ import { reportRejection } from 'lib/trap'
import { getAppSettingsFilePath } from 'lib/desktop' import { getAppSettingsFilePath } from 'lib/desktop'
import { isDesktop } from 'lib/isDesktop' import { isDesktop } from 'lib/isDesktop'
import { useFileSystemWatcher } from 'hooks/useFileSystemWatcher' import { useFileSystemWatcher } from 'hooks/useFileSystemWatcher'
import { codeManager } from 'lib/singletons'
import { createRouteCommands } from 'lib/commandBarConfigs/routeCommandConfig' import { createRouteCommands } from 'lib/commandBarConfigs/routeCommandConfig'
type MachineContext<T extends AnyStateMachine> = { type MachineContext<T extends AnyStateMachine> = {
@ -202,13 +201,13 @@ export const SettingsAuthProviderBase = ({
console.error('Error executing AST after settings change', e) console.error('Error executing AST after settings change', e)
} }
}, },
async persistSettings({ context, event }) { persistSettings: ({ context, event }) => {
// Without this, when a user changes the file, it'd // Without this, when a user changes the file, it'd
// create a detection loop with the file-system watcher. // create a detection loop with the file-system watcher.
if (event.doNotPersist) return if (event.doNotPersist) return
codeManager.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true // eslint-disable-next-line @typescript-eslint/no-floating-promises
return saveSettings(context, loadedProject?.project?.path) saveSettings(context, loadedProject?.project?.path)
}, },
}, },
}), }),
@ -222,7 +221,7 @@ export const SettingsAuthProviderBase = ({
}, []) }, [])
useFileSystemWatcher( useFileSystemWatcher(
async (eventType: string) => { async () => {
// If there is a projectPath but it no longer exists it means // If there is a projectPath but it no longer exists it means
// it was exterally removed. If we let the code past this condition // it was exterally removed. If we let the code past this condition
// execute it will recreate the directory due to code in // execute it will recreate the directory due to code in
@ -236,9 +235,6 @@ export const SettingsAuthProviderBase = ({
} }
} }
// Only reload if there are changes. Ignore everything else.
if (eventType !== 'change') return
const data = await loadAndValidateSettings(loadedProject?.project?.path) const data = await loadAndValidateSettings(loadedProject?.project?.path)
settingsSend({ settingsSend({
type: 'Set all settings', type: 'Set all settings',

View File

@ -96,10 +96,10 @@ export class KclPlugin implements PluginValue {
const newCode = viewUpdate.state.doc.toString() const newCode = viewUpdate.state.doc.toString()
codeManager.code = newCode codeManager.code = newCode
// eslint-disable-next-line @typescript-eslint/no-floating-promises
codeManager.writeToFile()
void codeManager.writeToFile().then(() => { this.scheduleUpdateDoc()
this.scheduleUpdateDoc()
})
} }
scheduleUpdateDoc() { scheduleUpdateDoc() {

View File

@ -26,7 +26,6 @@ export function useRefreshSettings(routeId: string = PATHS.INDEX) {
ctx.settings.send({ ctx.settings.send({
type: 'Set all settings', type: 'Set all settings',
settings: routeData, settings: routeData,
doNotPersist: true,
}) })
}, []) }, [])
} }

View File

@ -428,9 +428,13 @@ export class KclManager {
// Update the code state and the editor. // Update the code state and the editor.
codeManager.updateCodeStateEditor(code) codeManager.updateCodeStateEditor(code)
// Write back to the file system. // Write back to the file system.
void codeManager.writeToFile().then(() => this.executeCode()) // eslint-disable-next-line @typescript-eslint/no-floating-promises
codeManager.writeToFile()
// execute the code.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.executeCode()
} }
// There's overlapping responsibility between updateAst and executeAst. // There's overlapping responsibility between updateAst and executeAst.
// updateAst was added as it was used a lot before xState migration so makes the port easier. // updateAst was added as it was used a lot before xState migration so makes the port easier.

View File

@ -121,28 +121,20 @@ export default class CodeManager {
// Only write our buffer contents to file once per second. Any faster // Only write our buffer contents to file once per second. Any faster
// and file-system watchers which read, will receive empty data during // and file-system watchers which read, will receive empty data during
// writes. // writes.
clearTimeout(this.timeoutWriter) clearTimeout(this.timeoutWriter)
this.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true this.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true
this.timeoutWriter = setTimeout(() => {
return new Promise((resolve, reject) => { // Wait one event loop to give a chance for params to be set
this.timeoutWriter = setTimeout(() => { // Save the file to disk
if (!this._currentFilePath) this._currentFilePath &&
return reject(new Error('currentFilePath not set'))
// Wait one event loop to give a chance for params to be set
// Save the file to disk
window.electron window.electron
.writeFile(this._currentFilePath, this.code ?? '') .writeFile(this._currentFilePath, this.code ?? '')
.then(resolve)
.catch((err: Error) => { .catch((err: Error) => {
// TODO: add tracing per GH issue #254 (https://github.com/KittyCAD/modeling-app/issues/254) // TODO: add tracing per GH issue #254 (https://github.com/KittyCAD/modeling-app/issues/254)
console.error('error saving file', err) console.error('error saving file', err)
toast.error('Error saving file, please check file permissions') toast.error('Error saving file, please check file permissions')
reject(err)
}) })
}, 1000) }, 1000)
})
} else { } else {
safeLSSetItem(PERSIST_CODE_KEY, this.code) safeLSSetItem(PERSIST_CODE_KEY, this.code)
} }

View File

@ -186,7 +186,6 @@ export async function loadAndValidateSettings(
if (err(appSettingsPayload)) return Promise.reject(appSettingsPayload) if (err(appSettingsPayload)) return Promise.reject(appSettingsPayload)
let settingsNext = createSettings() let settingsNext = createSettings()
// Because getting the default directory is async, we need to set it after // Because getting the default directory is async, we need to set it after
if (onDesktop) { if (onDesktop) {
settings.app.projectDirectory.default = await getInitialDefaultDir() settings.app.projectDirectory.default = await getInitialDefaultDir()