Skip to content

Commit

Permalink
Fix soft wrap inconsistency in run console
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@citizenmatt @AlexPl292
citizenmatt authored and AlexPl292 committed Jun 28, 2024
1 parent c2401ec commit e748b7b
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 19 deletions.
62 changes: 52 additions & 10 deletions src/main/java/com/maddyhome/idea/vim/group/OptionGroup.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
56 changes: 47 additions & 9 deletions vim-engine/src/main/kotlin/com/maddyhome/idea/vim/api/VimOptionGroupBase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,14 @@ public interface LocalOptionValueOverride<T : VimDataType> : OptionValueOverride
* changed, not if the value has changed from [OptionValue.Default] to [OptionValue.User].
*/
public fun setLocalValue(storedValue: OptionValue<T>?, newValue: OptionValue<T>, 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
}

/**
Expand Down Expand Up @@ -924,6 +932,32 @@ private class OptionStorage {
}
}

fun <T : VimDataType> getOptionValueForInitialisation(
option: Option<T>,
sourceScope: OptionAccessScope,
targetScope: OptionAccessScope,
): OptionValue<T> {
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
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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 ->
Expand All @@ -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)
}
}
Expand All @@ -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 ->
Expand All @@ -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)
}
}
Expand All @@ -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<VimDataType>, scope: OptionAccessScope): OptionValue<VimDataType> {
private fun getOptionValueForInitialisation(
option: Option<VimDataType>,
sourceScope: OptionAccessScope,
targetScope: OptionAccessScope,
): OptionValue<VimDataType> {
return if (option.isLocalNoGlobal) {
OptionValue.Default(option.defaultValue)
}
else {
storage.getOptionValue(option, scope)
storage.getOptionValueForInitialisation(option, sourceScope, targetScope)
}
}
}
Expand Down

0 comments on commit e748b7b

Please sign in to comment.