Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Conversation

varungandhi-src
Copy link
Contributor

@varungandhi-src varungandhi-src commented Nov 15, 2023

The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.

Test plan

TODO:

  • Check final state with sg start web-standalone

@cla-bot cla-bot bot added the cla-signed label Nov 15, 2023
@varungandhi-src varungandhi-src changed the title client: Simplify some code in useCodeIntel.ts client: Minor cleanup for search-based code intel Nov 15, 2023
Base automatically changed from vg/simplify-useCodeIntel to main November 16, 2023 14:53
@varungandhi-src varungandhi-src marked this pull request as ready for review November 21, 2023 11:43
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Nov 21, 2023

📖 Storybook live preview

getSetting,
filter,
}: UseSearchBasedCodeIntelOptions): Promise<Location[]> {
function searchBasedReferencesViaSCIPLocals(options: UseSearchBasedCodeIntelOptions): Location[] | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my understanding: This is "search based" because, although using SCIP, we don't actually have any connections between the occurrences/symbols. We are just looking up occurrences by name (or something like that)?

Copy link
Contributor Author

@varungandhi-src varungandhi-src Nov 24, 2023

Choose a reason for hiding this comment

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

Right now, "search based" is the catch-all term for anything that is not precise (i.e. coming from a compiler/type-checker). In this case, the SCIP Document that is attached to the blob view came from the syntax highlighter, which is able to add information about local symbols using purely tree-sitter for some languages.

That's why this is "search based".

SCIP as a protocol does not dictate what the source of SCIP data is. We have multiple producers, such as the syntax highlighter, compilers etc.

@varungandhi-src varungandhi-src merged commit 8cd293b into main Nov 24, 2023
@varungandhi-src varungandhi-src deleted the vg/simplify-searchbased branch November 24, 2023 09:05
vovakulikov pushed a commit that referenced this pull request Dec 12, 2023
The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.
varungandhi-src added a commit that referenced this pull request Jan 16, 2024
The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.

(cherry-picked from e955cddec490d0cc2b5eba36be2ec4958ba06bf8)
@varungandhi-src varungandhi-src mentioned this pull request Jan 16, 2024
varungandhi-src added a commit that referenced this pull request Jan 17, 2024
The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.

(cherry-picked from e955cddec490d0cc2b5eba36be2ec4958ba06bf8)
varungandhi-src added a commit that referenced this pull request Jan 23, 2024
The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.

(cherry-picked from e955cddec490d0cc2b5eba36be2ec4958ba06bf8)
BolajiOlajide pushed a commit that referenced this pull request Jan 24, 2024
…58954) (#59636)

* client: Minor cleanup for search-based code intel (#58331)

The separation of the logic into different functions makes it clearer
what the order of searches is. It also makes it clearer that for some
reason, we're only using the locals information from the SCIP
Document for 'Find references', and not for 'Go to definition'.
Using the SCIP Document for for 'Go to definition' too could avoid
a network request.

(cherry-picked from e955cddec490d0cc2b5eba36be2ec4958ba06bf8)

* client: Avoid complex tokenization in ref panel code (#58954)

Previously, we relied on detecting the language from file paths,
then using various regexes associated with the language to identify
token boundaries. However, the code mirror blob view always provides
a full token range, which can be used directly, instead of attempting
to recompute the token boundaries.

For older URLs, we fallback to simple identifiers, which should
work for the vast majority of languages and identifiers.

We cannot yet remove the language detection here because the file
extensions associated with the language are later used for search-based
code navigation.

This patch also makes the language spec optional for search-based
code intel, as we do not have a solution to #56376 which would
guarantee that we always have a language available. If a language
is not available, search-based code intel falls back to searching
other files with the same extension as a best effort guess.

Locally tested for MATLAB code. The ref panel shows up correctly,
unlike the error earlier.

(cherry-picked from c42cad2)

* Fix lint error due to short variable name
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants