-
Notifications
You must be signed in to change notification settings - Fork 23
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 cody settings editor #1972
Add cody settings editor #1972
Conversation
import com.sourcegraph.config.ConfigUtil | ||
|
||
class CodySettingsChangeListener(private val project: Project) : FileDocumentManagerListener { | ||
override fun beforeDocumentSaving(document: Document) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any risk that the restart will happen to often while editing the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think not, this happen only on Ctrl+S, or if you switch IJ window.
val currentFile = FileDocumentManager.getInstance().getFile(document) | ||
if (currentFile?.toNioPath() == ConfigUtil.getSettingsFile(project)) { | ||
CodyAgentService.getInstance(project).restartAgent(project) | ||
// TODO: we should instead call `extensionConfiguration_didChange` there: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it an issue for Cody repo?
?: run { | ||
logger.warn("Project cannot be null") | ||
return | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can log it but it looks like a common practice is to simply ?: return
(see our other actions and actions in IntelliJ)
configName, | ||
JsonSchemaVersion.SCHEMA_7, | ||
schemaFile.toString(), | ||
false, | ||
listOf( | ||
UserDefinedJsonSchemaConfiguration.Item( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you name the params?
|
||
private fun reloadSchemaAsync(project: Project) { | ||
CodyAgentService.withAgentRestartIfNeeded(project) { agent -> | ||
val settingsSchema = agent.server.extensionConfiguration_getSettingsSchema(null).get() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this actions not only opens the file but also fetches the config from the agent and updates the content of the file, is that right? that makes the agent the source of truth at the moment we use this action - I wonder, shouldn't it be the client holding the truth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, extensionConfiguration_getSettingsSchema
always takes null
, why is that? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, shouldn't it be the client holding the truth?
I do not think so, and I'm not sure how that would be supposed to work?
After all it's VSC code which knows which settings are used, etc.
Keeping it in client would be artificial, and would also need to be duplicated for every client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. My main concern was about persisting the settings b/w the IDE runs. Now I can see that ConfigUtil::getAgentConfiguration
handles the initialization properly on the startup - all good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably I missed it but how do we notify the agent about the initial settings on IDE and plugin start up?
I be just started the IDE with your changes and got this:
(I assume my config json did not exist initially) |
|
## Changes This PR adds `extensionConfiguration/getSettingsSchema` endpoint which generates json schema file useful for validating cody settings and for documentation purposes. ## Test plan 1. Build Jenkins [PR #1972](sourcegraph/jetbrains#1972) with CODY_DIR set to this PR branch. 2. Run action `Open Cody Settings Editor` (you can use `shift shift` to find it) 3. Change some Cody settings. E.g. you can try to disable autocomplete: `"cody.autocomplete.enabled": true` 4. Hit `Ctrl + S` to trigger the config update 5. Try to use autocomplete in some other file and make sure it does not work. 6. Revert your changes and make sure autocomplete is enabled again.
89101b4
to
283fbd6
Compare
Fixes CODY-1310
Changes
This PR adds cody settings editor with schema validation and code completion.
Test plan
Open Cody Settings Editor
(you can useshift shift
to find it)"cody.autocomplete.enabled": true
Ctrl + S
to trigger the config update