perf: Simplify access to client config information (fixes #912) #925
Conversation
…client config aspects in a single call/iteration of the language servers.
| * Reference to a boolean value that simplifies mutation of that value in lambdas/inner classes. | ||
| */ | ||
| @ApiStatus.Internal | ||
| public class BooleanRef extends MutableRef<Boolean> { |
There was a problem hiding this comment.
Several mutable reference types to provide indirect access to a mutable value of the respective simple types in lambdas. For boolean values we want to be able to perform logical ANDs and ORs; for string values we want to be able to append; for enumerated constant values we want to be able to retain the highest/lowest-ordered value.
| * @param file the file | ||
| * @param processor the processor | ||
| */ | ||
| public void processLanguageServers(@NotNull PsiFile file, |
There was a problem hiding this comment.
This allows the caller to apply some logic to all started language servers.
| * | ||
| * @param <T> the value type | ||
| */ | ||
| class MutableRef<T> extends Ref<T> { |
There was a problem hiding this comment.
Base class for the above mutable ref types.
| * @return the value | ||
| */ | ||
| @Nullable | ||
| public T getValue() { |
There was a problem hiding this comment.
We have this so that subclasses can change from @Nullable to @NotNull as appropriate. Unfortunately get() is final.
| closeBraceChar | ||
| ); | ||
| // Gather all of the relevant client configuration | ||
| BooleanRef rangeFormattingSupportedRef = BooleanRef.create(); |
There was a problem hiding this comment.
Create mutable refs for all the info we need.
...om/redhat/devtools/lsp4ij/features/formatting/LSPClientSideOnTypeFormattingTypedHandler.java
Outdated
Show resolved
Hide resolved
| // Statement terminators | ||
| if (formattingFeature.isFormatOnStatementTerminator(file) && | ||
| ); | ||
| boolean rangeFormattingSupported = rangeFormattingSupportedRef.getValue(); |
There was a problem hiding this comment.
Dereference all of the mutable references into variables with simplified names.
| String formatOnCompletionTriggerCharacters = formatOnCompletionTriggerCharactersRef.getValue(); | ||
|
|
||
| // Close braces | ||
| if (formatOnCloseBrace && |
There was a problem hiding this comment.
And the rest of the updates are just to use those collected settings. For the best diff, hide whitespace-only differences.
| project, | ||
| editor, | ||
| file, | ||
| formattingFeature, | ||
| charTyped | ||
| formatOnCloseBraceScope, |
There was a problem hiding this comment.
We were only passing formattingFeature to get to formatOnCloseBraceScope, so just pass that scope.
| project, | ||
| editor, | ||
| file, | ||
| formatOnStatementTerminatorScope, |
There was a problem hiding this comment.
We were only passing formattingFeature to get to formatOnStatementTerminatorScope, so just pass that scope.
| } | ||
| } | ||
|
|
||
| return super.charTyped(charTyped, project, editor, file); | ||
| } | ||
|
|
||
| @Nullable |
There was a problem hiding this comment.
Now we can remove this.
| @@ -330,14 +330,4 @@ private static void format(@NotNull Project project, | |||
| } | |||
| } | |||
|
|
|||
| private static boolean hasLanguageServerSupportingOnlyFormatting(@NotNull PsiFile file) { | |||
There was a problem hiding this comment.
And we can remove this.
| // Statement terminators | ||
| if (formattingFeature.isFormatOnStatementTerminator(file) && | ||
| // Close braces | ||
| if (onTypeFormattingSettings.formatOnCloseBrace && |
There was a problem hiding this comment.
And everything below here is just updating to use that collected info. I recommend hiding whitespace-only changes for the best diff.
| project, | ||
| editor, | ||
| file, | ||
| formattingFeature, | ||
| charTyped | ||
| onTypeFormattingSettings.formatOnCloseBraceScope, |
There was a problem hiding this comment.
We were previously passing formattingFeature here just to get the formatting scope, so now we just pass the formatting scope directly.
| project, | ||
| editor, | ||
| file, | ||
| onTypeFormattingSettings.formatOnStatementTerminatorScope, |
There was a problem hiding this comment.
Same as the above change to pass the scope instead of the formatting feature.
| @@ -103,7 +103,7 @@ public static class ClientSideOnTypeFormattingSettings { | |||
| * The specific close brace characters that should trigger client-side on-type formatting. Defaults to the | |||
| * language's close brace characters. | |||
| */ | |||
| public String formatOnCloseBraceCharacters = null; | |||
| public String formatOnCloseBraceCharacters = ""; | |||
There was a problem hiding this comment.
Changed these to default to the empty string instead of null. Downstream consumers are just checking for an empty or null string, so it's a no-op other than that we don't have to check for null when collecting these from language server definitions before appending.
...om/redhat/devtools/lsp4ij/features/formatting/LSPClientSideOnTypeFormattingTypedHandler.java
Outdated
Show resolved
Hide resolved
| LSPClientFeatures clientFeatures = ls.getClientFeatures(); | ||
| LSPFormattingFeature formattingFeature = clientFeatures.getFormattingFeature(); | ||
| LSPOnTypeFormattingFeature onTypeFormattingFeature = clientFeatures.getOnTypeFormattingFeature(); | ||
| if (formattingFeature.isEnabled(file) && formattingFeature.isSupported(file) && |
There was a problem hiding this comment.
All of these feature flags default to false/disabled, so if we don't find a language server that supports formatting but not server-side on-type formatting, everything below will be an effective no-op.
...om/redhat/devtools/lsp4ij/features/formatting/LSPClientSideOnTypeFormattingTypedHandler.java
Outdated
Show resolved
Hide resolved
|
Thanks @SCWells72 ! |
This implements the optimization suggested in #912 and updates client-side on-type formatting to use it.