From d7008cd0652912d50cd962f870d230d988c0dbdf Mon Sep 17 00:00:00 2001
From: drendog <dwenya@dren.dog>
Date: Sun, 18 Jan 2026 21:58:04 +0100
Subject: [PATCH] refactor: improve popup state management for compile-time
 safety

---
 crates/composition/src/lib.rs           |  2 +-
 crates/composition/src/popup/mod.rs     |  7 +++-
 crates/composition/src/popup_builder.rs | 56 ++++++++++++++++---------
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/crates/composition/src/lib.rs b/crates/composition/src/lib.rs
index 5872cae..1c1b1d6 100644
--- a/crates/composition/src/lib.rs
+++ b/crates/composition/src/lib.rs
@@ -42,7 +42,7 @@ pub use layer_shika_domain::value_objects::{
 pub use layer_surface::{LayerSurfaceHandle, ShellSurfaceConfigHandler};
 pub use lock_selection::LockSelection;
 pub use popup::PopupShell;
-pub use popup_builder::PopupBuilder;
+pub use popup_builder::{Bound, PopupBuilder, Unbound};
 pub use selection::{PropertyError, Selection, SelectionResult};
 pub use selector::{Output, Selector, Surface, SurfaceInfo};
 pub use session_lock::{SessionLock, SessionLockBuilder};
diff --git a/crates/composition/src/popup/mod.rs b/crates/composition/src/popup/mod.rs
index b796f2e..4f3ad22 100644
--- a/crates/composition/src/popup/mod.rs
+++ b/crates/composition/src/popup/mod.rs
@@ -1,4 +1,4 @@
-use crate::popup_builder::PopupBuilder;
+use crate::popup_builder::{Bound, PopupBuilder};
 use crate::system::{PopupCommand, ShellCommand, ShellControl};
 use crate::{Error, Result};
 use layer_shika_adapters::platform::calloop::channel;
@@ -17,8 +17,11 @@ impl PopupShell {
         Self { sender }
     }
 
+    /// Creates a popup builder bound to this shell
+    ///
+    /// The returned builder can call `.show()` directly because it's bound to a shell.
     #[must_use]
-    pub fn builder(&self, component: impl Into<String>) -> PopupBuilder {
+    pub fn builder(&self, component: impl Into<String>) -> PopupBuilder<Bound> {
         PopupBuilder::new(component).with_shell(self.clone())
     }
 
diff --git a/crates/composition/src/popup_builder.rs b/crates/composition/src/popup_builder.rs
index 5b9f7e3..18ef3d7 100644
--- a/crates/composition/src/popup_builder.rs
+++ b/crates/composition/src/popup_builder.rs
@@ -1,7 +1,6 @@
+use crate::Result;
 use crate::popup::PopupShell;
-use crate::{Error, Result};
 use layer_shika_domain::dimensions::LogicalRect;
-use layer_shika_domain::errors::DomainError;
 use layer_shika_domain::value_objects::handle::PopupHandle;
 use layer_shika_domain::value_objects::output_target::OutputTarget;
 use layer_shika_domain::value_objects::popup_behavior::ConstraintAdjustment;
@@ -11,9 +10,19 @@ use layer_shika_domain::value_objects::popup_position::{
 };
 use layer_shika_domain::value_objects::popup_size::PopupSize;
 
+/// Type state indicating the builder is not bound to a shell
+pub struct Unbound;
+
+/// Type state indicating the builder is bound to a shell
+pub struct Bound {
+    shell: PopupShell,
+}
+
 /// Builder for configuring popups
 ///
-/// Produces a [`PopupConfig`] and can show it via [`PopupShell`].
+/// The builder uses phantom types to ensure compile-time safety:
+/// - [`PopupBuilder<Unbound>`] - Configuration only, cannot show popups
+/// - [`PopupBuilder<Bound>`] - Has shell reference, can show popups
 ///
 /// # Example
 /// ```rust,ignore
@@ -25,27 +34,34 @@ use layer_shika_domain::value_objects::popup_size::PopupSize;
 ///         .show()?;
 /// });
 /// ```
-pub struct PopupBuilder {
-    shell: Option<PopupShell>,
+pub struct PopupBuilder<State = Unbound> {
+    state: State,
     config: PopupConfig,
 }
 
-impl PopupBuilder {
+impl PopupBuilder<Unbound> {
     /// Creates a new popup builder for the specified component
+    ///
+    /// This builder is unbound and cannot show popups directly.
+    /// Use [`PopupShell::builder`] to create a bound builder that can call `.show()`.
     #[must_use]
     pub fn new(component: impl Into<String>) -> Self {
         Self {
-            shell: None,
+            state: Unbound,
             config: PopupConfig::new(component),
         }
     }
 
     #[must_use]
-    pub(crate) fn with_shell(mut self, shell: PopupShell) -> Self {
-        self.shell = Some(shell);
-        self
+    pub(crate) fn with_shell(self, shell: PopupShell) -> PopupBuilder<Bound> {
+        PopupBuilder {
+            state: Bound { shell },
+            config: self.config,
+        }
     }
+}
 
+impl<State> PopupBuilder<State> {
     #[must_use]
     pub fn position(mut self, position: PopupPosition) -> Self {
         self.config.position = position;
@@ -203,21 +219,21 @@ impl PopupBuilder {
         self
     }
 
+    /// Builds the configuration without showing the popup
+    ///
+    /// Returns a [`PopupConfig`] that can be shown later using [`PopupShell::show`].
     #[must_use]
     pub fn build(self) -> PopupConfig {
         self.config
     }
+}
 
+impl PopupBuilder<Bound> {
+    /// Shows the popup with the configured settings
+    ///
+    /// This method is only available on builders created via [`PopupShell::builder`],
+    /// ensuring at compile time that the builder has access to a shell.
     pub fn show(self) -> Result<PopupHandle> {
-        let Some(shell) = self.shell else {
-            return Err(Error::Domain(DomainError::Configuration {
-                message: "PopupBuilder::show() requires a builder created via `shell.popups().builder(...)`".to_string(),
-            }));
-        };
-        shell.show(self.config)
-    }
-
-    pub fn show_with_shell(self, shell: &PopupShell) -> Result<PopupHandle> {
-        shell.show(self.build())
+        self.state.shell.show(self.config)
     }
 }