Skip to content

Ensure special files are also indexed #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 2, 2023
Merged

Conversation

hendrikvanantwerpen
Copy link
Collaborator

◀️ #260 #262 ▶️

This PR ensures that special files (those that are relevant for a language, but not written in that language, such as tsconfig.json) are always indexed.

Previously, the language loader only returned the primary language of a file (i.e., the language the file is written in). These changes return the primary, as well as any secondary languages (i.e., languages for which the file is special). The indexer makes sure that all languages get their change to generate stack graph for the file.

I have wondered (again) whether there should be a difference between primary and secondary languages at all. I think that still makes sense for the following reasons:

  1. The contributions of a secondary language are typically not relevant when navigating the file. They are only useful for the effect they have on navigating the files of that secondary language.

  2. When processed as a special file, more information is provided to the file analyzer, such as the list of paths in the project. This makes the results inherently less context-free. I kind of like to make this an explicit choice in the API (even though it could be left to convention).

@hendrikvanantwerpen hendrikvanantwerpen requested a review from a team as a code owner April 19, 2023 10:58
@hendrikvanantwerpen hendrikvanantwerpen self-assigned this Apr 19, 2023
Base automatically changed from cli-canonical-paths to main May 10, 2023 09:44
.and_then(|file_name| lc.special_files.get(&file_name.to_string_lossy()))
{
Some(fa) => fa,
None => continue, // shouldn't really happen though
Copy link

Choose a reason for hiding this comment

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

Can we log “invariant violated” or something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

First I was going to turn this into a panic. But now I've changed it so that the loader returns not only a reference to the language configuration, but also the specific analyzer for the file. This should remove any possible confusion on (a) where to get the analyzer, and (b) whether that lookup could still fail.

@hendrikvanantwerpen hendrikvanantwerpen merged commit 868614b into main Jun 2, 2023
@hendrikvanantwerpen hendrikvanantwerpen deleted the index-special-files branch June 2, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants