Skip to content

docs(amazonq): Update autobuild setting strings #5301

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.

Sign up for GitHub

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

Merged
rli merged 4 commits into from
Jan 28, 2025
Merged

docs(amazonq): Update autobuild setting strings #5301

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
603f629
chore(amazonq): Update autobuild setting strings
neilk-aws Jan 28, 2025
583bccb
fix: Add @get:Property on autoBuildSetting field of CodeWhispererSett…
neilk-aws Jan 28, 2025
2ed8be0
test: Add tests to validate backwards compatibility of CodeWhispererS…
neilk-aws Jan 28, 2025
7ddc473
Merge branch 'feature/dev-execution' into feature/q-dev-execution
neilk-aws Jan 28, 2025
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
2 changes: 1 addition & 1 deletion ...ware/aws/toolkits/jetbrains/services/amazonqFeatureDev/controller/FeatureDevController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ class FeatureDevController(
return
}

val codeWhispererSettings = CodeWhispererSettings.getInstance().getAutoBuildFeatureConfiguration()
val codeWhispererSettings = CodeWhispererSettings.getInstance().getAutoBuildSetting()
val hasDevFile = session.context.checkForDevFile()
val isPromptedForAutoBuildFeature = codeWhispererSettings.containsKey(session.context.getWorkspaceRoot())

Expand Down
52 changes: 29 additions & 23 deletions ...tware/aws/toolkits/jetbrains/services/codewhisperer/settings/CodeWhispererConfigurable.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,29 +125,6 @@ class CodeWhispererConfigurable(private val project: Project) :
}
}

group(message("aws.settings.codewhisperer.allow_q_dev_build_test_commands")) {
row {
val settings = codeWhispererSettings.getAutoBuildFeatureConfiguration()
for ((key) in settings) {
checkBox(key).apply {
connect.subscribe(
ToolkitConnectionManagerListener.TOPIC,
object : ToolkitConnectionManagerListener {
override fun activeConnectionChanged(newConnection: ToolkitConnection?) {
enabled(isCodeWhispererEnabled(project))
}
}
)

bindSelected(
getter = { codeWhispererSettings.isAutoBuildFeatureEnabled(key) },
setter = { newValue -> codeWhispererSettings.toggleAutoBuildFeature(key, newValue) }
)
}
}
}
}

