-
Notifications
You must be signed in to change notification settings - Fork 95
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
[23] Partition support for markdown comments #1542 #1549
Conversation
@ktatavarthi Can you please point me to the relevant test suits where I can find examples and add new tests? |
org.eclipse.jdt.ui/ui/org/eclipse/jdt/ui/text/JavaSourceViewerConfiguration.java
Outdated
Show resolved
Hide resolved
It seems with pulling this change all markdown comments in the editor lose their coloring. Is this intended? Will you add new colors / preferences? Or should be declare that javadoc == javadoc, independent of format? (At face value we're mainly replacing HTML with markdown, but it's still javadoc, right?). Or should we use colors to remind users of which format they are seeing, so they won't accidentally mix both? |
Not intentional. I will work on this.
I would keep it simple and use the same color as Javadoc, at least for now. |
...e.manipulation/common/org/eclipse/jdt/internal/ui/text/AbstractFastJavaPartitionScanner.java
Show resolved
Hide resolved
@@ -57,4 +61,8 @@ public interface IJavaPartitions { | |||
* @since 3.20 |
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.
not related to this change: it looks like #1011 forgot to fix this version during move to another plugin. core.manipulation has no 3.20 to match this.
ContentAssistProcessor markdownProcessor= new JavadocCompletionProcessor(getEditor(), assistant); | ||
assistant.setContentAssistProcessor(markdownProcessor, IJavaPartitions.JAVA_MARKDOWN_COMMENT); | ||
|
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.
There's some inconsistency:
- the processor is registered for markdown partition
- the ctor of JavadocCompletionProcessor still associates it to the javadoc partition
When I tweak JavadocCompletionProcessor to pass the markdown partition into the super ctor, then no proposals will be shown in markdown any more.
IIUC, the reason is that plugin.xml does not associate any proposal computer with the markdown partition type.
Suggestion: add a parameter partition
to the ctor of JavadocCompletionProcessor, then selectively add markdown partition in plugin.xml. This should give us control to enable desired processors while excluding, e.g., HTMLTagCompletionProposalComputer.
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.
In order to boost my confidence in this area (and to get this code to a build), I will take this up also as part of my next iteration.
Unsorted observations & ideas while playing with this PR and stepping through code:
Concerning completion:
In summary: I could not find any obvious bugs, so the PR can be seen as an incremental step forward. I don't think all of the above topics need to be dealt with while this is a preview feature.
|
This actually worked already in many cases. Just some edge-conditions need slight improvement, see eclipse-jdt/eclipse.jdt.core#2861 |
@jarthana given that diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/CompilationUnitEditor.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/CompilationUnitEditor.java
index bf6465b..716be39 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/CompilationUnitEditor.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/javaeditor/CompilationUnitEditor.java
@@ -532,7 +532,7 @@
}
ITypedRegion partition= TextUtilities.getPartition(document, IJavaPartitions.JAVA_PARTITIONING, offset, true);
- if (!IDocument.DEFAULT_CONTENT_TYPE.equals(partition.getType()))
+ if (!IDocument.DEFAULT_CONTENT_TYPE.equals(partition.getType()) && !IJavaPartitions.JAVA_MARKDOWN_COMMENT.equals(partition.getType()))
return;
if (!validateEditorInputState())
return; Do you want to adopt this in your current change? |
The PR addresses this now. |
What it does
Fixes #1542
How to test
Author checklist