From b0fa259187eb6b2260cb687cd48b906f08d387c2 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sat, 30 May 2026 22:01:42 -0400 Subject: [PATCH] feat(friends): persist per-friend share preference across launches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host's "auto-share my code with" picker was session-scoped — an in-memory exclusion set that reset to share-with-all on every launch. Move the preference onto the friend itself: a `share: bool` on `Friend` in friends.toml, `#[serde(default = true)]` so new friends are included and an older file without the field loads as share-with-all. The picker now toggles the stored flag and persists immediately (like the other settings), and `selected_share_targets` filters on it. This drops the parallel `share_excluded` state and is self-cleaning: removing a friend takes their preference with them, no stale ids linger. `upsert`'s update path leaves `share` untouched, so a name/presence refresh can't reset the user's choice. Co-Authored-By: Claude Opus 4.8 --- src/common/friends.rs | 42 +++++++++++++++++++++++++++++++++- src/gui/mod.rs | 53 ++++++++++++++++++------------------------- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/src/common/friends.rs b/src/common/friends.rs index b4aa8d7..f95c20d 100644 --- a/src/common/friends.rs +++ b/src/common/friends.rs @@ -35,6 +35,16 @@ pub struct Friend { /// Display name — seeded from the name the peer reported, locally editable. pub name: String, pub state: FriendState, + /// Whether the host auto-shares its session code with this friend. Toggled + /// on the host's share picker; persisted here so the choice survives a + /// restart. Defaults to `true` so a newly added friend is included (and an + /// older `friends.toml` without the field loads as share-with-all). + #[serde(default = "default_share")] + pub share: bool, +} + +fn default_share() -> bool { + true } /// The persisted friends list. Serialises as a TOML array of tables @@ -120,7 +130,12 @@ impl FriendStore { f.state = state; f } else { - self.friends.push(Friend { id, name, state }); + self.friends.push(Friend { + id, + name, + state, + share: true, + }); self.friends.last_mut().expect("just pushed") } } @@ -186,6 +201,31 @@ mod tests { assert_eq!(back.friends, store.friends); } + #[test] + fn new_friends_default_to_shared_and_survive_round_trip() { + let mut store = FriendStore::default(); + let id = sample_id(); + store.upsert(id, "Alice".into(), FriendState::Accepted); + assert!(store.find(&id).unwrap().share, "new friends start shared"); + + // An older friends.toml predating the field loads as share-with-all. + let toml = format!("[[friends]]\nid = \"{id}\"\nname = \"Legacy\"\nstate = \"accepted\"\n"); + let back: FriendStore = toml::from_str(&toml).unwrap(); + assert!(back.friends[0].share); + } + + #[test] + fn upsert_preserves_share_across_refresh() { + let mut store = FriendStore::default(); + let id = sample_id(); + store.upsert(id, "Alice".into(), FriendState::Accepted); + store.find_mut(&id).unwrap().share = false; + // A later name/presence refresh re-upserts the same peer; the share + // choice must not be reset by it. + store.upsert(id, "Alice (new name)".into(), FriendState::Accepted); + assert!(!store.find(&id).unwrap().share); + } + #[test] fn upsert_updates_in_place() { let mut store = FriendStore::default(); diff --git a/src/gui/mod.rs b/src/gui/mod.rs index ff1f670..406de59 100644 --- a/src/gui/mod.rs +++ b/src/gui/mod.rs @@ -66,7 +66,7 @@ use winit::event_loop::{ActiveEventLoop, ControlFlow, EventLoop, EventLoopProxy} use winit::raw_window_handle::HasWindowHandle as _; use winit::window::{Window, WindowAttributes, WindowId}; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use self::child::{ChildEvent, ChildProc}; use self::presence::{PresenceEvent, PresenceHandle}; @@ -645,7 +645,6 @@ pub fn run(relay: Option) -> anyhow::Result<()> { friends, display_name: gui_settings.display_name, met: Vec::new(), - share_excluded: BTreeSet::new(), share_status: BTreeMap::new(), notices: Vec::new(), show_notices: false, @@ -713,17 +712,15 @@ fn control_hello(name: &str) -> ControlMsg { } } -/// The accepted friends a host's share push targets: every accepted friend the -/// user hasn't excluded on the host form. A free function (over the store + the -/// exclusion set) so the selection rule is unit-testable without a live UI. -fn selected_share_targets( - friends: &FriendStore, - excluded: &BTreeSet, -) -> Vec { +/// The accepted friends a host's share push targets: every accepted friend +/// whose per-friend `share` flag is on (toggled on the host form, persisted in +/// `friends.toml`). A free function over the store so the rule is unit-testable +/// without a live UI. +fn selected_share_targets(friends: &FriendStore) -> Vec { friends .friends .iter() - .filter(|f| f.state == FriendState::Accepted && !excluded.contains(&f.id)) + .filter(|f| f.state == FriendState::Accepted && f.share) .map(|f| f.id) .collect() } @@ -956,11 +953,6 @@ struct PixelPassApp { /// Peers met this session (over a connection) who aren't yet in the friends /// list — drives the "add friend" offer. Session-scoped, not persisted. met: Vec, - /// Accepted friends *excluded* from the host's share push, toggled on the - /// host form. Stored as the exclusion (not the inclusion) so the default — - /// an empty set — means "share with everyone," and a friend added mid-session - /// is included automatically. Session-scoped. - share_excluded: BTreeSet, /// Delivery state for the current host session's share campaign: a friend is /// present once targeted, `true` once their ACK arrives. Drives the live /// "delivered / retrying" list on the running host screen. Cleared on stop. @@ -1225,7 +1217,7 @@ impl PixelPassApp { let Some(code) = self.host.share_code.clone() else { return; }; - let targets = selected_share_targets(&self.friends, &self.share_excluded); + let targets = selected_share_targets(&self.friends); if targets.is_empty() { return; } @@ -1984,12 +1976,12 @@ impl PixelPassApp { if self.presence.is_none() { return; } - let accepted: Vec<(iroh::EndpointId, String)> = self + let accepted: Vec<(iroh::EndpointId, String, bool)> = self .friends .friends .iter() .filter(|f| f.state == FriendState::Accepted) - .map(|f| (f.id, f.name.clone())) + .map(|f| (f.id, f.name.clone(), f.share)) .collect(); if accepted.is_empty() { return; @@ -1997,14 +1989,13 @@ impl PixelPassApp { ui.add_space(12.0); ui.separator(); ui.label("Auto-share my code with:"); - for (id, name) in &accepted { - let mut on = !self.share_excluded.contains(id); + for (id, name, share) in &accepted { + let mut on = *share; if ui.checkbox(&mut on, name.as_str()).changed() { - if on { - self.share_excluded.remove(id); - } else { - self.share_excluded.insert(*id); + if let Some(f) = self.friends.find_mut(id) { + f.share = on; } + self.save_friends(); } } ui.label( @@ -2714,7 +2705,7 @@ mod tests { } #[test] - fn share_targets_are_accepted_friends_minus_excluded() { + fn share_targets_are_accepted_friends_with_share_on() { let mut friends = FriendStore::default(); let alice = id(); let bob = id(); @@ -2723,15 +2714,15 @@ mod tests { friends.upsert(bob, "Bob".into(), FriendState::Accepted); friends.upsert(pending, "Pat".into(), FriendState::PendingIncoming); - // No exclusions → every accepted friend, and never a pending one. - let all = selected_share_targets(&friends, &BTreeSet::new()); + // Default (share on for all) → every accepted friend, never a pending one. + let all = selected_share_targets(&friends); assert_eq!(all.len(), 2); assert!(all.contains(&alice) && all.contains(&bob)); assert!(!all.contains(&pending)); - // Excluding Bob drops only Bob. - let excluded: BTreeSet<_> = [bob].into_iter().collect(); - let some = selected_share_targets(&friends, &excluded); + // Turning Bob's share off drops only Bob. + friends.find_mut(&bob).unwrap().share = false; + let some = selected_share_targets(&friends); assert_eq!(some, vec![alice]); } @@ -2739,6 +2730,6 @@ mod tests { fn share_targets_empty_without_accepted_friends() { let mut friends = FriendStore::default(); friends.upsert(id(), "Out".into(), FriendState::PendingOutgoing); - assert!(selected_share_targets(&friends, &BTreeSet::new()).is_empty()); + assert!(selected_share_targets(&friends).is_empty()); } }