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

Eagerly initialise local options and introduce option scope #667

Merged
Merged
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0c1856e
Introduce declared scope for all options
citizenmatt Apr 6, 2023
e19d7f2
Update parsed value cache for declared scope
citizenmatt Apr 20, 2023
78928c3
Support local-to-buffer options
citizenmatt Apr 7, 2023
78c219f
Override IjVimEditor.toString for better debugging
citizenmatt Apr 10, 2023
fcbac54
Use Vim terminology in storage service
citizenmatt Apr 20, 2023
3924ff3
Initialise options when opening windows/buffers
citizenmatt Apr 24, 2023
5553086
Introduce AUTO scope for effective option values
citizenmatt Apr 24, 2023
35f4c02
Reset options for current editor only
citizenmatt Apr 24, 2023
d8de218
Only notify change if option has changed
citizenmatt Apr 25, 2023
92eca54
Support global-local options
citizenmatt Apr 25, 2023
346038a
Add support for resetting option to global value
citizenmatt Apr 25, 2023
fdf7f8c
Format unset global-local toggle options
citizenmatt Apr 25, 2023
dc6c5fd
Use accessor API to set global-local value
citizenmatt Apr 26, 2023
ef51df7
Add :setglobal command
citizenmatt Apr 27, 2023
a899ac0
Fix option scopes for :let command
citizenmatt Apr 27, 2023
9db8b26
Simplify guicursor caret attributes cache
citizenmatt Apr 28, 2023
0c08791
Introduce listener for global option changes
citizenmatt Apr 28, 2023
c04e0fa
Add simple one to many collection
citizenmatt Apr 28, 2023
1abaf33
Introduce option effective value change listener
citizenmatt May 1, 2023
1555c54
Migrate to effective value change listeners
citizenmatt May 1, 2023
f545350
Remove old option listener API
citizenmatt May 1, 2023
e662a49
Fix tests under latest SDK
citizenmatt Jul 19, 2023
0b331d9
Minor updates from code review
citizenmatt Jul 29, 2023
1793f75
Refactor parseOptionLine for readability
citizenmatt Jul 29, 2023
f096c85
Rename OptionsScope.AUTO to EFFECTIVE
citizenmatt Jul 29, 2023
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
Prev Previous commit
Next Next commit
Support local-to-buffer options
Previously, all local options were treated as local-to-window
citizenmatt committed Jul 30, 2023
commit 78928c3e7e936384c2a6aa7add35303cdb824823
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
/*
* Copyright 2003-2023 The IdeaVim authors
*
* Use of this source code is governed by an MIT-style
* license that can be found in the LICENSE.txt file or at
* https://opensource.org/licenses/MIT.
*/

package org.jetbrains.plugins.ideavim.option

import com.intellij.openapi.editor.Editor
import com.intellij.openapi.fileEditor.FileEditorManager
import com.intellij.openapi.fileEditor.TextEditor
import com.intellij.openapi.fileEditor.ex.FileEditorManagerEx
import com.intellij.openapi.fileEditor.impl.FileEditorManagerImpl
import com.intellij.testFramework.replaceService
import com.intellij.util.childScope
import com.maddyhome.idea.vim.api.injector
import com.maddyhome.idea.vim.newapi.vim
import com.maddyhome.idea.vim.options.OptionDeclaredScope
import com.maddyhome.idea.vim.options.OptionScope
import com.maddyhome.idea.vim.options.StringOption
import com.maddyhome.idea.vim.vimscript.model.datatypes.VimString
import org.jetbrains.plugins.ideavim.SkipNeovimReason
import org.jetbrains.plugins.ideavim.TestWithoutNeovim
import org.jetbrains.plugins.ideavim.VimTestCase
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInfo
import javax.swing.SwingConstants
import kotlin.test.assertEquals

