Skip to content
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

feat: codeBlockProvider improvements #716

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
92 changes: 92 additions & 0 deletions src/main/java/com/redhat/devtools/lsp4ij/LSPIJTextMateUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at http://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/

package com.redhat.devtools.lsp4ij;

import com.intellij.psi.PsiFile;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.Map;

/**
* Utilities for working with TextMate files in LSP4IJ.
*/
@ApiStatus.Internal
public final class LSPIJTextMateUtils {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new class for working with TextMate-specific stuff. Right now it's primarily for deriving the language's brace pairs, but as stated in the PR description, I can't do that in a way that works across all supported IDE versions due to a breaking change in TextMateBracePair back in September, 2023:

JetBrains/intellij-community@8df3d04#diff-08fc4fd41510ee4662c41d3f2a671ae2f654d1a2f6ff7608765f427c26eaeae7

Well, I can make it work via reflection, but I'm not sure if it's worth going that far. I guess the options here are:

  1. Leave it like this until the older IDE versions are no longer supported, then uncomment this and it will just work.
  2. Remove this commented code and TextMate support can be reimplemented when incompatible older versions are no longer supported.
  3. Use reflection to support both older and newer versions until older verions are no longer supported.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that I've hit this exact same issue with 2023.2 in another PR that I'll be submitting shortly. Any chance that you're thinking about dropping support for 2023.2 anytime soon? That would definitely help with some TextMate-related stuff because of these changes between 2023.2 and 2023.3+.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok let upgrade that.

Please do that in this PR or an another PR and please update the README also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok let upgrade that.

Please do that in this PR or an another PR and please update the README also.

That upgrade could be a non-trivial amount of work based on whether tests, etc., need to be updated. Let's keep that separate from this work, and I'm happy to do a follow-on PR that enables this logic once LSP4IJ no longer supports 2023.2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For what it's worth, I just tried to update the build for 2023.3 instead of 2023.2 in the context of this PR, and while I can build and run the plugin, the tests fail with:

Execution failed for task ':instrumentCode'.
> taskdef class com.intellij.ant.InstrumentIdeaExtensions cannot be found
   using the classloader AntClassLoader[]

It's possible that the build needs to be updated for the Gradle 2.0 plugin SDK to target higher versions. Either way, it seems like that upgrade should be handled as its own PR.


private LSPIJTextMateUtils() {
// Pure utility class
}

/**
* Returns the simple/single-character brace pairs for the file if it's a TextMate file.
*
* @param file the PSI file
* @return the simple brace pairs for the file if it's a TextMate file; otherwise null
*/
@Nullable
@ApiStatus.Internal
public static Map<Character, Character> getBracePairs(@NotNull PsiFile file) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be removed, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's up to you. I'd hate to lose it if LSP4IJ is going to be updated to drop support for 2023.2 soon and it should be restored, and it would likely be lost through a squash merge. If you'd like it removed since it's a no-op in its current form, I can do so, but it should hopefully only be commented out for a short time until 2023.2 support is dropped. Let me know what you'd prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thanks for the clarification. Lets keep like this.

// TODO: Unfortunately the interface changed in this commit:
// https://github.com/JetBrains/intellij-community/commit/8df3d04be0db4c54732a15250b789aa5d9a6de47#diff-08fc4fd41510ee4662c41d3f2a671ae2f654d1a2f6ff7608765f427c26eaeae7
// and would now require reflection to work in 2023.2 and later versions. Specifically it used to be
// "bracePair.left/right" which returned "char", but now it's "bracePair.getLeft()/getRight()" which return
// "CharSequence". I think it's worth leaving this in but commented out and returning "null" -- existing usages
// will degrade gracefully -- and then when all supported IDE versions have the same interface, this can be
// restored. Perhaps this even prompts removal of support for the oldest versions that have this issue?
return null;

/*
if (!(file instanceof TextMateFile)) {
return null;
}

Map<Character, Character> bracePairs = new LinkedHashMap<>();
Editor editor = LSPIJUtils.editorForElement(file);
TextMateScope selector = editor instanceof EditorEx ? TextMateEditorUtils.getCurrentScopeSelector((EditorEx) editor) : null;
if (selector != null) {
for (TextMateBracePair bracePair : getAllPairsForMatcher(selector)) {
CharSequence openBrace = bracePair.getLeft();
CharSequence closeBrace = bracePair.getRight();
if ((openBrace.length() == 1) && (closeBrace.length() == 1)) {
bracePairs.put(openBrace.charAt(0), closeBrace.charAt(0));
}
}
}
return bracePairs;
*/
}

