fix(editor): expose filesystem info for checkpoints - #47
Conversation
| local attrs = lfs.attributes(path) | ||
| if not attrs then return end | ||
|
|
||
| local types = { |
There was a problem hiding this comment.
Please represent the file types in the (pseudo-)type system so at least it's linter-validated by the editor tooling, and introduce an alias for {type: love.FileType, size: number, modtime: number}, e.g.
--- @class FileInfo
--- @field type love.FileType
--- @field size number
--- @field modtime numberAssuming this link/symlink renaming is necessary, I'm not sure I understand the point of it.
There was a problem hiding this comment.
LuaFileSystem calls a symbolic link "link", while LÖVE’s FileType calls it "symlink". The mapping was intended to make the test implementation return the same vocabulary as production.
However, the current code calls lfs.attributes, which seems to follow symbolic links and report the target as "file" or "directory". So this is essentially dead code. I'll remove it.
There was a problem hiding this comment.
That makes sense. Still, let's add an enum for it
Checkpoint timestamp lookup called FS.getInfo, but the adapter exposed only FS.exists, crashing the first checkpoint. Mirror getInfo across runtime and test backends, and route exists through it so metadata semantics stay centralized. Add a filesystem regression spec that verifies the metadata shape and type-filter behavior on the target branch. Refs: compy-editor-checkpoint-fs-getinfo-crash
2c125e1 to
38d7c75
Compare
|
@aldum I must clarify that I encountered this bug using the branch with a heavily updated editor that we now use for lessons. Didn't realize this at the moment. Thus, the bug was caused by the code that we've just wrote and that is not yet merged. However, I explored the options and came to a conclusion that this is, in fact, not a bad solution. We save checkpoints and need to check the last modification time. The only way to do that without adding any new API seemed to read the current project object and iterate over all files until a matching one is found. This is quite ugly imo (and I'm not sure it gets regenerated often). link/symlink rename is removed as it defends against a situation that should not happen. Type annotations added. |
| local attrs = lfs.attributes(path) | ||
| if not attrs then return end | ||
|
|
||
| local types = { |
There was a problem hiding this comment.
That makes sense. Still, let's add an enum for it
2 participants
What
FS.getInfo(path, filtertype, vfs)through the runtime filesystem adapterFileInfoshape for LuaCATS-aware editor toolinglfs.attributesFS.existsthroughFS.getInfoWhy
The checkpoint timestamp lookup calls
FS.getInfo, but the adapter only exposedFS.exists. The missing method crashes the first checkpoint attempt. This isolates the adapter fix as one commit on top ofaldum/dev.API compatibility
The production
FS.existssignature was already(path, filtertype, vfs). This change adds the optionalfiltertypeargument only to the test backend, bringing it in line with production. Omitting the argument still passesniland checks for any filesystem entry, so existing one-argument calls keep the same behavior. Existing production callers use the filter to distinguish files from directories.FileInfo.typeis alove.FileType;sizeandmodtimeare optional because LÖVE may be unable to determine them. The test backend useslfs.attributes, which follows symbolic links, so it maps only LuaFileSystem'sfileanddirectorymodes directly and reports other modes as LÖVE'sother. It does not claimsymlinksemantics.Validation
tests/util/fs_spec.lua: 9 successes, 0 failures, 0 errorsluac5.1 -p src/util/filesystem.lua tests/util/fs_spec.luagit diff --check origin/dev...HEAD