// Tests the implementation of global, local to buffer, local to window and global-local
@TestWithoutNeovim(reason = SkipNeovimReason.OPTION)
class OptionDeclaredScopeTest : VimTestCase() {
private val optionName = "test"
private val defaultValue = VimString("defaultValue")
private val localValue = VimString("localValue")
private lateinit var manager: FileEditorManagerImpl
private lateinit var originalEditor: Editor
private lateinit var newBuffer: Editor
private lateinit var newWindow: Editor

@BeforeEach
override fun setUp(testInfo: TestInfo) {
super.setUp(testInfo)

// Copied from FileEditorManagerTestCase to allow us to split windows
@Suppress("DEPRECATION")
manager = FileEditorManagerImpl(fixture.project, fixture.project.coroutineScope.childScope())
fixture.project.replaceService(FileEditorManager::class.java, manager, fixture.testRootDisposable)

// Create the new buffer before changing the value, or AUTO can write to both local and global values
val fileManager = FileEditorManagerEx.getInstanceEx(fixture.project)
val newFile = fixture.configureByText("bbb.txt", "new file")
newBuffer = (fileManager.openFile(newFile.virtualFile, false).first() as TextEditor).editor

// Create the original editor last, so that fixture.editor will point to this file
originalEditor = configureByText("\n")

newWindow = (fileManager.currentWindow!!.split(
SwingConstants.VERTICAL,
true,
originalEditor.virtualFile,
false
)!!.allComposites.first().selectedEditor as TextEditor).editor
}

@AfterEach
override fun tearDown(testInfo: TestInfo) {
super.tearDown(testInfo)
injector.optionGroup.removeOption(optionName)
}

@Test
fun `test global option affects all buffers and windows`() {
val option = StringOption(optionName, OptionDeclaredScope.GLOBAL, "test", defaultValue)
injector.optionGroup.addOption(option)

injector.optionGroup.setOptionValue(option, OptionScope.LOCAL(fixture.editor.vim), localValue)

// TODO: This should be AUTO
val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(localValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(localValue, newWindowValue)
}

@Test
fun `test buffer local option is different in new buffer, but same in new window`() {
val option = StringOption(optionName, OptionDeclaredScope.LOCAL_TO_BUFFER, "test", defaultValue)
injector.optionGroup.addOption(option)

injector.optionGroup.setOptionValue(option, OptionScope.LOCAL(fixture.editor.vim), localValue)

// TODO: This should be AUTO
val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(defaultValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(localValue, newWindowValue)
}

@Test
fun `test window local option is different in new buffer and new window`() {
val option = StringOption(optionName, OptionDeclaredScope.LOCAL_TO_WINDOW, "test", defaultValue)
injector.optionGroup.addOption(option)

// TODO: This should be AUTO
injector.optionGroup.setOptionValue(option, OptionScope.LOCAL(fixture.editor.vim), localValue)

val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(defaultValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(defaultValue, newWindowValue)
}

@Test
fun `test global-local with auto scope acts like global and affects all buffers and windows`() {
// It doesn't matter if we use local to buffer or window
val option = StringOption(optionName, OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_BUFFER, "test", defaultValue)
injector.optionGroup.addOption(option)

// TODO: This should be AUTO
// Using GLOBAL just sets the global value
injector.optionGroup.setOptionValue(option, OptionScope.GLOBAL, localValue)

val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(localValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(localValue, newWindowValue)
}

@Test
fun `test global or local to buffer option at local scope is different in new buffer, but same in new window`() {
val option = StringOption(optionName, OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_BUFFER, "test", defaultValue)
injector.optionGroup.addOption(option)

// Note that this should definitely be LOCAL! If we use AUTO, we get GLOBAL behaviour. There is a crossover with
// OptionScopeTests, but this is testing the behaviour of global-local, rather than the behaviour of AUTO
injector.optionGroup.setOptionValue(option, OptionScope.LOCAL(fixture.editor.vim), localValue)

val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(defaultValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(localValue, newWindowValue)
}

@Test
fun `test global or local to window option at local scope is different in new buffer and new window`() {
val option = StringOption(optionName, OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_WINDOW, "test", defaultValue)
injector.optionGroup.addOption(option)

// Note that this should definitely be LOCAL! If we use AUTO, we get GLOBAL behaviour. There is a crossover with
// OptionScopeTests, but this is testing the behaviour of global-local, rather than the behaviour of AUTO
injector.optionGroup.setOptionValue(option, OptionScope.LOCAL(fixture.editor.vim), localValue)

val newBufferValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newBuffer.vim))
assertEquals(defaultValue, newBufferValue)

val newWindowValue = injector.optionGroup.getOptionValue(option, OptionScope.LOCAL(newWindow.vim))
assertEquals(defaultValue, newWindowValue)
}
}
Original file line number Diff line number Diff line change
@@ -10,7 +10,11 @@ package com.maddyhome.idea.vim.api

import com.maddyhome.idea.vim.options.Option
import com.maddyhome.idea.vim.options.OptionChangeListener
import com.maddyhome.idea.vim.options.OptionDeclaredScope
import com.maddyhome.idea.vim.options.OptionDeclaredScope.GLOBAL
import com.maddyhome.idea.vim.options.OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_BUFFER
import com.maddyhome.idea.vim.options.OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_WINDOW
import com.maddyhome.idea.vim.options.OptionDeclaredScope.LOCAL_TO_BUFFER
import com.maddyhome.idea.vim.options.OptionDeclaredScope.LOCAL_TO_WINDOW
import com.maddyhome.idea.vim.options.OptionScope
import com.maddyhome.idea.vim.vimscript.model.datatypes.VimDataType

@@ -26,9 +30,17 @@ public abstract class VimOptionGroupBase : VimOptionGroup {
}

override fun <T : VimDataType> getOptionValue(option: Option<T>, scope: OptionScope): T {
return when (scope) {
is OptionScope.LOCAL -> getLocalOptionValue(option, scope.editor)
is OptionScope.GLOBAL -> getGlobalOptionValue(option)
val editor = when (scope) {
is OptionScope.LOCAL -> scope.editor
OptionScope.GLOBAL -> return getGlobalOptionValue(option)
}

return when (option.declaredScope) {
GLOBAL -> getGlobalOptionValue(option)
LOCAL_TO_BUFFER -> getBufferLocalOptionValue(option, editor)
LOCAL_TO_WINDOW -> getWindowLocalOptionValue(option, editor)
GLOBAL_OR_LOCAL_TO_BUFFER -> tryGetBufferLocalOptionValue(option, editor) ?: getGlobalOptionValue(option)
GLOBAL_OR_LOCAL_TO_WINDOW -> tryGetWindowLocalOptionValue(option, editor) ?: getGlobalOptionValue(option)
}
}

@@ -39,13 +51,26 @@ public abstract class VimOptionGroupBase : VimOptionGroup {
// We should introduce OptionScope.AUTO, that sets the option value at the appropriate scope - locally for local
// options and globally for global options. We would clear (and create) the parsed value only for AUTO
val oldValue = getOptionValue(option, scope)

when (scope) {
is OptionScope.LOCAL -> {
setLocalOptionValue(option.name, value, scope.editor)
injector.vimStorageService.getDataFromEditor(scope.editor, parsedEffectiveValueKey)?.remove(option.name)
when (option.declaredScope) {
GLOBAL -> {
setGlobalOptionValue(option, value)
globalParsedValues.remove(option.name)
}
LOCAL_TO_BUFFER, GLOBAL_OR_LOCAL_TO_BUFFER -> {
setBufferLocalOptionValue(option, scope.editor, value)
injector.vimStorageService.getDataFromBuffer(scope.editor, parsedEffectiveValueKey)?.remove(option.name)
}
LOCAL_TO_WINDOW, GLOBAL_OR_LOCAL_TO_WINDOW -> {
setWindowLocalOptionValue(option, scope.editor, value)
injector.vimStorageService.getDataFromEditor(scope.editor, parsedEffectiveValueKey)?.remove(option.name)
}
}
}
is OptionScope.GLOBAL -> {
setGlobalOptionValue(option.name, value)
setGlobalOptionValue(option, value)
globalParsedValues.remove(option.name)
}
}
@@ -62,11 +87,11 @@ public abstract class VimOptionGroupBase : VimOptionGroup {
// We have to cache global-local values locally, because they can be set locally. But if they're not overridden
// locally, we would cache a global value per-window. When the global value is changed with OptionScope.GLOBAL, we
// are unable to clear the per-window cached value, so windows would end up with stale cached (global) values.
check(option.declaredScope != OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_WINDOW
&& option.declaredScope != OptionDeclaredScope.GLOBAL_OR_LOCAL_TO_BUFFER
check(option.declaredScope != GLOBAL_OR_LOCAL_TO_WINDOW
&& option.declaredScope != GLOBAL_OR_LOCAL_TO_BUFFER
) { "Global-local options cannot currently be cached" }

val cachedValues = if (option.declaredScope == OptionDeclaredScope.GLOBAL) {
val cachedValues = if (option.declaredScope == GLOBAL) {
globalParsedValues
}
else {
@@ -86,36 +111,12 @@ public abstract class VimOptionGroupBase : VimOptionGroup {
override fun getOption(key: String): Option<VimDataType>? = Options.getOption(key)
override fun getAllOptions(): Set<Option<VimDataType>> = Options.getAllOptions()

private fun setGlobalOptionValue(optionName: String, value: VimDataType) {
globalValues[optionName] = value
}

private fun getLocalOptions(editor: VimEditor) =
injector.vimStorageService.getOrPutEditorData(editor, localOptionsKey) { mutableMapOf() }

private fun setLocalOptionValue(optionName: String, value: VimDataType, editor: VimEditor) {
val localOptions = getLocalOptions(editor)
localOptions[optionName] = value
}

private fun <T : VimDataType> getGlobalOptionValue(option: Option<T>): T {
// We know that the datatype is correct, because we added it via the same strongly typed option
@Suppress("UNCHECKED_CAST")
return globalValues[option.name] as? T ?: option.defaultValue
}

private fun <T : VimDataType> getLocalOptionValue(option: Option<T>, editor: VimEditor): T {
val localOptions = getLocalOptions(editor)
// Again, we know this cast is safe because we added it with the same strongly typed option
@Suppress("UNCHECKED_CAST")
return localOptions[option.name] as? T ?: getGlobalOptionValue(option)
}

override fun resetAllOptions() {
// TODO: This should be split into two functions. One for testing that resets everything, and one for `:set alL&`
// which should only reset the global options, and the options for the current editor
globalValues.clear()
injector.editorGroup.localEditors().forEach {
injector.vimStorageService.getDataFromBuffer(it, localOptionsKey)?.clear()
injector.vimStorageService.getDataFromEditor(it, localOptionsKey)?.clear()
injector.vimStorageService.getDataFromEditor(it, parsedEffectiveValueKey)?.clear()
}
@@ -152,4 +153,62 @@ public abstract class VimOptionGroupBase : VimOptionGroup {
override fun getGlobalOptions(): GlobalOptions = globalOptionsAccessor

override fun getEffectiveOptions(editor: VimEditor): EffectiveOptions = EffectiveOptions(OptionScope.LOCAL(editor))


private fun <T : VimDataType> getGlobalOptionValue(option: Option<T>): T {
// We set the value via Option<T> so it's safe to cast to T
@Suppress("UNCHECKED_CAST")
return globalValues[option.name] as? T ?: option.defaultValue
}

private fun <T : VimDataType> setGlobalOptionValue(option: Option<T>, value: T) {
globalValues[option.name] = value
}


private fun getBufferLocalOptionStorage(editor: VimEditor) =
injector.vimStorageService.getOrPutBufferData(editor, localOptionsKey) { mutableMapOf() }

private fun <T : VimDataType> setBufferLocalOptionValue(option: Option<T>, editor: VimEditor, value: T) {
val options = getBufferLocalOptionStorage(editor)
options[option.name] = value
}

private fun <T : VimDataType> getBufferLocalOptionValue(option: Option<T>, editor: VimEditor): T {
check(option.declaredScope == LOCAL_TO_BUFFER || option.declaredScope == GLOBAL_OR_LOCAL_TO_BUFFER)
// TODO: Once we initialise the options during buffer creation, we should never get null here
// What about options that are added dynamically, e.g. extensions?
return tryGetBufferLocalOptionValue(option, editor) ?: getGlobalOptionValue(option)
}

private fun <T : VimDataType> tryGetBufferLocalOptionValue(option: Option<T>, editor: VimEditor): T? {
val options = getBufferLocalOptionStorage(editor)

// We set the value via Option<T> so it's safe to cast to T
@Suppress("UNCHECKED_CAST")
return options[option.name] as? T
}


private fun getWindowLocalOptionStorage(editor: VimEditor) =
injector.vimStorageService.getOrPutEditorData(editor, localOptionsKey) { mutableMapOf() }

private fun <T : VimDataType> setWindowLocalOptionValue(option: Option<T>, editor: VimEditor, value: T) {
val options = getWindowLocalOptionStorage(editor)
options[option.name] = value
}

private fun <T : VimDataType> getWindowLocalOptionValue(option: Option<T>, editor: VimEditor): T {
check(option.declaredScope == LOCAL_TO_WINDOW || option.declaredScope == GLOBAL_OR_LOCAL_TO_WINDOW)
// TODO: Once we initialise the options during window creation, we should never get null here
return tryGetWindowLocalOptionValue(option, editor) ?: getGlobalOptionValue(option)
}

private fun <T : VimDataType> tryGetWindowLocalOptionValue(option: Option<T>, editor: VimEditor): T? {
val options = getWindowLocalOptionStorage(editor)

// We set the value via Option<T> so it's safe to cast to T
@Suppress("UNCHECKED_CAST")
return options[option.name] as? T
}
}