-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
add support for path references #90
base: master
Are you sure you want to change the base?
Changes from all commits
a59d3e9
bda6899
c67bbaa
210325b
8c786a1
71a79a4
6968fc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
package org.nixos.idea.imports | ||
|
||
import com.intellij.codeInsight.lookup.LookupElement | ||
import com.intellij.openapi.util.TextRange | ||
import com.intellij.openapi.util.io.FileUtil | ||
import com.intellij.openapi.vfs.LocalFileSystem | ||
import com.intellij.openapi.vfs.VirtualFile | ||
import com.intellij.openapi.vfs.VirtualFileSystem | ||
import com.intellij.patterns.PlatformPatterns | ||
import com.intellij.psi.PsiElement | ||
import com.intellij.psi.PsiManager | ||
import com.intellij.psi.PsiReference | ||
import com.intellij.psi.PsiReferenceBase | ||
import com.intellij.psi.PsiReferenceContributor | ||
import com.intellij.psi.PsiReferenceProvider | ||
import com.intellij.psi.PsiReferenceRegistrar | ||
import com.intellij.util.ProcessingContext | ||
import org.nixos.idea.psi.impl.NixExprStdPathMixin | ||
|
||
class NixFilePathReferenceContributor: PsiReferenceContributor() { | ||
override fun registerReferenceProviders(registrar: PsiReferenceRegistrar) { | ||
registrar.registerReferenceProvider( | ||
PlatformPatterns.psiElement(NixExprStdPathMixin::class.java), | ||
object : PsiReferenceProvider() { | ||
override fun getReferencesByElement( | ||
element: PsiElement, | ||
context: ProcessingContext | ||
): Array<PsiReference> { | ||
val it = element as? NixExprStdPathMixin ?: return emptyArray() | ||
return arrayOf(NixImportReferenceImpl(it)) | ||
} | ||
} | ||
) | ||
} | ||
} | ||
|
||
private class NixImportReferenceImpl(key: NixExprStdPathMixin) : PsiReferenceBase<NixExprStdPathMixin>(key) { | ||
override fun resolve(): PsiElement? { | ||
val path = element.containingFile.parent?.virtualFile?.path ?: return null | ||
val fs = LocalFileSystem.getInstance() | ||
val file = resolvePath(fs, path, element.text) ?: return null | ||
|
||
val project = element.project | ||
val psiFile = PsiManager.getInstance(project).findFile(file) | ||
|
||
return psiFile | ||
} | ||
|
||
override fun getVariants(): Array<out LookupElement> = LookupElement.EMPTY_ARRAY | ||
|
||
override fun calculateDefaultRangeInElement(): TextRange = TextRange.from(0, element.textLength) | ||
} | ||
|
||
private fun resolvePath(fs: VirtualFileSystem, cwd: String, target: String): VirtualFile? { | ||
val resolved = FileUtil.join(cwd, target) | ||
val resolvedFile = fs.findFileByPath(resolved) ?: return null | ||
|
||
if (resolvedFile.isDirectory) { | ||
return resolvedFile.findChild("default.nix") | ||
} | ||
|
||
return resolvedFile | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package org.nixos.idea.psi.impl | ||
|
||
import com.intellij.lang.ASTNode | ||
import com.intellij.psi.PsiReference | ||
import com.intellij.psi.impl.source.resolve.reference.ReferenceProvidersRegistry | ||
|
||
open class NixExprStdPathMixin(node: ASTNode): NixExprPathImpl(node) { | ||
override fun getReferences(): Array<PsiReference> = | ||
ReferenceProvidersRegistry.getReferencesFromProviders(this) | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -191,7 +191,9 @@ expr_uri ::= URI | |||
;{ extends("expr_lookup_path|expr_std_path")=expr_path } | ||||
expr_path ::= expr_lookup_path | expr_std_path | ||||
expr_lookup_path ::= SPATH | ||||
expr_std_path ::= PATH_SEGMENT (PATH_SEGMENT | antiquotation)* PATH_END | ||||
expr_std_path ::= PATH_SEGMENT (PATH_SEGMENT | antiquotation)* PATH_END { | ||||
mixin="org.nixos.idea.psi.impl.NixExprStdPathMixin" | ||||
} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Do you really need both, a reference contributor and the mixin? As far as I know, this are two different ways to implement references, you shouldn't need both. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mixin calls into the reference contributor here.
Seems like we don't need both but everyone uses the reference contributor + the mixin it seems. Without this explicit call the reference contributor will not work so I think you actually need both or can get away with only the mixin but not only the reference contributor. |
||||
expr_parens ::= LPAREN expr recover_parens RPAREN { pin=1 } | ||||
expr_attrs ::= [ REC | LET ] LCURLY recover_set (bind recover_set)* RCURLY { pin=2 } | ||||
expr_list ::= LBRAC recover_list (expr_select recover_list)* RBRAC { pin=1 } | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (non-blocking): Would be nice to have some test to cover this functionality. However, not sure how a test could look like. I have some tests regarding references in
SymbolNavigationTest
, but their setup is probably far too complex for your use case, and it also doesn't support asserting for references to files.It is not a hard requirement before I merge the PR, but it would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i thought about this a bit and concluded idk how to write the tests. maybe we can pair on it together? i'm happy to schedule some time over the next few weeks if you're up for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean as a live call? I somehow didn't expect that in the context of a pull request on GitHub. 😄 Unfortunately, it is difficult for me to plan specific times before the start of January. Not sure if I want to wait for January. I suppose you are not on the 38C3 in Hamburg (Germany), are you?^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah! a live call! i prefer it as we can communicate a bit faster. seconds rtt instead of minutes / hours / days.
no plans to go to this 38C3 thing, seems interesting though maybe that could change, will see whether it makes sense.
what if we do this, let's merge the PR if you're fine with the lack of tests, let's get a time on the books (probably 30m) for early Jan and if i make it to 38C3 (unlikely though) we can meetup and solve it in real life? how does jan 1 sound (probably like after 5pm germany time?)