-
Notifications
You must be signed in to change notification settings - Fork 5.6k
core: write explicit [projects] tables for trusted projects #2523
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
Conversation
- Avoid inline TOML tables for per-project entries - Ensure explicit table headers for [projects] and each project key - Add unit test to verify formatting (no inline tables)
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.
1 non-blocking comment about explicit-ness required. Thanks for fixing!
codex-rs/core/src/config.rs
Outdated
)); | ||
}; | ||
// Ensure we emit an explicit [projects] header | ||
projects_tbl.set_implicit(false); |
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.
Does toml require us to have [projects]
to set [projects."/path/to/project"]
? We parse [mcp_servers.playwright]
and [profiles.oss]
without [mcp_servers]
or [profiles]
, respectively. Or is this just required to ensure the sub-tables are not in-line?
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.
hm yeah it's probably not required.
// Run the function; it should convert to explicit tables and set trusted | ||
set_project_trusted(codex_home.path(), project_dir.path())?; | ||
|
||
let contents = std::fs::read_to_string(&config_path)?; |
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 a reason why we cannot assert_eq!()
on contents
? What is variable in this point of the test?
all of my trust_level settings in my ~/.codex/config.toml were on one line.