/*
// NOTE: Cloned from TextMateEditorUtils where this is private
@NotNull
private static Set<TextMateBracePair> getAllPairsForMatcher(@Nullable TextMateScope selector) {
if (selector == null) {
return Constants.DEFAULT_HIGHLIGHTING_BRACE_PAIRS;
}
Set<TextMateBracePair> result = new HashSet<>();
List<Preferences> preferencesForSelector = TextMateService.getInstance().getPreferenceRegistry().getPreferences(selector);
for (Preferences preferences : preferencesForSelector) {
final Set<TextMateBracePair> highlightingPairs = preferences.getHighlightingPairs();
if (highlightingPairs != null) {
if (highlightingPairs.isEmpty()) {
// smart typing pairs can be defined in preferences but can be empty (in order to disable smart typing completely)
return Collections.emptySet();
}
result.addAll(highlightingPairs);
}
}
return result;
}
*/
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,251 @@
/*******************************************************************************
* Copyright (c) 2024 Red Hat, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2025

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class already existed in 2024 and was moved across packages. It should still be 2024.

* Distributed under license by Red Hat, Inc. All rights reserved.
* This program is made available under the terms of the
* Eclipse Public License v2.0 which accompanies this distribution,
* and is available at http://www.eclipse.org/legal/epl-v20.html
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
******************************************************************************/

package com.redhat.devtools.lsp4ij.features.codeBlockProvider;

import com.intellij.codeInsight.editorActions.CodeBlockProvider;
import com.intellij.openapi.editor.Document;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.PsiFile;
import com.intellij.util.containers.ContainerUtil;
import com.redhat.devtools.lsp4ij.features.foldingRange.LSPFoldingRangeBuilder;
import com.redhat.devtools.lsp4ij.features.selectionRange.LSPSelectionRangeSupport;
import org.eclipse.lsp4j.FoldingRange;
import org.eclipse.lsp4j.SelectionRange;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.*;

