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

Add options for ConfigFactory to handle customConfig properly #2588

Merged
merged 2 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions src/main/kotlin/com/sourcegraph/config/ConfigUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,13 @@ object ConfigUtil {

return try {
val text = customConfigContent ?: getSettingsFile(project).readText()
val config = ConfigFactory.parseString(text).resolve()
var config = ConfigFactory.parseString(text).resolve()
additionalProperties.forEach { (key, value) ->
config.withValue(key, ConfigValueFactory.fromAnyRef(value))
Copy link
Contributor Author

@mkondratek mkondratek Nov 7, 2024

Choose a reason for hiding this comment

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

There was a bug 😄 configs are immutable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically, we ignored "foldingRanges": "indentation-based" - how important for us is this option?

config = config.withValue(key, ConfigValueFactory.fromAnyRef(value))
}
config.root().render(ConfigRenderOptions.defaults().setOriginComments(false))
config
.root()
.render(ConfigRenderOptions.defaults().setComments(false).setOriginComments(false))
} catch (e: Exception) {
logger.info("No user defined settings file found. Proceeding with empty custom config")
""
Expand Down
66 changes: 66 additions & 0 deletions src/test/kotlin/com/sourcegraph/config/ConfigUtilTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import com.google.gson.JsonParser
import com.intellij.openapi.project.Project
import com.sourcegraph.config.ConfigUtil
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Test
import org.mockito.Mockito.mock

class ConfigUtils {

private val mockProject = mock(Project::class.java)

@Test
fun testGetCustomConfiguration_addsAdditionalProperties() {
val input =
"""
{
"cody.debug": true,
"cody.autocomplete.enabled": true
}
"""
val result = ConfigUtil.getCustomConfiguration(mockProject, input)
val parsed = JsonParser.parseString(result).asJsonObject

assertEquals(
"indentation-based",
parsed
.get("cody")
.asJsonObject
.get("experimental")
.asJsonObject
.get("foldingRanges")
.asString)
Comment on lines +27 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

withValue adds the field as

"cody": {
  "experimental": {
    "foldingRanges": "indentation-based"
  }
}

instead of

"cody.experimental.foldingRanges": "indentation-based"

Is it a problem? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO It is not, those are equivalent in VSC config.

}

@Test
fun testGetCustomConfiguration_handlesTrailingCommas() {
val input =
"""
{
"cody.debug": true,
"cody.autocomplete.enabled": true,
}
"""
val result = ConfigUtil.getCustomConfiguration(mockProject, input)
assertNotNull(result)
val parsed = JsonParser.parseString(result).asJsonObject
assertEquals(2 + 1, parsed.size()) // +1 for the additional folding property
}

@Test
fun testGetCustomConfiguration_handlesComments() {
val input =
"""
{
// This is a comment
"cody.debug": true,
"cody.autocomplete.enabled": true,
}
"""
val result = ConfigUtil.getCustomConfiguration(mockProject, input)
assertNotNull(result)
val parsed = JsonParser.parseString(result).asJsonObject
assertEquals(2 + 1, parsed.size()) // +1 for the additional folding property
}
}
Loading