group(message("aws.settings.codewhisperer.group.q_chat")) {
row {
checkBox(message("aws.settings.codewhisperer.project_context")).apply {
Expand Down Expand Up @@ -214,6 +191,35 @@ class CodeWhispererConfigurable(private val project: Project) :
}
}

val autoBuildSetting = codeWhispererSettings.getAutoBuildSetting()
if (autoBuildSetting.isNotEmpty()) {
group(message("aws.settings.codewhisperer.feature_development")) {
row {
text(message("aws.settings.codewhisperer.feature_development.allow_running_code_and_test_commands"))
}
row {
val settings = codeWhispererSettings.getAutoBuildSetting()
for ((key) in settings) {
checkBox(key).apply {
connect.subscribe(
ToolkitConnectionManagerListener.TOPIC,
object : ToolkitConnectionManagerListener {
override fun activeConnectionChanged(newConnection: ToolkitConnection?) {
enabled(isCodeWhispererEnabled(project))
}
}
)

bindSelected(
getter = { codeWhispererSettings.isAutoBuildFeatureEnabled(key) },
setter = { newValue -> codeWhispererSettings.toggleAutoBuildFeature(key, newValue) }
)
}
}
}
}
}

group(message("aws.settings.codewhisperer.code_review")) {
row {
ExpandableTextField(ParametersListUtil.COLON_LINE_PARSER, ParametersListUtil.COLON_LINE_JOINER).also {
Expand Down
8 changes: 4 additions & 4 deletions ...jetbrains-community/src/software/aws/toolkits/jetbrains/settings/CodeWhispererSettings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ class CodeWhispererSettings : PersistentStateComponent<CodeWhispererConfiguratio
false
)

fun getAutoBuildFeatureConfiguration() = state.projectAutoBuildConfigurationMap
fun getAutoBuildSetting() = state.autoBuildSetting

fun toggleAutoBuildFeature(project: String?, value: Boolean) {
if (project == null) return

state.projectAutoBuildConfigurationMap[project] = value
state.autoBuildSetting[project] = value
}

fun isAutoBuildFeatureEnabled(project: String?) = state.projectAutoBuildConfigurationMap.getOrDefault(project, false)
fun isAutoBuildFeatureEnabled(project: String?) = state.autoBuildSetting.getOrDefault(project, false)

fun toggleImportAdder(value: Boolean) {
state.value[CodeWhispererConfigurationType.IsImportAdderEnabled] = value
Expand Down Expand Up @@ -153,7 +153,7 @@ class CodeWhispererConfiguration : BaseState() {
@get:Property
val stringValue by map<CodeWhispererStringConfigurationType, String>()

val projectAutoBuildConfigurationMap by map<String, Boolean>()
val autoBuildSetting by map<String, Boolean>()
Copy link
Contributor

@rli rli Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

needs @get:Property like the other properties in here
after release, renaming this will be a breaking change for customers so it is probably worth a test case

Copy link
Contributor Author

@neilk-aws neilk-aws Jan 28, 2025

Choose a reason for hiding this comment

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

Added the missing @get:Property. Do you have a recommendation on how to test that we aren't renaming a variable? I recognize renaming this would be breaking and is only done now since we are pre-release.

Copy link
Contributor

@rli rli Jan 28, 2025

Choose a reason for hiding this comment

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

oh great there are no existing tests on this state component
basic ser/deser tests similar to below should be sufficient coverage:

aws-toolkit-jetbrains/plugins/amazonq/codewhisperer/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/codewhisperer/CodeWhispererModelConfiguratorTest.kt

Lines 297 to 440 in 2e39dc0

@Test
fun serialize() {
val element = xmlElement(
"""
<component name="codewhispererCustomizationStates">
</component>
""".trimIndent()
)
val state = CodeWhispererCustomizationState().apply {
this.previousAvailableCustomizations.putAll(
mapOf(
"fake-sso-url" to mutableListOf("arn_1", "arn_2")
)
)
this.connectionIdToActiveCustomizationArn.putAll(
mapOf(
"fake-sso-url" to CodeWhispererCustomization(arn = "arn_2", name = "name_2", description = "description_2")
)
)
}
XmlSerializer.serializeInto(state, element)
val actual = XMLOutputter().outputString(element)
val expected = "<component name=\"codewhispererCustomizationStates\">\n" +
"<option name=\"connectionIdToActiveCustomizationArn\">" +
"<map>" +
"<entry key=\"fake-sso-url\">" +
"<value>" +
"<CodeWhispererCustomization>" +
"<option name=\"arn\" value=\"arn_2\" />" +
"<option name=\"name\" value=\"name_2\" />" +
"<option name=\"description\" value=\"description_2\" />" +
"</CodeWhispererCustomization>" +
"</value>" +
"</entry>" +
"</map>" +
"</option>" +
"<option name=\"previousAvailableCustomizations\">" +
"<map>" +
"<entry key=\"fake-sso-url\">" +
"<value>" +
"<list>" +
"<option value=\"arn_1\" />" +
"<option value=\"arn_2\" />" +
"</list>" +
"</value>" +
"</entry>" +
"</map>" +
"</option>" +
"</component>"
assertThat(actual).isEqualTo(expected)
}
@Test
fun `deserialize empty data`() {
val element = xmlElement(
"""
<component name="codewhispererCustomizationStates">
</component>
"""
)
val actual = XmlSerializer.deserialize(element, CodeWhispererCustomizationState::class.java)
assertThat(actual.connectionIdToActiveCustomizationArn).hasSize(0)
assertThat(actual.previousAvailableCustomizations).hasSize(0)
}
@Test
fun `deserialize users choosing a customization`() {
val element = xmlElement(
"""
<component name="codewhispererCustomizationStates">
<option name="connectionIdToActiveCustomizationArn">
<map>
<entry key="fake-sso-url">
<value>
<CodeWhispererCustomization>
<option name="arn" value="arn_2" />
<option name="name" value="name_2" />
<option name="description" value="description_2" />
</CodeWhispererCustomization>
</value>
</entry>
</map>
</option>
<option name="previousAvailableCustomizations">
<map>
<entry key="fake-sso-url">
<value>
<list>
<option value="arn_1" />
<option value="arn_2" />
<option value="arn_3" />
</list>
</value>
</entry>
</map>
</option>
</component>
"""
)
val actual = XmlSerializer.deserialize(element, CodeWhispererCustomizationState::class.java)
assertThat(actual.connectionIdToActiveCustomizationArn).hasSize(1)
assertThat(actual.connectionIdToActiveCustomizationArn["fake-sso-url"]).isEqualTo(
CodeWhispererCustomization(
arn = "arn_2",
name = "name_2",
description = "description_2"
)
)
assertThat(actual.previousAvailableCustomizations).hasSize(1)
assertThat(actual.previousAvailableCustomizations["fake-sso-url"]).isEqualTo(listOf("arn_1", "arn_2", "arn_3"))
}
@Test
fun `deserialize users choosing default customization`() {
val element = xmlElement(
"""
<component name="codewhispererCustomizationStates">
<option name="previousAvailableCustomizations">
<map>
<entry key="fake-sso-url">
<value>
<list>
<option value="arn_1" />
<option value="arn_2" />
<option value="arn_3" />
</list>
</value>
</entry>
</map>
</option>
</component>
"""
)
val actual = XmlSerializer.deserialize(element, CodeWhispererCustomizationState::class.java)
assertThat(actual.connectionIdToActiveCustomizationArn).hasSize(0)
assertThat(actual.previousAvailableCustomizations).hasSize(1)
assertThat(actual.previousAvailableCustomizations["fake-sso-url"]).isEqualTo(listOf("arn_1", "arn_2", "arn_3"))
}

Copy link
Contributor

@rli rli Jan 28, 2025

Choose a reason for hiding this comment

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

on another note, doing a map of project -> boolean isnt really the best way to model this.

if you don't want the setting to be application-wide, you should define a new project-level state component so that the setting is saved per-project. right now the implementation looks like it makes a new check box for every project the user opens?

Copy link
Contributor Author

@neilk-aws neilk-aws Jan 28, 2025

Choose a reason for hiding this comment

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

The setting is application wide and mirrors the structure that we're able to give in VSCode. I think there is opportunity for improvement in modeling, but I don't think I want to block on that right now. I'd love to learn about where project-level state component and what other features are using it. If we do migrate, we'll implement the logic to import the settings accordingly.

Copy link
Contributor Author

@neilk-aws neilk-aws Jan 28, 2025

Choose a reason for hiding this comment

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

Thanks for the pointer on the tests. I've added them here, with an initial focus on our property. I've kept it generic for other teams to be able to contribute more validation there.

Copy link
Contributor

@rli rli Jan 28, 2025

Choose a reason for hiding this comment

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

switching between the two is not exactly trivial so it would be better to get that right now.

the configurable is already set up to support project-level settings so the only difference would be creating a new state service, and you can get rid of the weird map layer

    private val codeWhispererProjectSettings
        get() = CodeWhispererProjectSettings.getInstance(project)
            group(message("aws.settings.codewhisperer.feature_development")) {
                row {
                    text(message("aws.settings.codewhisperer.feature_development.allow_running_code_and_test_commands"))
                }
                row {
                    checkBox(codeWhispererProjectSettings.getAutoBuildSetting()).apply {
                        connect.subscribe(
                            ToolkitConnectionManagerListener.TOPIC,
                            object : ToolkitConnectionManagerListener {
                                override fun activeConnectionChanged(newConnection: ToolkitConnection?) {
                                    enabled(isCodeWhispererEnabled(project))
                                }
                            }
                        )

                        bindSelected(
                            getter = { codeWhispererProjectSettings.isAutoBuildFeatureEnabled() },
                            setter = { newValue -> codeWhispererProjectSettings.toggleAutoBuildFeature(newValue) }
                        )
                    }
                }
            }    
@Service(Level.PROJECT)
@State(name = "codewhispererProejctSettings", storages = [Storage("aws.xml")])
class CodeWhispererProjectSettings(private val project: Project) : PersistentStateComponent<CodeWhispererProjectConfiguration> {
...
}

class CodeWhispererProjectConfiguration : BaseState() {
    @get:Property
    val autoBuildSetting by property(false)
}

Copy link
Contributor Author

@neilk-aws neilk-aws Jan 28, 2025

Choose a reason for hiding this comment

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

We can take this as a follow up. Given we're trying to merge the feature branch into main today, I think we'll accept the risk of needing to implement a pathway to import settings from the storage layer from the existing format into a the new format once we migrate to it.

Copy link
Contributor

@rli rli Jan 28, 2025

Choose a reason for hiding this comment

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

sure

}

enum class CodeWhispererConfigurationType {
Expand Down
3 changes: 2 additions & 1 deletion plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ aws.settings.auto_update.notification_enable.tooltip=If unchecked, updates will
aws.settings.auto_update.progress.message=Updating AWS plugins
aws.settings.auto_update.text=Automatically install plugin updates when available
aws.settings.aws_cli_settings=AWS CLI Settings
aws.settings.codewhisperer.allow_q_dev_build_test_commands=Amazon Q: Allow Q /dev to run code and test commands
aws.settings.codewhisperer.feature_development=Feature Development
aws.settings.codewhisperer.feature_development.allow_running_code_and_test_commands=Allow /dev to run code and test commands
aws.settings.codewhisperer.automatic_import_adder=Imports recommendation
aws.settings.codewhisperer.automatic_import_adder.tooltip=Amazon Q will add import statements with code suggestions when necessary
aws.settings.codewhisperer.code_review=Code Review
Expand Down
Loading