Skip to content

Add API to index in-memory sources#587

Merged
vinistock merged 1 commit intomainfrom
02-17-add_api_to_index_in-memory_sources
Feb 24, 2026
Merged

Add API to index in-memory sources#587
vinistock merged 1 commit intomainfrom
02-17-add_api_to_index_in-memory_sources

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Feb 17, 2026

This PR allows us to index documents that are stored in-memory (like unsaved files) rather than on disk.

I was starting the work on the Ruby LSP adoption and realized that all of our tests need to be able to index in-memory sources. Rather than re-writing the entire test suite, I figured we ought to add this API already since we need it for the regular LSP operations anyway.

My proposal is that we create a separate API for in-memory sources that is disconnected from indexing all files from disk. The reason is because we have these scenarios:

  • Boot: we need to index everything from disk based on the workspace path + dependency paths
  • Watched files change notifications: these notifications are received when a file modification was committed to disk and we always receive a list of modifications. Using the existing index_all API, which accepts an array and runs in parallel is the adequate choice
  • Regular file modifications: these are the only ones that happen in-memory and they are always sent to the LSP one by one

@vinistock vinistock self-assigned this Feb 17, 2026
@vinistock vinistock added the enhancement New feature or request label Feb 17, 2026 — with Graphite App
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vinistock vinistock marked this pull request as ready for review February 17, 2026 19:35
@vinistock vinistock requested a review from a team as a code owner February 17, 2026 19:35
let source = normalize_indentation(source);
let local_index = Self::index_source(uri, &source);
self.graph.update(local_index);
indexing::index_source(&mut self.graph, uri, &source, &LanguageId::Ruby);
Copy link
Member Author

Choose a reason for hiding this comment

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

For tests, I would rather expose an index_rbs_uri rather than modifying the signature to accept a new language_id parameter. Let me know if you disagree.

Copy link
Contributor

Choose a reason for hiding this comment

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

A separate helper for indexing rbs uris is fine for testing 👍

@vinistock vinistock force-pushed the 02-17-add_api_to_index_in-memory_sources branch from 2be87f9 to 0f56225 Compare February 23, 2026 15:00
Comment on lines +86 to +87
let language = self.path.extension().map_or(LanguageId::Ruby, LanguageId::from);
let local_graph = build_local_graph(url.to_string(), &source, &language);

Choose a reason for hiding this comment

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

Can these 2 lines be replaced with just a call to your new index_source function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not quite. There are two subtle differences:

  • index_source expects the language ID to be an enum since that gets passed by the editor to us. This is reading and transforming from the file's extension
  • index_source immediately updates the graph, but we don't want that to happen in the parallel indexing case. The threads send the indexing result through a channel and the main thread is responsible for the merge

@vinistock vinistock requested a review from amomchilov February 23, 2026 21:31
@vinistock vinistock merged commit 01d9125 into main Feb 24, 2026
27 checks passed
@vinistock vinistock deleted the 02-17-add_api_to_index_in-memory_sources branch February 24, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants