Mikayla Maki created
TODO:
- [x] Audit all TODO comments in Zed source and mark port related ones
with a !
- [x] Resolve all todos written as `todo!`
Release Notes:
- N/Acrates/call/src/call.rs | 3
crates/collab/src/tests/channel_message_tests.rs | 266 +++++++++--------
crates/collab_ui/src/collab_ui.rs | 1
crates/editor/src/display_map/wrap_map.rs | 2
crates/editor/src/element.rs | 1
crates/file_finder/src/file_finder.rs | 2
crates/gpui/src/platform/mac/window.rs | 9
crates/gpui/src/text_system/line_wrapper.rs | 131 ++++----
crates/gpui/src/window.rs | 11
crates/search/src/buffer_search.rs | 2
crates/theme/src/styles/players.rs | 2
crates/zed/src/zed.rs | 1
12 files changed, 218 insertions(+), 213 deletions(-)
@@ -239,7 +239,8 @@ impl ActiveCall {
if result.is_ok() {
this.update(&mut cx, |this, cx| this.report_call_event("invite", cx))?;
} else {
- // TODO: Resport collaboration error
+ //TODO: report collaboration error
+ log::error!("invite failed: {:?}", result);
}
this.update(&mut cx, |this, cx| {
@@ -1,7 +1,9 @@
use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer};
use channel::{ChannelChat, ChannelMessageId, MessageParams};
+use collab_ui::chat_panel::ChatPanel;
use gpui::{BackgroundExecutor, Model, TestAppContext};
use rpc::Notification;
+use workspace::dock::Panel;
#[gpui::test]
async fn test_basic_channel_messages(
@@ -273,135 +275,135 @@ fn assert_messages(chat: &Model<ChannelChat>, messages: &[&str], cx: &mut TestAp
);
}
-//todo!(collab_ui)
-// #[gpui::test]
-// async fn test_channel_message_changes(
-// executor: BackgroundExecutor,
-// cx_a: &mut TestAppContext,
-// cx_b: &mut TestAppContext,
-// ) {
-// let mut server = TestServer::start(&executor).await;
-// let client_a = server.create_client(cx_a, "user_a").await;
-// let client_b = server.create_client(cx_b, "user_b").await;
-
-// let channel_id = server
-// .make_channel(
-// "the-channel",
-// None,
-// (&client_a, cx_a),
-// &mut [(&client_b, cx_b)],
-// )
-// .await;
-
-// // Client A sends a message, client B should see that there is a new message.
-// let channel_chat_a = client_a
-// .channel_store()
-// .update(cx_a, |store, cx| store.open_channel_chat(channel_id, cx))
-// .await
-// .unwrap();
-
-// channel_chat_a
-// .update(cx_a, |c, cx| c.send_message("one".into(), cx).unwrap())
-// .await
-// .unwrap();
-
-// executor.run_until_parked();
-
-// let b_has_messages = cx_b.read_with(|cx| {
-// client_b
-// .channel_store()
-// .read(cx)
-// .has_new_messages(channel_id)
-// .unwrap()
-// });
-
-// assert!(b_has_messages);
-
-// // Opening the chat should clear the changed flag.
-// cx_b.update(|cx| {
-// collab_ui::init(&client_b.app_state, cx);
-// });
-// let project_b = client_b.build_empty_local_project(cx_b);
-// let workspace_b = client_b.build_workspace(&project_b, cx_b).root(cx_b);
-// let chat_panel_b = workspace_b.update(cx_b, |workspace, cx| ChatPanel::new(workspace, cx));
-// chat_panel_b
-// .update(cx_b, |chat_panel, cx| {
-// chat_panel.set_active(true, cx);
-// chat_panel.select_channel(channel_id, None, cx)
-// })
-// .await
-// .unwrap();
-
-// executor.run_until_parked();
-
-// let b_has_messages = cx_b.read_with(|cx| {
-// client_b
-// .channel_store()
-// .read(cx)
-// .has_new_messages(channel_id)
-// .unwrap()
-// });
-
-// assert!(!b_has_messages);
-
-// // Sending a message while the chat is open should not change the flag.
-// channel_chat_a
-// .update(cx_a, |c, cx| c.send_message("two".into(), cx).unwrap())
-// .await
-// .unwrap();
-
-// executor.run_until_parked();
-
-// let b_has_messages = cx_b.read_with(|cx| {
-// client_b
-// .channel_store()
-// .read(cx)
-// .has_new_messages(channel_id)
-// .unwrap()
-// });
-
-// assert!(!b_has_messages);
-
-// // Sending a message while the chat is closed should change the flag.
-// chat_panel_b.update(cx_b, |chat_panel, cx| {
-// chat_panel.set_active(false, cx);
-// });
-
-// // Sending a message while the chat is open should not change the flag.
-// channel_chat_a
-// .update(cx_a, |c, cx| c.send_message("three".into(), cx).unwrap())
-// .await
-// .unwrap();
-
-// executor.run_until_parked();
-
-// let b_has_messages = cx_b.read_with(|cx| {
-// client_b
-// .channel_store()
-// .read(cx)
-// .has_new_messages(channel_id)
-// .unwrap()
-// });
-
-// assert!(b_has_messages);
-
-// // Closing the chat should re-enable change tracking
-// cx_b.update(|_| drop(chat_panel_b));
-
-// channel_chat_a
-// .update(cx_a, |c, cx| c.send_message("four".into(), cx).unwrap())
-// .await
-// .unwrap();
-
-// executor.run_until_parked();
-
-// let b_has_messages = cx_b.read_with(|cx| {
-// client_b
-// .channel_store()
-// .read(cx)
-// .has_new_messages(channel_id)
-// .unwrap()
-// });
-
-// assert!(b_has_messages);
-// }
+#[gpui::test]
+async fn test_channel_message_changes(
+ executor: BackgroundExecutor,
+ cx_a: &mut TestAppContext,
+ cx_b: &mut TestAppContext,
+) {
+ let mut server = TestServer::start(executor.clone()).await;
+ let client_a = server.create_client(cx_a, "user_a").await;
+ let client_b = server.create_client(cx_b, "user_b").await;
+
+ let channel_id = server
+ .make_channel(
+ "the-channel",
+ None,
+ (&client_a, cx_a),
+ &mut [(&client_b, cx_b)],
+ )
+ .await;
+
+ // Client A sends a message, client B should see that there is a new message.
+ let channel_chat_a = client_a
+ .channel_store()
+ .update(cx_a, |store, cx| store.open_channel_chat(channel_id, cx))
+ .await
+ .unwrap();
+
+ channel_chat_a
+ .update(cx_a, |c, cx| c.send_message("one".into(), cx).unwrap())
+ .await
+ .unwrap();
+
+ executor.run_until_parked();
+
+ let b_has_messages = cx_b.update(|cx| {
+ client_b
+ .channel_store()
+ .read(cx)
+ .has_new_messages(channel_id)
+ .unwrap()
+ });
+
+ assert!(b_has_messages);
+
+ // Opening the chat should clear the changed flag.
+ cx_b.update(|cx| {
+ collab_ui::init(&client_b.app_state, cx);
+ });
+ let project_b = client_b.build_empty_local_project(cx_b);
+ let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b);
+
+ let chat_panel_b = workspace_b.update(cx_b, |workspace, cx| ChatPanel::new(workspace, cx));
+ chat_panel_b
+ .update(cx_b, |chat_panel, cx| {
+ chat_panel.set_active(true, cx);
+ chat_panel.select_channel(channel_id, None, cx)
+ })
+ .await
+ .unwrap();
+
+ executor.run_until_parked();
+
+ let b_has_messages = cx_b.update(|cx| {
+ client_b
+ .channel_store()
+ .read(cx)
+ .has_new_messages(channel_id)
+ .unwrap()
+ });
+
+ assert!(!b_has_messages);
+
+ // Sending a message while the chat is open should not change the flag.
+ channel_chat_a
+ .update(cx_a, |c, cx| c.send_message("two".into(), cx).unwrap())
+ .await
+ .unwrap();
+
+ executor.run_until_parked();
+
+ let b_has_messages = cx_b.update(|cx| {
+ client_b
+ .channel_store()
+ .read(cx)
+ .has_new_messages(channel_id)
+ .unwrap()
+ });
+
+ assert!(!b_has_messages);
+
+ // Sending a message while the chat is closed should change the flag.
+ chat_panel_b.update(cx_b, |chat_panel, cx| {
+ chat_panel.set_active(false, cx);
+ });
+
+ // Sending a message while the chat is open should not change the flag.
+ channel_chat_a
+ .update(cx_a, |c, cx| c.send_message("three".into(), cx).unwrap())
+ .await
+ .unwrap();
+
+ executor.run_until_parked();
+
+ let b_has_messages = cx_b.update(|cx| {
+ client_b
+ .channel_store()
+ .read(cx)
+ .has_new_messages(channel_id)
+ .unwrap()
+ });
+
+ assert!(b_has_messages);
+
+ // Closing the chat should re-enable change tracking
+ cx_b.update(|_| drop(chat_panel_b));
+
+ channel_chat_a
+ .update(cx_a, |c, cx| c.send_message("four".into(), cx).unwrap())
+ .await
+ .unwrap();
+
+ executor.run_until_parked();
+
+ let b_has_messages = cx_b.update(|cx| {
+ client_b
+ .channel_store()
+ .read(cx)
+ .has_new_messages(channel_id)
+ .unwrap()
+ });
+
+ assert!(b_has_messages);
+}
@@ -111,7 +111,6 @@ fn notification_window_options(
let screen_bounds = screen.bounds();
let size: Size<GlobalPixels> = window_size.into();
- // todo!() use content bounds instead of screen.bounds and get rid of magics in point's 2nd argument.
let bounds = gpui::Bounds::<GlobalPixels> {
origin: screen_bounds.upper_right()
- point(
@@ -1043,7 +1043,7 @@ mod tests {
#[gpui::test(iterations = 100)]
async fn test_random_wraps(cx: &mut gpui::TestAppContext, mut rng: StdRng) {
- // todo!() this test is flaky
+ // todo this test is flaky
init_test(cx);
cx.background_executor.set_block_on_ticks(0..=50);
@@ -1017,7 +1017,6 @@ impl EditorElement {
.chars_at(cursor_position)
.next()
.and_then(|(character, _)| {
- // todo!() currently shape_line panics if text conatins newlines
let text = if character == '\n' {
SharedString::from(" ")
} else {
@@ -312,7 +312,7 @@ impl FileFinderDelegate {
cx: &mut ViewContext<FileFinder>,
) -> Self {
cx.observe(&project, |file_finder, _, cx| {
- //todo!() We should probably not re-render on every project anything
+ //todo We should probably not re-render on every project anything
file_finder
.picker
.update(cx, |picker, cx| picker.refresh(cx))
@@ -269,6 +269,7 @@ unsafe fn build_window_class(name: &'static str, superclass: &Class) -> *const C
sel!(windowShouldClose:),
window_should_close as extern "C" fn(&Object, Sel, id) -> BOOL,
);
+
decl.add_method(sel!(close), close_window as extern "C" fn(&Object, Sel));
decl.add_method(
@@ -685,9 +686,6 @@ impl Drop for MacWindow {
this.executor
.spawn(async move {
unsafe {
- // todo!() this panic()s when you click the red close button
- // unless should_close returns false.
- // (luckliy in zed it always returns false)
window.close();
}
})
@@ -1116,7 +1114,7 @@ extern "C" fn handle_key_event(this: &Object, native_event: id, key_equivalent:
// we don't match cmd/fn because they don't seem to use IME
modifiers: Default::default(),
key: ime_text.clone().unwrap(),
- ime_key: None, // todo!("handle IME key")
+ ime_key: None,
},
};
handled = callback(InputEvent::KeyDown(event_with_ime_text));
@@ -1570,6 +1568,9 @@ extern "C" fn insert_text(this: &Object, _: Sel, text: id, replacement_range: NS
replacement_range,
text: text.to_string(),
});
+ if text.to_string().to_ascii_lowercase() != pending_key_down.0.keystroke.key {
+ pending_key_down.0.keystroke.ime_key = Some(text.to_string());
+ }
window_state.lock().pending_key_down = Some(pending_key_down);
}
}
@@ -137,7 +137,7 @@ impl Boundary {
#[cfg(test)]
mod tests {
use super::*;
- use crate::{font, TestAppContext, TestDispatcher};
+ use crate::{font, TestAppContext, TestDispatcher, TextRun, WrapBoundary};
use rand::prelude::*;
#[test]
@@ -206,75 +206,70 @@ mod tests {
});
}
- // todo!("move this to a test on TextSystem::layout_text")
- // todo! repeat this test
- // #[test]
- // fn test_wrap_shaped_line() {
- // App::test().run(|cx| {
- // let text_system = cx.text_system().clone();
+ // For compatibility with the test macro
+ use crate as gpui;
- // let normal = TextRun {
- // len: 0,
- // font: font("Helvetica"),
- // color: Default::default(),
- // underline: Default::default(),
- // };
- // let bold = TextRun {
- // len: 0,
- // font: font("Helvetica").bold(),
- // color: Default::default(),
- // underline: Default::default(),
- // };
+ #[crate::test]
+ fn test_wrap_shaped_line(cx: &mut TestAppContext) {
+ cx.update(|cx| {
+ let text_system = cx.text_system().clone();
+
+ let normal = TextRun {
+ len: 0,
+ font: font("Helvetica"),
+ color: Default::default(),
+ underline: Default::default(),
+ background_color: None,
+ };
+ let bold = TextRun {
+ len: 0,
+ font: font("Helvetica").bold(),
+ color: Default::default(),
+ underline: Default::default(),
+ background_color: None,
+ };
- // impl TextRun {
- // fn with_len(&self, len: usize) -> Self {
- // let mut this = self.clone();
- // this.len = len;
- // this
- // }
- // }
+ impl TextRun {
+ fn with_len(&self, len: usize) -> Self {
+ let mut this = self.clone();
+ this.len = len;
+ this
+ }
+ }
- // let text = "aa bbb cccc ddddd eeee".into();
- // let lines = text_system
- // .layout_text(
- // &text,
- // px(16.),
- // &[
- // normal.with_len(4),
- // bold.with_len(5),
- // normal.with_len(6),
- // bold.with_len(1),
- // normal.with_len(7),
- // ],
- // None,
- // )
- // .unwrap();
- // let line = &lines[0];
+ let text = "aa bbb cccc ddddd eeee".into();
+ let lines = text_system
+ .shape_text(
+ text,
+ px(16.),
+ &[
+ normal.with_len(4),
+ bold.with_len(5),
+ normal.with_len(6),
+ bold.with_len(1),
+ normal.with_len(7),
+ ],
+ Some(px(72.)),
+ )
+ .unwrap();
- // let mut wrapper = LineWrapper::new(
- // text_system.font_id(&normal.font).unwrap(),
- // px(16.),
- // text_system.platform_text_system.clone(),
- // );
- // assert_eq!(
- // wrapper
- // .wrap_shaped_line(&text, &line, px(72.))
- // .collect::<Vec<_>>(),
- // &[
- // ShapedBoundary {
- // run_ix: 1,
- // glyph_ix: 3
- // },
- // ShapedBoundary {
- // run_ix: 2,
- // glyph_ix: 3
- // },
- // ShapedBoundary {
- // run_ix: 4,
- // glyph_ix: 2
- // }
- // ],
- // );
- // });
- // }
+ assert_eq!(
+ lines[0].layout.wrap_boundaries(),
+ &[
+ WrapBoundary {
+ run_ix: 1,
+ glyph_ix: 3
+ },
+ WrapBoundary {
+ run_ix: 2,
+ glyph_ix: 3
+ },
+ WrapBoundary {
+ run_ix: 4,
+ glyph_ix: 2
+ }
+ ],
+ );
+ });
+ }
}
@@ -1904,7 +1904,16 @@ impl<'a> WindowContext<'a> {
let mut this = self.to_async();
self.window
.platform_window
- .on_should_close(Box::new(move || this.update(|_, cx| f(cx)).unwrap_or(true)))
+ .on_should_close(Box::new(move || {
+ this.update(|_, cx| {
+ // Ensure that the window is removed from the app if it's been closed.
+ if f(cx) {
+ cx.remove_window();
+ }
+ false
+ })
+ .unwrap_or(true)
+ }))
}
}
@@ -1130,7 +1130,6 @@ mod tests {
#[gpui::test]
async fn test_search_simple(cx: &mut TestAppContext) {
let (editor, search_bar, cx) = init_test(cx);
- // todo! osiewicz: these tests asserted on background color as well, that should be brought back.
let display_points_of = |background_highlights: Vec<(Range<DisplayPoint>, Hsla)>| {
background_highlights
.into_iter()
@@ -1395,7 +1394,6 @@ mod tests {
})
.await
.unwrap();
- // todo! osiewicz: these tests previously asserted on background color highlights; that should be introduced back.
let display_points_of = |background_highlights: Vec<(Range<DisplayPoint>, Hsla)>| {
background_highlights
.into_iter()
@@ -22,7 +22,7 @@ pub struct PlayerColors(pub Vec<PlayerColor>);
impl Default for PlayerColors {
/// Don't use this!
/// We have to have a default to be `[refineable::Refinable]`.
- /// todo!("Find a way to not need this for Refinable")
+ /// TODO "Find a way to not need this for Refinable"
fn default() -> Self {
Self::dark()
}
@@ -148,6 +148,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
cx.on_window_should_close(move |cx| {
handle
.update(cx, |workspace, cx| {
+ // We'll handle closing asynchoronously
workspace.close_window(&Default::default(), cx);
false
})