oak-team/oak-gpui
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

settings_ui: Get editor settings working (#37330)

Browse Source 
Closes #ISSUE

This PR includes the necessary work to get `EditorSettings` showing up
in the settings UI. Including making the `path` field on
`SettingsUiItem`'s optional so that top level items such as
`EditorSettings` which have `Settings::KEY = None` (i.e. are treated
like `serde(flatten)`) have their paths computed correctly for JSON
reading/updating.

It includes the first examples of a pattern I expect to continue with
the `SettingsUi` work with respect to settings reorganization, that
being adding missing defaults, and adding explicit values (or aliases)
to settings which previously relied on `null` being a value for optional
fields.

Release Notes:

- N/A *or* Added/Fixed/Improved ...
This commit is contained in:
Ben Kunkle
2025-09-02 00:26:42 +00:00
committed by GitHub
parent 60d17cccd3
commit 54cec5b484
9 changed files with 246 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/settings_ui/src/settings_ui.rs
+91 -46
View File
@@ -9,7 +9,10 @@ use command_palette_hooks::CommandPaletteFilter;
use editor::EditorSettingsControls;
use feature_flags::{FeatureFlag, FeatureFlagViewExt};
use gpui::{App, Entity, EventEmitter, FocusHandle, Focusable, ReadGlobal, actions};
use settings::{SettingsStore, SettingsUiEntryVariant, SettingsUiItemSingle, SettingsValue};
use settings::{
NumType, SettingsStore, SettingsUiEntry, SettingsUiItem, SettingsUiItemDynamic,
SettingsUiItemGroup, SettingsUiItemSingle, SettingsValue,
};
use smallvec::SmallVec;
use ui::{NumericStepper, SwitchField, ToggleButtonGroup, ToggleButtonSimple, prelude::*};
use workspace::{
@@ -134,7 +137,7 @@ impl Item for SettingsPage {
struct UiEntry {
title: &'static str,
path: &'static str,
path: Option<&'static str>,
_depth: usize,
// a
// b < a descendant range < a total descendant range
@@ -182,14 +185,14 @@ struct SettingsUiTree {
fn build_tree_item(
tree: &mut Vec<UiEntry>,
entry: SettingsUiEntryVariant,
entry: SettingsUiEntry,
depth: usize,
prev_index: Option<usize>,
) {
let index = tree.len();
tree.push(UiEntry {
title: "",
path: "",
title: entry.title,
path: entry.path,
_depth: depth,
descendant_range: index + 1..index + 1,
total_descendant_range: index + 1..index + 1,
@@ -200,14 +203,8 @@ fn build_tree_item(
if let Some(prev_index) = prev_index {
tree[prev_index].next_sibling = Some(index);
}
match entry {
SettingsUiEntryVariant::Group {
path,
title,
items: group_items,
} => {
tree[index].path = path;
tree[index].title = title;
match entry.item {
SettingsUiItem::Group(SettingsUiItemGroup { items: group_items }) => {
for group_item in group_items {
let prev_index = tree[index]
.descendant_range
@@ -215,22 +212,17 @@ fn build_tree_item(
.not()
.then_some(tree[index].descendant_range.end - 1);
tree[index].descendant_range.end = tree.len() + 1;
build_tree_item(tree, group_item.item, depth + 1, prev_index);
build_tree_item(tree, group_item, depth + 1, prev_index);
tree[index].total_descendant_range.end = tree.len();
}
}
SettingsUiEntryVariant::Item { path, item } => {
tree[index].path = path;
// todo(settings_ui) create title from path in macro, and use here
tree[index].title = path;
SettingsUiItem::Single(item) => {
tree[index].render = Some(item);
}
SettingsUiEntryVariant::Dynamic {
path,
SettingsUiItem::Dynamic(SettingsUiItemDynamic {
options,
determine_option,
} => {
tree[index].path = path;
}) => {
tree[index].select_descendant = Some(determine_option);
for option in options {
let prev_index = tree[index]
@@ -239,11 +231,11 @@ fn build_tree_item(
.not()
.then_some(tree[index].descendant_range.end - 1);
tree[index].descendant_range.end = tree.len() + 1;
build_tree_item(tree, option.item, depth + 1, prev_index);
build_tree_item(tree, option, depth + 1, prev_index);
tree[index].total_descendant_range.end = tree.len();
}
}
SettingsUiEntryVariant::None => {
SettingsUiItem::None => {
return;
}
}
@@ -255,21 +247,17 @@ impl SettingsUiTree {
let mut tree = vec![];
let mut root_entry_indices = vec![];
for item in settings_store.settings_ui_items() {
if matches!(item.item, SettingsUiEntryVariant::None) {
if matches!(item.item, SettingsUiItem::None)
// todo(settings_ui): How to handle top level single items? BaseKeymap is in this category. Probably need a way to
// link them to other groups
|| matches!(item.item, SettingsUiItem::Single(_))
{
continue;
}
assert!(
matches!(item.item, SettingsUiEntryVariant::Group { .. }),
"top level items must be groups: {:?}",
match item.item {
SettingsUiEntryVariant::Item { path, .. } => path,
_ => unreachable!(),
}
);
let prev_root_entry_index = root_entry_indices.last().copied();
root_entry_indices.push(tree.len());
build_tree_item(&mut tree, item.item, 0, prev_root_entry_index);
build_tree_item(&mut tree, item, 0, prev_root_entry_index);
}
root_entry_indices.sort_by_key(|i| tree[*i].title);
@@ -314,9 +302,12 @@ fn render_content(
.size_full()
.child(Label::new(SharedString::new_static("No settings found")).color(Color::Error));
};
let mut content = v_flex().size_full().gap_4();
let mut content = v_flex().size_full().gap_4().overflow_hidden();
let mut path = smallvec::smallvec![active_entry.path];
let mut path = smallvec::smallvec![];
if let Some(active_entry_path) = active_entry.path {
path.push(active_entry_path);
}
let mut entry_index_queue = VecDeque::new();
if let Some(child_index) = active_entry.first_descendant_index() {
@@ -331,7 +322,11 @@ fn render_content(
while let Some(index) = entry_index_queue.pop_front() {
// todo(settings_ui): subgroups?
let child = &tree.entries[index];
path.push(child.path);
let mut pushed_path = false;
if let Some(child_path) = child.path {
path.push(child_path);
pushed_path = true;
}
let settings_value = settings_value_from_settings_and_path(
path.clone(),
// PERF: how to structure this better? There feels like there's a way to avoid the clone
@@ -347,7 +342,9 @@ fn render_content(
entry_index_queue.push_front(descendant_index);
}
}
path.pop();
if pushed_path {
path.pop();
}
let Some(child_render) = child.render.as_ref() else {
continue;
};
@@ -433,8 +430,8 @@ fn render_item_single(
SettingsUiItemSingle::SwitchField => {
render_any_item(settings_value, render_switch_field, window, cx)
}
SettingsUiItemSingle::NumericStepper => {
render_any_item(settings_value, render_numeric_stepper, window, cx)
SettingsUiItemSingle::NumericStepper(num_type) => {
render_any_numeric_stepper(settings_value, *num_type, window, cx)
}
SettingsUiItemSingle::ToggleGroup(variants) => {
render_toggle_button_group(settings_value, variants, window, cx)
@@ -468,6 +465,7 @@ fn downcast_any_item<T: serde::de::DeserializeOwned>(
.map(|value| serde_json::from_value::<T>(value).expect("value is not a T"));
// todo(settings_ui) Create test that constructs UI tree, and asserts that all elements have default values
let default_value = serde_json::from_value::<T>(settings_value.default_value)
.with_context(|| format!("path: {:?}", settings_value.path.join(".")))
.expect("default value is not an Option<T>");
let deserialized_setting_value = SettingsValue {
title: settings_value.title,
@@ -488,14 +486,62 @@ fn render_any_item<T: serde::de::DeserializeOwned>(
render_fn(deserialized_setting_value, window, cx)
}
fn render_numeric_stepper(
value: SettingsValue<u64>,
fn render_any_numeric_stepper(
settings_value: SettingsValue<serde_json::Value>,
num_type: NumType,
window: &mut Window,
cx: &mut App,
) -> AnyElement {
match num_type {
NumType::U64 => render_numeric_stepper::<u64>(
downcast_any_item(settings_value),
u64::saturating_sub,
u64::saturating_add,
|n| {
serde_json::Number::try_from(n)
.context("Failed to convert u64 to serde_json::Number")
},
window,
cx,
),
NumType::U32 => render_numeric_stepper::<u32>(
downcast_any_item(settings_value),
u32::saturating_sub,
u32::saturating_add,
|n| {
serde_json::Number::try_from(n)
.context("Failed to convert u32 to serde_json::Number")
},
window,
cx,
),
NumType::F32 => render_numeric_stepper::<f32>(
downcast_any_item(settings_value),
|a, b| a - b,
|a, b| a + b,
|n| {
serde_json::Number::from_f64(n as f64)
.context("Failed to convert f32 to serde_json::Number")
},
window,
cx,
),
}
}
fn render_numeric_stepper<
T: serde::de::DeserializeOwned + std::fmt::Display + Copy + From<u8> + 'static,
>(
value: SettingsValue<T>,
saturating_sub: fn(T, T) -> T,
saturating_add: fn(T, T) -> T,
to_serde_number: fn(T) -> anyhow::Result<serde_json::Number>,
_window: &mut Window,
_cx: &mut App,
) -> AnyElement {
let id = element_id_from_path(&value.path);
let path = value.path.clone();
let num = value.value.unwrap_or_else(|| value.default_value);
let num = *value.read();
NumericStepper::new(
id,
@@ -503,8 +549,7 @@ fn render_numeric_stepper(
{
let path = value.path.clone();
move |_, _, cx| {
let Some(number) = serde_json::Number::from_u128(num.saturating_sub(1) as u128)
else {
let Some(number) = to_serde_number(saturating_sub(num, 1.into())).ok() else {
return;
};
let new_value = serde_json::Value::Number(number);
@@ -512,7 +557,7 @@ fn render_numeric_stepper(
}
},
move |_, _, cx| {
let Some(number) = serde_json::Number::from_u128(num.saturating_add(1) as u128) else {
let Some(number) = to_serde_number(saturating_add(num, 1.into())).ok() else {
return;
};