lywsvip/warp
1
0
Fork 0
mirror of https://github.com/warpdotdev/warp.git synced 2026-05-06 23:32:51 +08:00
Code Issues Packages Projects Releases Wiki Activity

Use feature flag in settings crate (#9507)

Browse Source 
The `settings` crate previously couldn't depend on `warp_features`, so
we plumbed `FeatureFlag::SettingsFile.is_enabled()` into a
process-global `SETTINGS_FILE_ENABLED: AtomicBool` at startup and the
crate read that mirror instead. The dependency restriction is gone, so
this PR removes the indirection and calls the feature flag directly from
the settings crate.
This commit is contained in:
Daniel Peng
2026-05-04 14:33:16 -07:00
committed by GitHub
parent 693cd58de7
commit b1cb96f984
8 changed files with 18 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@@ -11922,7 +11922,6 @@ dependencies = [
"schemars",
"serde",
"serde_json",
"serial_test",
"settings_value",
"tempfile",
"warp_features",

5
app/src/lib.rs
View File

@@ -1025,11 +1025,6 @@ fn run_internal(mut launch_mode: LaunchMode) -> Result<()> {
ctx.add_singleton_model(move |_ctx| private_preferences);
let startup_toml_parse_error = startup_toml_parse_error;
// Tell the settings crate whether the TOML settings file is active.
// This must happen after preferences are registered but before settings
// are initialized, so the routing logic picks the correct backend.
::settings::set_settings_file_enabled(FeatureFlag::SettingsFile.is_enabled());
#[cfg(enable_crash_recovery)]
ctx.add_singleton_model(move |_ctx| crash_recovery);

34
app/src/settings/init_tests.rs
View File

@@ -1,8 +1,5 @@
use instant::Duration;
use settings::{
is_settings_file_enabled, set_settings_file_enabled, PrivatePreferences, PublicPreferences,
Setting, SettingsManager,
};
use settings::{PrivatePreferences, PublicPreferences, Setting, SettingsManager};
use settings_value::SettingsValue;
use warp_core::features::FeatureFlag;
use warp_core::settings::{macros::define_settings_group, SupportedPlatforms, SyncToCloud};
@@ -58,33 +55,12 @@ fn init_test_app(ctx: &mut warpui::AppContext) {
MigrationTestSettings::register(ctx);
}
struct SettingsFileEnabledGuard(bool);
impl SettingsFileEnabledGuard {
fn new(enabled: bool) -> Self {
let previous = is_settings_file_enabled();
set_settings_file_enabled(enabled);
Self(previous)
}
}
impl Drop for SettingsFileEnabledGuard {
fn drop(&mut self) {
set_settings_file_enabled(self.0);
}
}
// Only tests that toggle the process-global SettingsFile routing flag need to
// run serially.
#[test]
#[serial_test::serial]
fn test_migration_copies_public_settings_from_native_store() {
warpui::App::test((), |mut app| async move {
// Enable the settings file so `preferences_for_setting` routes
// public setting writes to the Model singleton (not the private store).
let _guard = FeatureFlag::SettingsFile.override_enabled(true);
let _settings_file_enabled = SettingsFileEnabledGuard::new(true);
app.update(init_test_app);
@@ -172,11 +148,9 @@ fn test_migration_writes_marker_to_native_store() {
}
#[test]
#[serial_test::serial]
fn test_migration_skips_settings_absent_from_native_store() {
warpui::App::test((), |mut app| async move {
let _guard = FeatureFlag::SettingsFile.override_enabled(true);
let _settings_file_enabled = SettingsFileEnabledGuard::new(true);
app.update(init_test_app);
// Don't seed anything in the native store — all settings are absent.
@@ -282,11 +256,9 @@ fn test_migration_does_not_rerun_when_marker_present() {
}
#[test]
#[serial_test::serial]
fn test_migration_with_multiple_setting_types() {
warpui::App::test((), |mut app| async move {
let _guard = FeatureFlag::SettingsFile.override_enabled(true);
let _settings_file_enabled = SettingsFileEnabledGuard::new(true);
app.update(init_test_app);
@@ -462,11 +434,9 @@ fn test_notifications_from_file_value_rejects_serde_format_duration() {
// -- Migration integration tests: these demonstrate end-to-end data loss -----
#[test]
#[serial_test::serial]
fn test_migration_preserves_notifications_mode() {
warpui::App::test((), |mut app| async move {
let _guard = FeatureFlag::SettingsFile.override_enabled(true);
let _settings_file_enabled = SettingsFileEnabledGuard::new(true);
app.update(init_notifications_migration_test_app);
@@ -501,11 +471,9 @@ fn test_migration_preserves_notifications_mode() {
}
#[test]
#[serial_test::serial]
fn test_migration_preserves_custom_long_running_threshold() {
warpui::App::test((), |mut app| async move {
let _guard = FeatureFlag::SettingsFile.override_enabled(true);
let _settings_file_enabled = SettingsFileEnabledGuard::new(true);
app.update(init_notifications_migration_test_app);

2
crates/settings/Cargo.toml
View File

@@ -19,8 +19,8 @@ warpui = { workspace = true, features = ["settings_value"] }
warpui_extras = { workspace = true, features = ["default"] }
[dev-dependencies]
serial_test = "0.8.0"
tempfile.workspace = true
warp_features = { workspace = true, features = ["test-util"] }
warpui_extras = { workspace = true, features = ["user_preferences-toml"] }
[features]

26
crates/settings/src/lib.rs
View File

@@ -18,7 +18,6 @@ pub use settings_value::SettingsValue;
use std::fmt::Debug;
use std::ops::Deref;
use std::sync::atomic::{AtomicBool, Ordering};
/// Extracts the storage key (last segment after the final `.`) from a toml_path.
///
@@ -60,31 +59,10 @@ pub const fn toml_path_hierarchy(path: &str) -> Option<&str> {
use anyhow::{Context, Result};
use serde::{Serialize, de::DeserializeOwned};
use warp_features::FeatureFlag;
use warpui::{AppContext, Entity, ModelContext};
use warpui_extras::user_preferences::UserPreferences;
/// Whether the TOML-backed settings file is active.
///
/// Set once during startup via [`set_settings_file_enabled`]. When `false`,
/// public settings fall back to the private (platform-native) backend so
/// that all settings share a single instance.
static SETTINGS_FILE_ENABLED: AtomicBool = AtomicBool::new(false);
/// Records whether the TOML-backed settings file feature is active.
///
/// Call this once during startup after checking `FeatureFlag::SettingsFile`.
/// The value is read by [`Setting::preferences_for_setting`] and
/// [`SettingsManager::read_local_setting_value`] to decide which backend
/// to use for public settings.
pub fn set_settings_file_enabled(enabled: bool) {
SETTINGS_FILE_ENABLED.store(enabled, Ordering::Relaxed);
}
/// Returns whether the TOML-backed settings file is currently active.
pub fn is_settings_file_enabled() -> bool {
SETTINGS_FILE_ENABLED.load(Ordering::Relaxed)
}
/// A newtype wrapper for the public preferences backend.
///
/// Public settings (those marked `private: false` in `define_settings_group!`)
@@ -403,7 +381,7 @@ pub trait Setting {
if Self::is_private() {
<PrivatePreferences as SingletonEntity>::as_ref(ctx).deref()
} else if is_settings_file_enabled() {
} else if FeatureFlag::SettingsFile.is_enabled() {
<PublicPreferences as SingletonEntity>::as_ref(ctx).as_preferences()
} else {
// When the settings file is disabled, fall back to the private

44
crates/settings/src/macros_tests.rs
View File

@@ -627,11 +627,6 @@ mod file_transform_tests {
// Private / public settings split tests
// ---------------------------------------------------------------------------
// Tests that call `set_settings_file_enabled` are marked `#[serial_test::serial]`
// because they mutate the process-global `SETTINGS_FILE_ENABLED` AtomicBool and
// would race under `cargo test` (thread-based parallelism). This can be removed
// when the SettingsFile feature flag is cleaned up and the global flag is deleted.
#[test]
fn test_is_private_returns_false_for_public_setting() {
assert!(!SimpleSetting::is_private());
@@ -643,18 +638,8 @@ fn test_is_private_returns_true_for_private_setting() {
}
#[test]
#[serial_test::serial]
fn test_settings_file_enabled_flag_round_trip() {
crate::set_settings_file_enabled(true);
assert!(crate::is_settings_file_enabled());
crate::set_settings_file_enabled(false);
assert!(!crate::is_settings_file_enabled());
}
#[test]
#[serial_test::serial]
fn test_public_setting_writes_to_public_prefs_when_flag_enabled() {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -690,9 +675,8 @@ fn test_public_setting_writes_to_public_prefs_when_flag_enabled() {
}
#[test]
#[serial_test::serial]
fn test_private_setting_writes_to_private_prefs_when_flag_enabled() {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -737,9 +721,8 @@ fn test_private_setting_writes_to_private_prefs_when_flag_enabled() {
}
#[test]
#[serial_test::serial]
fn test_new_from_storage_reads_from_correct_backend_when_flag_enabled() {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -775,9 +758,8 @@ fn test_new_from_storage_reads_from_correct_backend_when_flag_enabled() {
}
#[test]
#[serial_test::serial]
fn test_clear_value_clears_from_correct_backend() {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -831,9 +813,8 @@ fn test_clear_value_clears_from_correct_backend() {
}
#[test]
#[serial_test::serial]
fn test_public_setting_uses_private_prefs_when_flag_disabled() {
crate::set_settings_file_enabled(false);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(false);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -872,9 +853,8 @@ fn test_public_setting_uses_private_prefs_when_flag_disabled() {
}
#[test]
#[serial_test::serial]
fn test_private_setting_uses_private_prefs_when_flag_disabled() {
crate::set_settings_file_enabled(false);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(false);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -906,9 +886,8 @@ fn test_private_setting_uses_private_prefs_when_flag_disabled() {
}
#[test]
#[serial_test::serial]
fn test_new_from_storage_reads_from_private_backend_when_flag_disabled() {
crate::set_settings_file_enabled(false);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(false);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -1003,9 +982,8 @@ fn test_manager_default_values_for_settings_file_excludes_private() {
}
#[test]
#[serial_test::serial]
fn test_manager_read_local_setting_value_routes_when_flag_enabled() {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -1051,9 +1029,8 @@ fn test_manager_read_local_setting_value_routes_when_flag_enabled() {
}
#[test]
#[serial_test::serial]
fn test_manager_read_local_setting_value_falls_back_when_flag_disabled() {
crate::set_settings_file_enabled(false);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(false);
warpui::App::test((), |mut app| async move {
app.update(init_and_register_preferences);
app.add_singleton_model(|_| SettingsManager::default());
@@ -1102,11 +1079,10 @@ fn test_manager_read_local_setting_value_falls_back_when_flag_disabled() {
/// section like `[account]` are invisible to the SettingsManager and the
/// cloud preferences syncer clobbers them with stale cloud state.
#[test]
#[serial_test::serial]
fn test_manager_read_local_setting_value_respects_hierarchy_with_settings_file() {
use warpui_extras::user_preferences::toml_backed::TomlBackedUserPreferences;
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
let dir = tempfile::tempdir().unwrap();
let file_path = dir.path().join("settings.toml");

3
crates/settings/src/manager.rs
View File

@@ -3,6 +3,7 @@ use std::collections::HashMap;
use std::ops::Deref;
use anyhow::{Result, anyhow};
use warp_features::FeatureFlag;
use warpui::{AppContext, Entity, ModelContext, SingletonEntity};
use warpui_extras::user_preferences::UserPreferences;
@@ -232,7 +233,7 @@ impl SettingsManager {
<PrivatePreferences as SingletonEntity>::as_ref(ctx).deref();
let prefs: &dyn UserPreferences = if self.is_private_for_storage_key(storage_key) {
private
} else if super::is_settings_file_enabled() {
} else if FeatureFlag::SettingsFile.is_enabled() {
<super::PublicPreferences as SingletonEntity>::as_ref(ctx).as_preferences()
} else {
// When the settings file is disabled, fall back to the private

4
crates/settings/src/mod_tests.rs
View File

@@ -385,7 +385,7 @@ mod reload_all_public_settings_tests {
#[test]
fn test_validate_detects_invalid_values() {
warpui::App::test((), |mut app| async move {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
app.update(init_prefs);
app.add_singleton_model(|_| SettingsManager::default());
ReloadTestSettings::register(&mut app);
@@ -424,7 +424,7 @@ mod reload_all_public_settings_tests {
#[test]
fn test_validate_returns_empty_when_all_valid() {
warpui::App::test((), |mut app| async move {
crate::set_settings_file_enabled(true);
let _guard = warp_features::FeatureFlag::SettingsFile.override_enabled(true);
app.update(init_prefs);
app.add_singleton_model(|_| SettingsManager::default());
ReloadTestSettings::register(&mut app);