5c810e5 · helix · git.iko.soy

git.iko.soy/helix

Fix bug with auto replacing components in compositor (#1711)

* Fix bug with auto replacing components in compositor This was last known to be working with 5995568c at the time of commit, but now doesn't work with latest rust stable. The issue probably stems from using std::any::type_name() for finding a component in the compositor, for which the docs explicitly warn against considering it as a unique identifier for types. `replace_or_push()` takes a boxed `Component` and passes it to `find_id()` which compares this with a bare Component. `type_name()` returns `Box<T>` for the former and `T` for latter and we have a false negative. This has been solved by using a generics instead of trait objects to pass in a `T: Component` and then use it for comparison. I'm not exactly sure how this worked fine at the time of commit of 5995568c; maybe the internal implementation of `type_name()` changed to properly indicate indirection with Box. * Do not compare by type name in compositor find_id

commit5c810e5e52a3409a5e70aeae14e6f28dd440c681 authorGokul Soumya <gokulps15@gmail.com> dateThu 3 Mar 2022, 06:44 (5 years ago) parent86b1236

3 files changed+6−7

compare with master

unifiedsplit

helix-term/src/commands.rs+1−1 helix-term/src/commands/lsp.rs+2−2 helix-term/src/compositor.rs+3−4

helix-term/src/commands.rs+1−1

@@ -2980,7 +2980,7 @@ pub mod cmd {

29802980 Box::new(move |editor: &mut Editor, compositor: &mut Compositor| {

29812981 let contents = ui::Markdown::new(contents, editor.syn_loader.clone());

29822982 let popup = Popup::new("hover", contents).auto_close(true);

2983− compositor.replace_or_push("hover", Box::new(popup));

2983+ compositor.replace_or_push("hover", popup);

29842984 });

29852985 Ok(call)

29862986 };

helix-term/src/commands/lsp.rs+2−2

@@ -254,7 +254,7 @@ pub fn code_action(cx: &mut Context) {

254254 vertical: 1,

255255 horizontal: 1,

256256 });

257− compositor.replace_or_push("code-action", Box::new(popup));

257+ compositor.replace_or_push("code-action", popup);

258258 },

259259 )

260260}

@@ -637,7 +637,7 @@ pub fn hover(cx: &mut Context) {

637637

638638 let contents = ui::Markdown::new(contents, editor.syn_loader.clone());

639639 let popup = Popup::new("hover", contents).auto_close(true);

640− compositor.replace_or_push("hover", Box::new(popup));

640+ compositor.replace_or_push("hover", popup);

641641 }

642642 },

643643 );

helix-term/src/compositor.rs+3−4

@@ -128,11 +128,11 @@ impl Compositor {

128128

129129 /// Replace a component that has the given `id` with the new layer and if

130130 /// no component is found, push the layer normally.

131− pub fn replace_or_push(&mut self, id: &'static str, layer: Box<dyn Component>) {

131+ pub fn replace_or_push<T: Component>(&mut self, id: &'static str, layer: T) {

132132 if let Some(component) = self.find_id(id) {

133133 *component = layer;

134134 } else {

135− self.push(layer)

135+ self.push(Box::new(layer))

136136 }

137137 }

138138

@@ -221,10 +221,9 @@ impl Compositor {

221221 }

222222

223223 pub fn find_id<T: 'static>(&mut self, id: &'static str) -> Option<&mut T> {

224− let type_name = std::any::type_name::<T>();

225224 self.layers

226225 .iter_mut()

227− .find(|component| component.type_name() == type_name && component.id() == Some(id))

226+ .find(|component| component.id() == Some(id))

228227 .and_then(|component| component.as_any_mut().downcast_mut())

229228 }

230229}