-
Notifications
You must be signed in to change notification settings - Fork 8.8k
feat(tui): move some experimental keys from config to ENV as well #11216
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
base: dev
Are you sure you want to change the base?
feat(tui): move some experimental keys from config to ENV as well #11216
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
4020239 to
1f3bf56
Compare
|
Personally, I think that it would feel more natural to me if the priority worked the other way around, allowing the environment variable flags be used ad-hoc to temporarily override the values specified in my configuration file. I'm sure the opinions on this will vary, though. |
…ng sent back as assistant message content (anomalyco#11270) Co-authored-by: opencode-agent[bot] <opencode-agent[bot]@users.noreply.github.com>
Yes that makes sense, but the issue here is that not all feature flags are available in either system, in this PR I am just trying to sync up the undocumented flags in The reason to give config priority over ENV is just for backwards compatibility for those flags, currently there is no override system in place, allowing ENV to take precedence over config means some flags does and some doesn't override. Long term I think once the system is standarized, it makes sense to give ENV precedence over the config. |
|
@IdrisGit Alright. To me, it seems that we could potentially take care of both sides of it in one fell swoop: make all the experimental options available through either environment variables or configuration settings, and make the environment variables always take priority, getting the whole job done and making the behaviour both consistent and flexible in one shot. You've likely thought this issue through more deeply than I have yet, though, so I'm sure that you have good reason for taking a more conservative approach and I will happily defer to your wisdom here. |
|
@ariane-emory yeah I was thinking about working on later, I created this PR adhoc when I was talking to someone on discord and found this issue, it was confirmed by Aiden as well that there is discrepancy, so plan with this PR is just to sync that up. I need to think about it abit, before I create a PR on that issue, till then syncing should do. |
00637c0 to
71e0ba2
Compare
f1ae801 to
08fa7f7
Compare
Motivation
currently there are two ways to enable experimental features in OpenCode, one via the config file and another using ENV variables, there are some features which can only be enabled via config right now but those are not documented anywhere in the docs, however ENV based experimental feature toggle is documented well enough.
What does this PR do?
migrates these feature flags:
disable_paste_summary,batch_tool,openTelemetry,continue_loop_on_denyandmcp_timeoutto the ENV based feature flags as well, with config values getting priority over ENV for compatibility.