From e748b7b26526cd1d7f1fe9dc1553015293c509c0 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Thu, 6 Jun 2024 16:24:15 +0100 Subject: [PATCH] Fix soft wrap inconsistency in run console IntelliJ has multiple soft wrap options. One for main editors, another for consoles and a third for previews. This can lead to inconsistencies if initialising a console based on a main editor when both have default values, versus the same scenario when the main editor has an explicit value. Furthermore, the run console's soft-wraps toggle button uses the global value, so can get out of sync if the local value is initialised to an explicit value. This change will only copy the soft wrap value over during initialisation for similar editors (main editor, preview, diff) and not for different editors (console). Fixes VIM-3450 --- .../maddyhome/idea/vim/group/OptionGroup.kt | 62 ++++++++++++++++--- .../idea/vim/api/VimOptionGroupBase.kt | 56 ++++++++++++++--- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt b/src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt index ce05377382..9afb485cb4 100644 --- a/src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt +++ b/src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt @@ -12,10 +12,12 @@ import com.intellij.application.options.CodeStyle import com.intellij.codeStyle.AbstractConvertLineSeparatorsAction import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.editor.EditorKind import com.intellij.openapi.editor.EditorSettings.LineNumerationType import com.intellij.openapi.editor.ScrollPositionCalculator import com.intellij.openapi.editor.ex.EditorEx import com.intellij.openapi.editor.ex.EditorSettingsExternalizable +import com.intellij.openapi.editor.impl.softwrap.SoftWrapAppliancePlaces import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.fileEditor.FileEditorManagerEvent import com.intellij.openapi.fileEditor.TextEditor @@ -1228,23 +1230,63 @@ private class WrapOptionMapper(wrapOption: ToggleOption, internalOptionValueAcce setIsUseSoftWraps(editor, value.asBoolean()) } + override fun canInitialiseOptionFrom(sourceEditor: VimEditor, targetEditor: VimEditor): Boolean { + // IntelliJ's soft-wrap settings are based on editor kind, so there can be different wrap options for consoles, + // main editors, etc. This is particularly noticeable in the console when running an application. The main editor + // might have the Vim default with line wrap enabled. Initialising the run console will also have a default value, + // and won't be updated by the options subsystem. It might have wrap enabled or not. If the editors were the same + // kind, the same default value would be used. + // However, if the main editor has 'wrap' explicitly set, this value is copied to the console, so the behaviour is + // inconsistent. Furthermore, the run console has a soft-wraps toggle button that works at the global level, and + // IdeaVim only sets the local value, so the toggle button can be inconsistent too. + // By denying copying the main editor value during initialisation, the console gets the default value, and the IDE + // decides what it should be. The behaviour is now more consistent. + // We're happy to initialise diff editors from main editors, as there isn't a different soft wrap setting there. + // Preview tabs might also have different settings, but because they're a type of main editor, it doesn't matter + // so much + fun editorKindToSoftWrapAppliancesPlace(kind: EditorKind) = when (kind) { + EditorKind.UNTYPED, + EditorKind.DIFF, + EditorKind.MAIN_EDITOR -> SoftWrapAppliancePlaces.MAIN_EDITOR + EditorKind.CONSOLE -> SoftWrapAppliancePlaces.CONSOLE + // Treat PREVIEW as a kind of MAIN_EDITOR instead of SWAP.PREVIEW. There are fewer noticeable differences + EditorKind.PREVIEW -> SoftWrapAppliancePlaces.MAIN_EDITOR + } + + val sourceKind = editorKindToSoftWrapAppliancesPlace(sourceEditor.ij.editorKind) + val targetKind = editorKindToSoftWrapAppliancesPlace(targetEditor.ij.editorKind) + return sourceKind == targetKind + } + + private fun getGlobalIsUseSoftWraps(editor: VimEditor): Boolean { + val softWrapAppliancePlace = when (editor.ij.editorKind) { + EditorKind.UNTYPED, + EditorKind.DIFF, + EditorKind.MAIN_EDITOR -> SoftWrapAppliancePlaces.MAIN_EDITOR + EditorKind.CONSOLE -> SoftWrapAppliancePlaces.CONSOLE + EditorKind.PREVIEW -> SoftWrapAppliancePlaces.PREVIEW + } + val settings = EditorSettingsExternalizable.getInstance() - if (settings.isUseSoftWraps) { - val masks = settings.softWrapFileMasks - if (masks.trim() == "*") return true - - editor.ij.virtualFile?.let { file -> - masks.split(";").forEach { mask -> - val trimmed = mask.trim() - if (trimmed.isNotEmpty() && PatternUtil.fromMask(trimmed).matcher(file.name).matches()) { - return true + if (softWrapAppliancePlace == SoftWrapAppliancePlaces.MAIN_EDITOR) { + if (settings.isUseSoftWraps) { + val masks = settings.softWrapFileMasks + if (masks.trim() == "*") return true + + editor.ij.virtualFile?.let { file -> + masks.split(";").forEach { mask -> + val trimmed = mask.trim() + if (trimmed.isNotEmpty() && PatternUtil.fromMask(trimmed).matcher(file.name).matches()) { + return true + } } } } + return false } - return false + return settings.isUseSoftWraps(softWrapAppliancePlace) } private fun getEffectiveIsUseSoftWraps(editor: VimEditor) = editor.ij.settings.isUseSoftWraps diff --git a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimOptionGroupBase.kt b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimOptionGroupBase.kt index d9f6ed726f..b428dd7283 100644 --- a/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimOptionGroupBase.kt +++ b/vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimOptionGroupBase.kt @@ -372,6 +372,14 @@ public interface LocalOptionValueOverride : OptionValueOverride * changed, not if the value has changed from [OptionValue.Default] to [OptionValue.User]. */ public fun setLocalValue(storedValue: OptionValue?, newValue: OptionValue, editor: VimEditor): Boolean + + /** + * Allows the override to veto copying the option value from the source editor to the target editor + * + * This is required for the `'wrap'` option in IntelliJ IDEs, because IntelliJ has different options for different + * editor kinds which can lead to unexpected results when trying to initialise a console editor from a main editor. + */ + public fun canInitialiseOptionFrom(sourceEditor: VimEditor, targetEditor: VimEditor): Boolean = true } /** @@ -924,6 +932,32 @@ private class OptionStorage { } } + fun getOptionValueForInitialisation( + option: Option, + sourceScope: OptionAccessScope, + targetScope: OptionAccessScope, + ): OptionValue { + fun getScopeEditor(scope: OptionAccessScope) = when (scope) { + is OptionAccessScope.EFFECTIVE -> null // Should never happen + is OptionAccessScope.GLOBAL -> scope.editor + is OptionAccessScope.LOCAL -> scope.editor + } + + // If this option is a local option, and has an override, give it the chance to veto initialising the option from + // the source editor. If it's not a local option, getLocalOptionValueOverride will return null. + // This is currently required to handle 'wrap' in IntelliJ, which stores different values based on editor kind, so + // we don't initialise a console editor with defaults/current values from a main editor + val sourceEditor = getScopeEditor(sourceScope) + val targetEditor = getScopeEditor(targetScope) + return if (sourceEditor != null && targetEditor != null + && getLocalOptionValueOverride(option)?.canInitialiseOptionFrom(sourceEditor, targetEditor) == false) { + OptionValue.Default(option.defaultValue) + } + else { + getOptionValue(option, sourceScope) + } + } + fun isOptionStorageInitialised(editor: VimEditor): Boolean { // Local window option storage will exist if we've previously initialised this editor return injector.vimStorageService.getDataFromWindow(editor, localOptionsKey) != null @@ -1212,7 +1246,7 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { val sourceGlobalScope = OptionAccessScope.GLOBAL(sourceEditor) val targetGlobalScope = OptionAccessScope.GLOBAL(targetEditor) forEachOption(LOCAL_TO_WINDOW) { option -> - val value = getOptionValueForInitialisation(option, sourceGlobalScope) + val value = getOptionValueForInitialisation(option, sourceGlobalScope, targetGlobalScope) storage.setOptionValue(option, targetGlobalScope, value) } } @@ -1222,7 +1256,7 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { val globalScope = OptionAccessScope.GLOBAL(editor) val localScope = OptionAccessScope.LOCAL(editor) forEachOption(LOCAL_TO_BUFFER) { option -> - val value = getOptionValueForInitialisation(option, globalScope) + val value = getOptionValueForInitialisation(option, globalScope, localScope) storage.setOptionValue(option, localScope, value) } forEachOption(GLOBAL_OR_LOCAL_TO_BUFFER) { option -> @@ -1235,11 +1269,11 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { val sourceLocalScope = OptionAccessScope.LOCAL(sourceEditor) val targetLocalScope = OptionAccessScope.LOCAL(targetEditor) forEachOption(LOCAL_TO_BUFFER) { option -> - val value = getOptionValueForInitialisation(option, sourceLocalScope) + val value = getOptionValueForInitialisation(option, sourceLocalScope, targetLocalScope) storage.setOptionValue(option, targetLocalScope, value) } forEachOption(GLOBAL_OR_LOCAL_TO_BUFFER) { option -> - val value = getOptionValueForInitialisation(option, sourceLocalScope) + val value = getOptionValueForInitialisation(option, sourceLocalScope, targetLocalScope) storage.setOptionValue(option, targetLocalScope, value) } } @@ -1248,7 +1282,7 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { val globalScope = OptionAccessScope.GLOBAL(editor) val localScope = OptionAccessScope.LOCAL(editor) forEachOption(LOCAL_TO_WINDOW) { option -> - val value = getOptionValueForInitialisation(option, globalScope) + val value = getOptionValueForInitialisation(option, globalScope, localScope) storage.setOptionValue(option, localScope, value) } forEachOption(GLOBAL_OR_LOCAL_TO_WINDOW) { option -> @@ -1265,11 +1299,11 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { val sourceLocalScope = OptionAccessScope.LOCAL(sourceEditor) val targetLocalScope = OptionAccessScope.LOCAL(targetEditor) forEachOption(LOCAL_TO_WINDOW) { option -> - val value = getOptionValueForInitialisation(option, sourceLocalScope) + val value = getOptionValueForInitialisation(option, sourceLocalScope, targetLocalScope) storage.setOptionValue(option, targetLocalScope, value) } forEachOption(GLOBAL_OR_LOCAL_TO_WINDOW) { option -> - val value = getOptionValueForInitialisation(option, sourceLocalScope) + val value = getOptionValueForInitialisation(option, sourceLocalScope, targetLocalScope) storage.setOptionValue(option, targetLocalScope, value) } } @@ -1291,12 +1325,16 @@ private class OptionInitialisationStrategy(private val storage: OptionStorage) { * value is always set to default, which will use the current value detected by the IDE. * See `:help local-noglobal` */ - private fun getOptionValueForInitialisation(option: Option, scope: OptionAccessScope): OptionValue { + private fun getOptionValueForInitialisation( + option: Option, + sourceScope: OptionAccessScope, + targetScope: OptionAccessScope, + ): OptionValue { return if (option.isLocalNoGlobal) { OptionValue.Default(option.defaultValue) } else { - storage.getOptionValue(option, scope) + storage.getOptionValueForInitialisation(option, sourceScope, targetScope) } } }