-
Notifications
You must be signed in to change notification settings - Fork 21
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
Revamp Jelly completion suggestions #197
Open
janfaracik
wants to merge
32
commits into
jenkinsci:master
Choose a base branch
from
janfaracik:namespace-imports
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+443
−113
Open
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
dbcbb39
Init
janfaracik fe83f21
Support multiple namespace imports
janfaracik 25dfb8a
Update JellyCompletionContributor.java
janfaracik ed250f4
Working build
janfaracik 297b2f9
Working build
janfaracik 4370f0f
WB
janfaracik 09929c2
Update icon
janfaracik 10ca5c0
Remove old files
janfaracik 8430cc9
Update JellyCompletionContributor.java
janfaracik a669a09
Update JellyCompletionContributor.java
janfaracik 8e7b583
Delete stapler.png
janfaracik ecf90e7
Update JellyCompletionContributor.java
janfaracik e589f59
Update JellyCompletionContributor.java
janfaracik 95acb67
Update JellyCompletionContributor.java
janfaracik 10cd3fa
Add tests
janfaracik 2c0fde5
Push
janfaracik 13079d2
Update StaplerCustomJellyTagLibraryXmlNSDescriptor.java
janfaracik bc74174
Merge branch 'lint' into namespace-imports
janfaracik e7fda3c
Lint
janfaracik 7fae33a
Tests passing
janfaracik 7287f3e
Tidy up
janfaracik 05b3d95
Update StaplerCustomJellyTagLibraryXmlNSDescriptor.java
janfaracik c9ee9c5
Delete smokeJexlInspection2.jelly
janfaracik 253c79e
Tidy up
janfaracik 89d2225
Merge branch 'master' into namespace-imports
janfaracik de929d5
Update JellyCompletionContributor.java
janfaracik 8f85902
Update JellyCompletionContributor.java
janfaracik ea766c2
Update JellyCompletionContributor.java
janfaracik f3b5aa3
Find project namespaces and offer them
janfaracik 488e1a3
Update NamespaceUtil.java
janfaracik 7ac6a63
Fix bug with duplicate entries + handle jelly libs
janfaracik d9e8d5e
Merge branch 'master' into namespace-imports
janfaracik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
51 changes: 51 additions & 0 deletions
51
src/main/java/io/jenkins/stapler/idea/jelly/NamespaceUtil.java
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
package io.jenkins.stapler.idea.jelly; | ||
|
||
import com.intellij.openapi.project.Project; | ||
import com.intellij.psi.PsiFile; | ||
import com.intellij.psi.PsiManager; | ||
import com.intellij.psi.search.FileTypeIndex; | ||
import com.intellij.psi.search.GlobalSearchScope; | ||
import com.intellij.psi.xml.XmlTag; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import org.kohsuke.stapler.idea.language.JellyFileType; | ||
import org.kohsuke.stapler.idea.psi.JellyFile; | ||
|
||
public class NamespaceUtil { | ||
|
||
/** Collects all namespaces (and prefixes) from .jelly files in the project */ | ||
public static Map<String, String> collectProjectNamespaces(Project project) { | ||
Map<String, String> namespaces = new HashMap<>(); | ||
|
||
FileTypeIndex.processFiles( | ||
JellyFileType.INSTANCE, | ||
file -> { | ||
PsiFile psiFile = PsiManager.getInstance(project).findFile(file); | ||
if (psiFile instanceof JellyFile jellyFile) { | ||
XmlTag rootTag = jellyFile.getRootTag(); | ||
if (rootTag != null) { | ||
String[] uris = rootTag.knownNamespaces(); | ||
|
||
for (String uri : uris) { | ||
String prefix = rootTag.getPrefixByNamespace(uri); | ||
|
||
if (prefix == null || prefix.isEmpty()) { | ||
continue; | ||
} | ||
|
||
// Ignore local prefixes as they're not for global usage | ||
if (prefix.equals("local") || prefix.equals("this")) { | ||
continue; | ||
} | ||
|
||
namespaces.put(prefix, uri); | ||
} | ||
} | ||
} | ||
return true; | ||
}, | ||
GlobalSearchScope.projectScope(project)); | ||
|
||
return namespaces; | ||
} | ||
} |
285 changes: 184 additions & 101 deletions
285
src/main/java/org/kohsuke/stapler/idea/JellyCompletionContributor.java
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains 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
This file contains 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
5 changes: 5 additions & 0 deletions
5
src/main/resources/org/kohsuke/stapler/idea/icons/component.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
101 changes: 101 additions & 0 deletions
101
src/test/java/org/kohsuke/stapler/idea/JellyCompletionContributorTest.java
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
package org.kohsuke.stapler.idea; | ||
|
||
import com.intellij.testFramework.fixtures.BasePlatformTestCase; | ||
|
||
public class JellyCompletionContributorTest extends BasePlatformTestCase { | ||
|
||
@Override | ||
protected String getTestDataPath() { | ||
return "src/test/testData"; | ||
} | ||
|
||
public void testDefaultTagLibrary() { | ||
assertDefaultTagLibrary( | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core"> | ||
<l:b<caret> | ||
</j:jelly> | ||
""", | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout"> | ||
<l:basic /> | ||
</j:jelly> | ||
"""); | ||
} | ||
|
||
public void testRequiredAttributes() { | ||
assertDefaultTagLibrary( | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core"> | ||
<l:req<caret> | ||
</j:jelly> | ||
""", | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout"> | ||
<l:required title="" /> | ||
</j:jelly> | ||
"""); | ||
} | ||
|
||
public void testInvokeBody() { | ||
assertDefaultTagLibrary( | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core"> | ||
<l:child<caret> | ||
</j:jelly> | ||
""", | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout"> | ||
<l:children> | ||
\s | ||
</l:children> | ||
</j:jelly> | ||
"""); | ||
} | ||
|
||
public void testCustomTagLibrary() { | ||
myFixture.copyDirectoryToProject("testlib", "testlib"); | ||
myFixture.configureByText( | ||
"basic.jelly", | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:t="/testlib"> | ||
<t:t<caret> | ||
</j:jelly> | ||
"""); | ||
|
||
myFixture.completeBasic(); | ||
|
||
myFixture.checkResult( | ||
""" | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:t="/testlib"> | ||
<t:test /> | ||
</j:jelly> | ||
"""); | ||
} | ||
|
||
private void assertDefaultTagLibrary(String body, String expected) { | ||
// Simulate the default tag libraries in core | ||
myFixture.copyDirectoryToProject("lib", "lib"); | ||
myFixture.configureByText("basic.jelly", body); | ||
|
||
myFixture.completeBasic(); | ||
|
||
myFixture.checkResult(expected); | ||
} | ||
} |
1 change: 0 additions & 1 deletion
1
src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
This file was deleted.
Oops, something went wrong.
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core"> | ||
<p>Test</p> | ||
</j:jelly> |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:d="jelly:define"> | ||
<d:invokeBody /> | ||
</j:jelly> |
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler"> | ||
<st:documentation> | ||
<st:attribute name="title" use="required" /> | ||
</st:documentation> | ||
<p>Test</p> | ||
</j:jelly> |
Empty file.
File renamed without changes.
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.
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.
Need to check the performance of this and how often its called and if there's any caching
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.
Aye, need to see if there's any performance regressions, and if so how we can avoid them with cache.
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.
this seems to be called on every key press although I didn't notice any delay... might be worth optimising if it doesn't cause other issues