/**
* Code block provider that uses information from {@link LSPSelectionRangeSupport} and {@link LSPFoldingRangeBuilder}.
*/
public class LSPCodeBlockProvider implements CodeBlockProvider {
Copy link
Contributor Author

@SCWells72 SCWells72 Dec 20, 2024

Choose a reason for hiding this comment

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

Now that this is no longer strictly tied to the folding range feature -- which it still uses, but alongside the selection range feature now -- so I moved it into its own package. Unfortunately there are enough other changes that Git doesn't see it as a move so there's not a good before/after diff here.


@Override
@Nullable
public TextRange getCodeBlockRange(Editor editor, PsiFile file) {
Document document = editor.getDocument();
CharSequence documentChars = document.getCharsSequence();
int documentLength = documentChars.length();

// Adjust the offset slightly based on before/after brace to ensure evaluation occurs "within" the braced block
int offset = editor.getCaretModel().getOffset();
Character beforeCharacter = offset > 0 ? documentChars.charAt(offset - 1) : null;
Character afterCharacter = offset < documentLength ? documentChars.charAt(offset) : null;
if (LSPCodeBlockUtils.isCodeBlockStartChar(file, afterCharacter)) {
offset++;
} else if (LSPCodeBlockUtils.isCodeBlockEndChar(file, beforeCharacter)) {
offset--;
}

// See if we're anchored by a known brace character
int openBraceOffset = -1;
Character openBraceChar = null;
int closeBraceOffset = -1;
Character closeBraceChar = null;
if ((offset > 0) && LSPCodeBlockUtils.isCodeBlockStartChar(file, documentChars.charAt(offset - 1))) {
openBraceOffset = offset - 1;
openBraceChar = documentChars.charAt(offset - 1);
closeBraceChar = LSPCodeBlockUtils.getCodeBlockEndChar(file, openBraceChar);
} else if (LSPCodeBlockUtils.isCodeBlockEndChar(file, documentChars.charAt(offset))) {
closeBraceOffset = offset;
closeBraceChar = documentChars.charAt(offset);
openBraceChar = LSPCodeBlockUtils.getCodeBlockStartChar(file, closeBraceChar);
} else if ((offset < (documentLength - 1)) && LSPCodeBlockUtils.isCodeBlockEndChar(file, documentChars.charAt(offset + 1))) {
closeBraceOffset = offset + 1;
closeBraceChar = documentChars.charAt(offset + 1);
openBraceChar = LSPCodeBlockUtils.getCodeBlockStartChar(file, closeBraceChar);
}

// Try to find it first using the selection ranges which tend to be more accurate; we must use the effective
// offset for selection ranges to act as if we're in the adjusted braced block
TextRange codeBlockRange = getUsingSelectionRanges(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Selection ranges from the current offset generally provide more detailed code block information, so try finding the closest containing block using that feature first.

file,
editor,
offset,
openBraceChar,
openBraceOffset,
closeBraceChar,
closeBraceOffset
);
if (codeBlockRange != null) {
return codeBlockRange;
}

// Failing that, try to find it using the folding ranges
return getUsingFoldingRanges(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we couldn't find a good code block from the selection ranges, try to find one using the folding ranges.

file,
document,
offset,
openBraceChar,
openBraceOffset,
closeBraceChar,
closeBraceOffset
);
}

@Nullable
private TextRange getUsingSelectionRanges(@NotNull PsiFile file,
@NotNull Editor editor,
int offset,
@Nullable Character openBraceChar,
int openBraceOffset,
@Nullable Character closeBraceChar,
int closeBraceOffset) {
Document document = editor.getDocument();
List<SelectionRange> selectionRanges = LSPSelectionRangeSupport.getSelectionRanges(file, document, offset);
if (!ContainerUtil.isEmpty(selectionRanges)) {
// Convert the selection ranges into text ranges
Set<TextRange> textRanges = new LinkedHashSet<>(selectionRanges.size());
for (SelectionRange selectionRange : selectionRanges) {
textRanges.add(LSPSelectionRangeSupport.getTextRange(selectionRange, document));
for (SelectionRange parentSelectionRange = selectionRange.getParent();
parentSelectionRange != null;
parentSelectionRange = parentSelectionRange.getParent()) {
textRanges.add(LSPSelectionRangeSupport.getTextRange(parentSelectionRange, document));
}
}

CharSequence documentChars = document.getCharsSequence();
int documentLength = documentChars.length();

// Find containing text ranges that are bounded by brace pairs
List<TextRange> containingTextRanges = new ArrayList<>(textRanges.size());
for (TextRange textRange : textRanges) {
if (textRange.getLength() > 1) {
int startOffset = textRange.getStartOffset();
int endOffset = textRange.getEndOffset();

char startChar = documentChars.charAt(startOffset);
char endChar = documentChars.charAt(endOffset - 1);

// If aligned on an open brace and this ends with the expected close brace, use it
if ((startOffset == openBraceOffset)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first two conditions here are for when the caret starts exactly on an open or close brace(/bracket/paren) and the evaluated range begins/ends on that offset and ends/begins with the appropriate paired brace. Basically these are an exact brace match.

if ((closeBraceChar != null) && (closeBraceChar == endChar)) {
return textRange;
} else {
return null;
}
}
// If aligned on a close brace and this starts with the expected open brace, use it
else if (((endOffset - 1) == closeBraceOffset)) {
if ((openBraceChar != null) && (openBraceChar == startChar)) {
return textRange;
} else {
return null;
}
}
// Otherwise see if it starts and ends with a known brace pair and we'll find the "closest" below
else if ((openBraceOffset == -1) && (closeBraceOffset == -1) && LSPCodeBlockUtils.isCodeBlockStartChar(file, startChar)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise we see whether or not the selection range starts and ends with a supported brace pair and, if so, we add it to the list of containing code blocks.

Character pairedCloseBraceChar = LSPCodeBlockUtils.getCodeBlockEndChar(file, startChar);
if ((pairedCloseBraceChar != null) && (pairedCloseBraceChar == endChar)) {
containingTextRanges.add(textRange);
}
}
// Also try to see if these are exactly the contents of a code block
else if ((openBraceOffset == -1) && (closeBraceOffset == -1) && (startOffset > 0) && (endOffset < documentLength)) {
startChar = documentChars.charAt(startOffset - 1);
endChar = documentChars.charAt(endOffset);

if (LSPCodeBlockUtils.isCodeBlockStartChar(file, startChar)) {
Character pairedCloseBraceChar = LSPCodeBlockUtils.getCodeBlockEndChar(file, startChar);
if ((pairedCloseBraceChar != null) && (pairedCloseBraceChar == endChar)) {
containingTextRanges.add(textRange);
}
}
}
}
}

// Return the closest (i.e., smallest) containing text range
if (!ContainerUtil.isEmpty(containingTextRanges)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And then we return the smallest containing code block if at least one was found.

containingTextRanges.sort(Comparator.comparingInt(TextRange::getLength));
return ContainerUtil.getFirstItem(containingTextRanges);
}
}

return null;
}

@Nullable
private static TextRange getUsingFoldingRanges(@NotNull PsiFile file,
@NotNull Document document,
int offset,
@Nullable Character openBraceChar,
int openBraceOffset,
@Nullable Character closeBraceChar,
int closeBraceOffset) {
List<FoldingRange> foldingRanges = LSPFoldingRangeBuilder.getFoldingRanges(file);
Copy link
Contributor Author

@SCWells72 SCWells72 Dec 20, 2024

Choose a reason for hiding this comment

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

This has been updated to behave similar to how we process selection ranges.

if (!ContainerUtil.isEmpty(foldingRanges)) {
CharSequence documentChars = document.getCharsSequence();
int documentLength = documentChars.length();

List<TextRange> containingTextRanges = new ArrayList<>(foldingRanges.size());
for (FoldingRange foldingRange : foldingRanges) {
TextRange textRange = LSPFoldingRangeBuilder.getTextRange(
foldingRange,
file,
document,
openBraceChar,
closeBraceChar
);
if ((textRange != null) && (textRange.getLength() > 1)) {
int startOffset = Math.max(0, textRange.getStartOffset() - 1);
int endOffset = Math.min(documentLength - 1, textRange.getEndOffset());
// These ranges can tend to add whitespace at the end, so trim that before looking for braces
while ((endOffset > startOffset) && Character.isWhitespace(documentChars.charAt(endOffset))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We sometimes have to perform a pseudo-"trim" on the tail of the folding range to find the closing brace.

endOffset--;
}

char startChar = documentChars.charAt(startOffset);
char endChar = documentChars.charAt(endOffset);

// If aligned on an open brace and this ends with the expected close brace, use it
if ((startOffset == openBraceOffset)) {
if ((closeBraceChar != null) && (closeBraceChar == endChar)) {
return textRange;
} else {
return null;
}
}
// If aligned on a close brace and this starts with the expected open brace, use it
else if ((endOffset == closeBraceOffset)) {
if ((openBraceChar != null) && (openBraceChar == startChar)) {
return textRange;
} else {
return null;
}
}
// Otherwise see if it starts and ends with a known brace pair and we'll find the "closest" below
else if (textRange.containsOffset(offset) &&
(openBraceOffset == -1) &&
(closeBraceOffset == -1) &&
LSPCodeBlockUtils.isCodeBlockStartChar(file, startChar)) {
Character pairedCloseBraceChar = LSPCodeBlockUtils.getCodeBlockEndChar(file, startChar);
if ((pairedCloseBraceChar != null) && (pairedCloseBraceChar == endChar)) {
containingTextRanges.add(textRange);
}
}
}
}

// Return the closest (i.e., smallest) containing text range
if (!ContainerUtil.isEmpty(containingTextRanges)) {
containingTextRanges.sort(Comparator.comparingInt(TextRange::getLength));
return ContainerUtil.getFirstItem(containingTextRanges);
}
}

return null;
}
}
Loading
Loading