add:did-create/rename/delete#21
Conversation
fix fix fix:server capability fix:please refer the comment in scheme-langserver.sls test:did-create/delete test:did-create/delete fix:please refer the comment in scheme-langserver.sls fix fix fix fix fix fix
There was a problem hiding this comment.
Pull Request Overview
Adds support for workspace file-change notifications (create, rename, delete) by syncing handlers from xmain and updating tests.
- Introduces JSON fixtures and test integration for did-create and did-delete events.
- Registers new workspace file operations in the server and implements corresponding handlers.
- Refactors workspace initialization with filter facets and adds helper functions for attaching/removing files.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/resources/did-delete.json | New fixture for workspace/didDeleteFiles |
| tests/resources/did-create.json | New fixture for workspace/didCreateFiles |
| tests/protocol/apis/test-file-change-notification.sps | Test script updated to send did-delete and did-create notifications |
| tests/analysis/test-workspace.sps | Updated init-workspace test to use txt filter in resources path |
| scheme-langserver.sls | Registered handlers and capabilities for file-change notifications |
| protocol/apis/file-change-notification.sls | Added did-create, did-rename, did-delete implementations |
| analysis/workspace.sls | Refactored init-workspace to use facets; added attach/remove helpers |
| analysis/package-manager/txt-filter.sls | Simplified generate-txt-file-filter signature and logic |
Comments suppressed due to low confidence (2)
protocol/apis/file-change-notification.sls:69
- [nitpick] The parameter name
siblinsis misspelled and can be misleading; consider renaming it tosiblingorsibling-nodefor clarity.
(lambda (siblins)
tests/protocol/apis/test-file-change-notification.sps:53
- There’s no test coverage for the new
workspace/didRenameFileshandler. Consider adding adid-renameJSON fixture, header block, and corresponding assertions to this test.
did-delete-header did-delete-json
| (define (remove-new-file path parent my-filter) | ||
| (let ([f (walk-file parent path)]) | ||
| (cond | ||
| [(not (null? f)) '()] |
There was a problem hiding this comment.
The early-return condition in remove-new-file is inverted: it currently returns immediately when f is found and only removes children when f is absent. Swap the branches so that when f is null you return, otherwise proceed to remove the node.
| [(not (null? f)) '()] | |
| [(null? f) '()] |
| (define (remove-new-file path parent my-filter) | ||
| (let ([f (walk-file parent path)]) | ||
| (cond | ||
| [(not (null? f)) '()] | ||
| [else | ||
| (file-node-children-set! | ||
| (file-node-parent f) | ||
| (filter | ||
| (lambda (x) (not (equal? x f))) | ||
| (file-node-children (file-node-parent f))))]))) | ||
|
|
||
| (define (attach-new-file path parent my-filter) | ||
| (let ([f (walk-file parent path)]) | ||
| (cond | ||
| [(not (my-filter path)) '()] | ||
| [(not (file-exists? path)) '()] | ||
| [(not (null? f)) f] | ||
| [(file-node-folder? parent) | ||
| (let ([maybe-parent |
There was a problem hiding this comment.
remove-new-file is defined but never used; either remove this function to reduce dead code or integrate it into did-delete for consistency.
| (define (remove-new-file path parent my-filter) | |
| (let ([f (walk-file parent path)]) | |
| (cond | |
| [(not (null? f)) '()] | |
| [else | |
| (file-node-children-set! | |
| (file-node-parent f) | |
| (filter | |
| (lambda (x) (not (equal? x f))) | |
| (file-node-children (file-node-parent f))))]))) | |
| (define (attach-new-file path parent my-filter) | |
| (let ([f (walk-file parent path)]) | |
| (cond | |
| [(not (my-filter path)) '()] | |
| [(not (file-exists? path)) '()] | |
| [(not (null? f)) f] | |
| [(file-node-folder? parent) | |
| (let ([maybe-parent | |
| ;; Logic from `remove-new-file` integrated into `did-delete` function. | |
| ;; Remove the `remove-new-file` function entirely. |
sync from xmain