Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ public KeyWithDefaultValue<Boolean> getKey() {
}

@Override
public void processText(Project project,
Template template,
Document document,
RangeMarker templateRange,
Editor editor) {
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {
Comment on lines 46 to +47

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consistency nit: this PR adds @RequiredReadAction to processText in ShortenToStaticImportProcessor, but this one does the same kind of PSI work (PsiUtilBase.getPsiFileInEditor, plus JavaCodeStyleManager#shortenClassReferences, which is itself @RequiredReadAction in JavaCodeStyleManagerImpl) and stays unannotated. Since the signature line is being reformatted here anyway, annotating both keeps the pair symmetric.

Needs import consulo.annotation.access.RequiredReadAction; next to the other consulo.annotation.* imports.

Suggested change
@Override
public void processText(Project project,
Template template,
Document document,
RangeMarker templateRange,
Editor editor) {
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {
@Override
@RequiredReadAction
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {

try {
PsiDocumentManager.getInstance(project).commitDocument(document);
JavaCodeStyleManager javaStyle = JavaCodeStyleManager.getInstance(project);
Expand All @@ -66,4 +62,9 @@ public void processText(Project project,
public LocalizeValue getOptionText() {
return CodeInsightLocalize.dialogEditTemplateCheckboxShortenFqNames();
}

@Override
public boolean isVisible(Template template) {
return true;
}
Comment on lines +66 to +69

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Correct sync, and worth spelling out why it is needed here, because Consulo differs from JB at the interface level:

  • JB's TemplateOptionalProcessor#isVisible defaults to return true, and ShortenFQNamesProcessor simply doesn't override it — hence "always visible".
  • Consulo's default is template.containsOption(getKey()) (TemplateOptionalProcessor.java), and TemplateImpl#containsOption is myOptions.containsKey(...) — i.e. true only when the option was explicitly persisted in the template XML. Since KEY defaults to true and no bundled Java template stores java-shorted-fq-names, the checkbox was invisible for effectively every template while isEnabled still returned true. The option was silently on and un-toggleable; this override fixes that.

Side effect worth being aware of: LiveTemplateSettingsEditor builds the option checkboxes for all processors (processor.isVisible(myTemplate)), so this checkbox now also shows for non-Java templates (XML, HTML, ...). That matches JB and is harmless at runtime — JavaCodeStyleManagerImpl.shortenClassReferences(element, start, end) is a no-op when ReferenceAdjuster.forLanguage(...) returns null (JavaCodeStyleManagerImpl.java:118-126) — but a Java-specific option is now shown in every live template editor. If that is undesirable, the fix belongs in the platform (a context-aware isVisible(Template, TemplateContext), as JB has), not here.

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

/**
* @author Denis Zhdanov
* @since 4/27/11 3:07 PM
* @since 2011-04-27
*/
@ExtensionImpl
public class ShortenToStaticImportProcessor implements TemplateOptionalProcessor {
Expand All @@ -57,21 +57,22 @@ public KeyWithDefaultValue<Boolean> getKey() {
}

@Override
@RequiredReadAction
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {
Comment on lines 59 to 61

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this file is already being touched, consider whether the "Use static import" checkbox needs the same treatment — right now it is unreachable from the UI:

  • KEY is KeyWithDefaultValue.create("java-use-static-import", false) and this class does not override isVisible, so the platform default applies: template.containsOption(KEY) → TemplateImpl.myOptions.containsKey("java-use-static-import").
  • TemplateListPanel.createOptions(...) only records processors for which isVisible(template) is true, and TemplateImpl.applyOptions(...) writes back only those recorded entries — so an invisible option is never persisted, and containsOption can never flip to true.
  • No bundled template in this repo carries java-use-static-import (grep finds the key only in this file), so the checkbox is never rendered and the feature can never be switched on by a user.

JB solves this with a context-aware override (ShortenToStaticImportProcessor#isVisible(Template, TemplateContext) returns true when a JavaCodeContextType/JavaCommentContextType is enabled). Consulo's isVisible takes only Template, so the closest available equivalent is an unconditional true — same trade-off as ShortenFQNamesProcessor in this PR (it becomes visible for non-Java templates too, but AddSingleMemberStaticImportAction/AddOnDemandStaticImportAction return null for non-Java elements, so processText stays a no-op there).

Suggested change
@Override
@RequiredReadAction
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {
@Override
public boolean isVisible(Template template) {
return true;
}
@Override
@RequiredReadAction
public void processText(Project project, Template template, Document document, RangeMarker templateRange, Editor editor) {

If you would rather keep this PR narrowly scoped to the JB sync, that is fine — but then it is worth filing a follow-up, because the option is currently dead UI.

PsiDocumentManager.getInstance(project).commitDocument(document);
PsiFile file = PsiUtilBase.getPsiFileInEditor(editor, project);
if (file == null) {
return;
}

List<Pair<PsiElement, StaticImporter>> staticImportTargets = new ArrayList<Pair<PsiElement, StaticImporter>>();
List<Pair<PsiElement, StaticImporter>> staticImportTargets = new ArrayList<>();
for (
PsiElement element = PsiUtilCore.getElementAtOffset(file, templateRange.getStartOffset());
element != null && element.getTextRange().getStartOffset() < templateRange.getEndOffset();
element = getNext(element)) {
for (StaticImporter importer : IMPORTERS) {
if (importer.canPerform(element)) {
staticImportTargets.add(new Pair<PsiElement, StaticImporter>(element, importer));
staticImportTargets.add(Pair.create(element, importer));
break;
}
}
Expand All @@ -85,6 +86,7 @@ public void processText(Project project, Template template, Document document, R
}

@Nullable
@RequiredReadAction
private static PsiElement getNext(PsiElement element) {
PsiElement result = element.getNextSibling();
for (PsiElement current = element; current != null && result == null; current = current.getParent()) {
Expand All @@ -106,6 +108,7 @@ private interface StaticImporter {

private static class SingleMemberStaticImporter implements StaticImporter {
@Override
@RequiredReadAction
public boolean canPerform(PsiElement element) {
return AddSingleMemberStaticImportAction.getStaticImportClass(element) != null;
}
Expand Down
Loading