Skip to content
This repository was archived by the owner on Jun 19, 2026. It is now read-only.

Cache overlay#52

Merged
itsfuad merged 6 commits into
mainfrom
cache-overlay
Apr 22, 2026
Merged

Cache overlay#52
itsfuad merged 6 commits into
mainfrom
cache-overlay

Conversation

@itsfuad

@itsfuad itsfuad commented Apr 21, 2026

Copy link
Copy Markdown
Member

No description provided.

itsfuad added 4 commits April 21, 2026 10:48
Collect struct-literal completion candidates into completionIndex to
pick
best span at completion time. Change struct literal prefix check to
accept
source.Location. Replace local maxInt64 usage with existing max and drop
the helper. Add dry_test.fer and fix import.

Rules check:
- wrapper added: no
- duplicated logic: no
- helper added: collectStructLiteralCompletionCandidates to centralize
  candidate collection (allowed by RULES.md)
Introduce Compiler.ParsePathWithOverlay and pipeline overlay
methods (ParseEntryOverlay, ParseEntryOverlayForIDE) with shared
finishEntryParse and scheduleParseOverlayFile to parse temporary
overlay contents without changing module identity. LSP now writes
hover overlays into FERRET_CACHE (or user cache), prunes stale
overlays, and uses ParsePathWithOverlay for in-memory buffers.
Add ferretCacheDir, hoverOverlayDir, functionPointer helpers and
tests for overlay behavior.

Rules check: no pass-through wrappers added; reused pipeline
logic; added helpers limited to cache/overlay management.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment on lines 9 to 13
"compiler/internal/core/context"
"compiler/internal/core/diagnostics"
"compiler/internal/core/phase"
compiler "compiler/internal/driver"
"compiler/internal/driver"
"compiler/internal/ir/hir"

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import was changed from an aliased import to an unaliased one, but this file still calls compiler.New(...). As-is, this test won’t compile (undefined compiler) and the driver import may become unused. Either restore the aliased import (compiler "compiler/internal/driver") or update call sites to use driver.New(...) consistently.

Copilot uses AI. Check for mistakes.
Comment thread internal/lsp/server.go
Comment on lines 1762 to +1770
tempPath, err := writeHoverOverlay(path, text)
if err != nil {
return parse(path), path, func() {}
}
cleanup := func() { _ = os.Remove(tempPath) }
return parse(tempPath), tempPath, cleanup
if functionPointer(parse) != functionPointer(defaultParse) {
return parse(path), path, cleanup
}
return compiler.ParsePathWithOverlay(path, tempPath, ide), path, cleanup

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If writeHoverOverlay fails (e.g., cache dir can’t be created, FERRET_CACHE is relative, or FERRET_CACHE=off), parseForProject silently falls back to parsing the on-disk file, which makes hover/completion/rename ignore unsaved overlay text. Consider falling back to os.CreateTemp("", ...) (or another writable temp location) so overlays still work, and/or surfacing the error to the client/logs so it’s not silently incorrect.

Copilot uses AI. Check for mistakes.
Comment thread dry_test.fer Outdated
Comment on lines +1 to +5
import "std/string"

#[allow_unused]
fn usefn(val: i32, callback: fn(i32) ) {
callback(val * 2)

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new .fer file doesn’t appear to be referenced by tests, docs, or build scripts (looks like a local scratch/repro file). If it’s not intended as a committed fixture/example, it should be removed or moved under an appropriate examples/tests directory so it doesn’t add noise to the repo root.

Copilot uses AI. Check for mistakes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread internal/lsp/server.go Outdated
}
item := strings.TrimSpace(body[itemStart:])
if item == "" {
return "", true

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In struct literal completion context, this currently returns ok=true even when the current item is empty (no leading .). Since struct field assignments in Ferret syntax require the .field = value form (see internal/frontend/parser/expr.go:292-300), the LSP completion items (which only provide Label) will insert name/age without the leading ., producing invalid (or at least unintended) syntax when a user accepts a completion at an empty line.

Consider either (a) only enabling struct-field completions when the user has already typed . (i.e., require strings.HasPrefix(item, ".")), or (b) extending the completion payload to support an insert text/text edit that inserts . + field name when needed.

Suggested change
return "", true
return "", false

Copilot uses AI. Check for mistakes.
Comment thread internal/pipeline/pipeline.go Outdated
content, err := os.ReadFile(overlayFile)
if err != nil {
p.ctx.Diagnostics.Add(
diagnostics.NewError(fmt.Sprintf("cannot read module %s", mod.ImportPath)).

Copilot AI Apr 21, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseOverlayFile reports read failures using the same message/code as normal module file reads ("cannot read module %s" + ErrModuleNotFound), but in this path the failing resource is the overlay file (overlayFile). This makes diagnostics misleading when the overlay temp file is missing/unreadable.

Consider mentioning the overlay path in the diagnostic (and/or using a more specific error code/message) so users can tell whether the real module file or the overlay failed to read.

Suggested change
diagnostics.NewError(fmt.Sprintf("cannot read module %s", mod.ImportPath)).
diagnostics.NewError(fmt.Sprintf("cannot read overlay file %s for module %s", overlayFile, mod.ImportPath)).

Copilot uses AI. Check for mistakes.
@itsfuad itsfuad self-assigned this Apr 22, 2026
@itsfuad itsfuad added the enhancement New feature or request label Apr 22, 2026
@itsfuad itsfuad merged commit ac033d7 into main Apr 22, 2026
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants