Implement LSIF emitter in Java.#127
Merged
olafurpg merged 2 commits intosourcegraph:mainfrom Mar 24, 2021
Merged
Conversation
23 tasks
Strum355
reviewed
Mar 24, 2021
| .zipWithIndex | ||
| .foreach { case (line, i) => | ||
| out.append(line) | ||
| out.append(line.replace("\t", "→")) |
Contributor
There was a problem hiding this comment.
Im curious if theres a reason we cant do replace("\t", " ") instead here.
Contributor
Author
There was a problem hiding this comment.
I we can do that but it removes the fact that there's a tab character, which may be helpful to debug an issue.
Strum355
reviewed
Mar 24, 2021
| writer.emitItem(ids.referenceResult, rangeId, doc.id); | ||
|
|
||
| // Definition | ||
| if (occ.getRole() == SymbolOccurrence.Role.DEFINITION && ids.isDefinitionDefined()) { |
Contributor
There was a problem hiding this comment.
in what case is ids.isDefinitionDefined() == true aka when would we have two identical definitions?
Contributor
Author
There was a problem hiding this comment.
The definition ID should always be available in this case, it's a bug if it's not. I updated the code to report an error message in this case instead of silently ignoring it.
Strum355
approved these changes
Mar 24, 2021
Previously, we shelled out to `lsif-semanticdb` to convert SemanticDB files into LSIF. This commit ports the original Go implemementation to Java so that we can call a normal method instead of shelling out to a separate process. The biggest problem with the previous solution is that it complicated installation for users. As we evolve lsif-java, the conversion logic will also evolve and we want to avoid the situation where users have an outdated version of lsif-semanticdb installed. With this commit, users only need to install `lsif-java` and everything should work correctly. This commit also adds a new `snapshot-lsif` command to pretty-print an LSIF index, similar to how we pretty-print SemanticDB payloads. This command is mostly useful for testing purposes, but could be helpful to troubleshoot user issues.
This was referenced Mar 29, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, we shelled out to
lsif-semanticdbto convert SemanticDBfiles into LSIF. This commit ports the original Go implemementation to
Java so that we can call a normal method instead of shelling out to a
separate process.
The biggest problem with the previous solution is that it complicated
installation for users. As we evolve lsif-java, the conversion logic
will also evolve and we want to avoid the situation where users have
an outdated version of lsif-semanticdb installed. With this commit,
users only need to install
lsif-javaand everything should workcorrectly.
This PR also adds a new
snapshot-lsifcommand to pretty-print anLSIF index, similar to how we pretty-print SemanticDB payloads. This
command is mostly useful for testing purposes, but could be helpful
to troubleshoot user issues.
LSIF snapshot generated for lsif-go
LSIF snapshot generated for lsif-node