Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Buffer::apply_ops infallible #18089

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions crates/assistant/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ impl Context {
buffer.set_text(saved_context.text.as_str(), cx)
});
let operations = saved_context.into_ops(&this.buffer, cx);
this.apply_ops(operations, cx).unwrap();
this.apply_ops(operations, cx);
this
}

Expand Down Expand Up @@ -756,7 +756,7 @@ impl Context {
&mut self,
ops: impl IntoIterator<Item = ContextOperation>,
cx: &mut ModelContext<Self>,
) -> Result<()> {
) {
let mut buffer_ops = Vec::new();
for op in ops {
match op {
Expand All @@ -765,10 +765,8 @@ impl Context {
}
}
self.buffer
.update(cx, |buffer, cx| buffer.apply_ops(buffer_ops, cx))?;
.update(cx, |buffer, cx| buffer.apply_ops(buffer_ops, cx));
self.flush_ops(cx);

Ok(())
}

fn flush_ops(&mut self, cx: &mut ModelContext<Context>) {
Expand Down
8 changes: 2 additions & 6 deletions crates/assistant/src/context/context_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1166,9 +1166,7 @@ async fn test_random_context_collaboration(cx: &mut TestAppContext, mut rng: Std
);

network.lock().broadcast(replica_id, ops_to_send);
context
.update(cx, |context, cx| context.apply_ops(ops_to_receive, cx))
.unwrap();
context.update(cx, |context, cx| context.apply_ops(ops_to_receive, cx));
} else if rng.gen_bool(0.1) && replica_id != 0 {
log::info!("Context {}: disconnecting", context_index);
network.lock().disconnect_peer(replica_id);
Expand All @@ -1180,9 +1178,7 @@ async fn test_random_context_collaboration(cx: &mut TestAppContext, mut rng: Std
.map(ContextOperation::from_proto)
.collect::<Result<Vec<_>>>()
.unwrap();
context
.update(cx, |context, cx| context.apply_ops(ops, cx))
.unwrap();
context.update(cx, |context, cx| context.apply_ops(ops, cx));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/assistant/src/context_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl ContextStore {
if let Some(context) = this.loaded_context_for_id(&context_id, cx) {
let operation_proto = envelope.payload.operation.context("invalid operation")?;
let operation = ContextOperation::from_proto(operation_proto)?;
context.update(cx, |context, cx| context.apply_ops([operation], cx))?;
context.update(cx, |context, cx| context.apply_ops([operation], cx));
}
Ok(())
})?
Expand Down Expand Up @@ -394,7 +394,7 @@ impl ContextStore {
.collect::<Result<Vec<_>>>()
})
.await?;
context.update(&mut cx, |context, cx| context.apply_ops(operations, cx))??;
context.update(&mut cx, |context, cx| context.apply_ops(operations, cx))?;
this.update(&mut cx, |this, cx| {
if let Some(existing_context) = this.loaded_context_for_id(&context_id, cx) {
existing_context
Expand Down Expand Up @@ -531,7 +531,7 @@ impl ContextStore {
.collect::<Result<Vec<_>>>()
})
.await?;
context.update(&mut cx, |context, cx| context.apply_ops(operations, cx))??;
context.update(&mut cx, |context, cx| context.apply_ops(operations, cx))?;
this.update(&mut cx, |this, cx| {
if let Some(existing_context) = this.loaded_context_for_id(&context_id, cx) {
existing_context
Expand Down
4 changes: 2 additions & 2 deletions crates/channel/src/channel_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl ChannelBuffer {
let capability = channel_store.read(cx).channel_capability(channel.id);
language::Buffer::remote(buffer_id, response.replica_id as u16, capability, base_text)
})?;
buffer.update(&mut cx, |buffer, cx| buffer.apply_ops(operations, cx))??;
buffer.update(&mut cx, |buffer, cx| buffer.apply_ops(operations, cx))?;

let subscription = client.subscribe_to_entity(channel.id.0)?;

Expand Down Expand Up @@ -151,7 +151,7 @@ impl ChannelBuffer {
cx.notify();
this.buffer
.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))
})??;
})?;

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/channel/src/channel_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,7 @@ impl ChannelStore {
.into_iter()
.map(language::proto::deserialize_operation)
.collect::<Result<Vec<_>>>()?;
buffer.apply_ops(incoming_operations, cx)?;
buffer.apply_ops(incoming_operations, cx);
anyhow::Ok(outgoing_operations)
})
.log_err();
Expand Down
4 changes: 1 addition & 3 deletions crates/collab/src/db/queries/buffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,9 +689,7 @@ impl Database {
}

let mut text_buffer = text::Buffer::new(0, text::BufferId::new(1).unwrap(), base_text);
text_buffer
.apply_ops(operations.into_iter().filter_map(operation_from_wire))
.unwrap();
text_buffer.apply_ops(operations.into_iter().filter_map(operation_from_wire));

let base_text = text_buffer.text();
let epoch = buffer.epoch + 1;
Expand Down
18 changes: 8 additions & 10 deletions crates/collab/src/db/tests/buffer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,14 @@ async fn test_channel_buffers(db: &Arc<Database>) {
text::BufferId::new(1).unwrap(),
buffer_response_b.base_text,
);
buffer_b
.apply_ops(buffer_response_b.operations.into_iter().map(|operation| {
let operation = proto::deserialize_operation(operation).unwrap();
if let language::Operation::Buffer(operation) = operation {
operation
} else {
unreachable!()
}
}))
.unwrap();
buffer_b.apply_ops(buffer_response_b.operations.into_iter().map(|operation| {
let operation = proto::deserialize_operation(operation).unwrap();
if let language::Operation::Buffer(operation) = operation {
operation
} else {
unreachable!()
}
}));

assert_eq!(buffer_b.text(), "hello, cruel world");

Expand Down
5 changes: 2 additions & 3 deletions crates/language/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1972,7 +1972,7 @@ impl Buffer {
&mut self,
ops: I,
cx: &mut ModelContext<Self>,
) -> Result<()> {
) {
self.pending_autoindent.take();
let was_dirty = self.is_dirty();
let old_version = self.version.clone();
Expand All @@ -1991,14 +1991,13 @@ impl Buffer {
}
})
.collect::<Vec<_>>();
self.text.apply_ops(buffer_ops)?;
self.text.apply_ops(buffer_ops);
self.deferred_ops.insert(deferred_ops);
self.flush_deferred_ops(cx);
self.did_edit(&old_version, was_dirty, cx);
// Notify independently of whether the buffer was edited as the operations could include a
// selection update.
cx.notify();
Ok(())
}

fn flush_deferred_ops(&mut self, cx: &mut ModelContext<Self>) {
Expand Down
46 changes: 20 additions & 26 deletions crates/language/src/buffer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ fn test_edit_events(cx: &mut gpui::AppContext) {
// Incorporating a set of remote ops emits a single edited event,
// followed by a dirty changed event.
buffer2.update(cx, |buffer, cx| {
buffer.apply_ops(buffer1_ops.lock().drain(..), cx).unwrap();
buffer.apply_ops(buffer1_ops.lock().drain(..), cx);
});
assert_eq!(
mem::take(&mut *buffer_1_events.lock()),
Expand All @@ -332,7 +332,7 @@ fn test_edit_events(cx: &mut gpui::AppContext) {
// Incorporating the remote ops again emits a single edited event,
// followed by a dirty changed event.
buffer2.update(cx, |buffer, cx| {
buffer.apply_ops(buffer1_ops.lock().drain(..), cx).unwrap();
buffer.apply_ops(buffer1_ops.lock().drain(..), cx);
});
assert_eq!(
mem::take(&mut *buffer_1_events.lock()),
Expand Down Expand Up @@ -2274,13 +2274,11 @@ fn test_serialization(cx: &mut gpui::AppContext) {
.block(buffer1.read(cx).serialize_ops(None, cx));
let buffer2 = cx.new_model(|cx| {
let mut buffer = Buffer::from_proto(1, Capability::ReadWrite, state, None).unwrap();
buffer
.apply_ops(
ops.into_iter()
.map(|op| proto::deserialize_operation(op).unwrap()),
cx,
)
.unwrap();
buffer.apply_ops(
ops.into_iter()
.map(|op| proto::deserialize_operation(op).unwrap()),
cx,
);
buffer
});
assert_eq!(buffer2.read(cx).text(), "abcDF");
Expand Down Expand Up @@ -2401,13 +2399,11 @@ fn test_random_collaboration(cx: &mut AppContext, mut rng: StdRng) {
.block(base_buffer.read(cx).serialize_ops(None, cx));
let mut buffer =
Buffer::from_proto(i as ReplicaId, Capability::ReadWrite, state, None).unwrap();
buffer
.apply_ops(
ops.into_iter()
.map(|op| proto::deserialize_operation(op).unwrap()),
cx,
)
.unwrap();
buffer.apply_ops(
ops.into_iter()
.map(|op| proto::deserialize_operation(op).unwrap()),
cx,
);
buffer.set_group_interval(Duration::from_millis(rng.gen_range(0..=200)));
let network = network.clone();
cx.subscribe(&cx.handle(), move |buffer, _, event, _| {
Expand Down Expand Up @@ -2523,14 +2519,12 @@ fn test_random_collaboration(cx: &mut AppContext, mut rng: StdRng) {
None,
)
.unwrap();
new_buffer
.apply_ops(
old_buffer_ops
.into_iter()
.map(|op| deserialize_operation(op).unwrap()),
cx,
)
.unwrap();
new_buffer.apply_ops(
old_buffer_ops
.into_iter()
.map(|op| deserialize_operation(op).unwrap()),
cx,
);
log::info!(
"New replica {} text: {:?}",
new_buffer.replica_id(),
Expand Down Expand Up @@ -2570,7 +2564,7 @@ fn test_random_collaboration(cx: &mut AppContext, mut rng: StdRng) {
ops
);
new_buffer.update(cx, |new_buffer, cx| {
new_buffer.apply_ops(ops, cx).unwrap();
new_buffer.apply_ops(ops, cx);
});
}
}
Expand Down Expand Up @@ -2598,7 +2592,7 @@ fn test_random_collaboration(cx: &mut AppContext, mut rng: StdRng) {
ops.len(),
ops
);
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx).unwrap());
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx));
}
}
_ => {}
Expand Down
12 changes: 5 additions & 7 deletions crates/multi_buffer/src/multi_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5019,13 +5019,11 @@ mod tests {
.background_executor()
.block(host_buffer.read(cx).serialize_ops(None, cx));
let mut buffer = Buffer::from_proto(1, Capability::ReadWrite, state, None).unwrap();
buffer
.apply_ops(
ops.into_iter()
.map(|op| language::proto::deserialize_operation(op).unwrap()),
cx,
)
.unwrap();
buffer.apply_ops(
ops.into_iter()
.map(|op| language::proto::deserialize_operation(op).unwrap()),
cx,
);
buffer
});
let multibuffer = cx.new_model(|cx| MultiBuffer::singleton(guest_buffer.clone(), cx));
Expand Down
9 changes: 5 additions & 4 deletions crates/project/src/buffer_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ impl BufferStore {
}
hash_map::Entry::Occupied(mut entry) => {
if let OpenBuffer::Operations(operations) = entry.get_mut() {
buffer.update(cx, |b, cx| b.apply_ops(operations.drain(..), cx))?;
buffer.update(cx, |b, cx| b.apply_ops(operations.drain(..), cx));
} else if entry.get().upgrade().is_some() {
if is_remote {
return Ok(());
Expand Down Expand Up @@ -1051,12 +1051,12 @@ impl BufferStore {
match this.opened_buffers.entry(buffer_id) {
hash_map::Entry::Occupied(mut e) => match e.get_mut() {
OpenBuffer::Strong(buffer) => {
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))?;
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx));
}
OpenBuffer::Operations(operations) => operations.extend_from_slice(&ops),
OpenBuffer::Weak(buffer) => {
if let Some(buffer) = buffer.upgrade() {
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))?;
buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx));
}
}
},
Expand Down Expand Up @@ -1217,7 +1217,8 @@ impl BufferStore {
.into_iter()
.map(language::proto::deserialize_operation)
.collect::<Result<Vec<_>>>()?;
buffer.update(cx, |buffer, cx| buffer.apply_ops(operations, cx))
buffer.update(cx, |buffer, cx| buffer.apply_ops(operations, cx));
anyhow::Ok(())
});

if let Err(error) = result {
Expand Down
32 changes: 16 additions & 16 deletions crates/text/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,25 +515,25 @@ fn test_undo_redo() {
let entries = buffer.history.undo_stack.clone();
assert_eq!(entries.len(), 3);

buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[0].transaction.clone());
assert_eq!(buffer.text(), "1cdef234");
buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[0].transaction.clone());
assert_eq!(buffer.text(), "1abcdef234");

buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[1].transaction.clone());
assert_eq!(buffer.text(), "1abcdx234");
buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[2].transaction.clone());
assert_eq!(buffer.text(), "1abx234");
buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[1].transaction.clone());
assert_eq!(buffer.text(), "1abyzef234");
buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[2].transaction.clone());
assert_eq!(buffer.text(), "1abcdef234");

buffer.undo_or_redo(entries[2].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[2].transaction.clone());
assert_eq!(buffer.text(), "1abyzef234");
buffer.undo_or_redo(entries[0].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[0].transaction.clone());
assert_eq!(buffer.text(), "1yzef234");
buffer.undo_or_redo(entries[1].transaction.clone()).unwrap();
buffer.undo_or_redo(entries[1].transaction.clone());
assert_eq!(buffer.text(), "1234");
}

Expand Down Expand Up @@ -692,12 +692,12 @@ fn test_concurrent_edits() {
let buf3_op = buffer3.edit([(5..6, "56")]);
assert_eq!(buffer3.text(), "abcde56");

buffer1.apply_op(buf2_op.clone()).unwrap();
buffer1.apply_op(buf3_op.clone()).unwrap();
buffer2.apply_op(buf1_op.clone()).unwrap();
buffer2.apply_op(buf3_op).unwrap();
buffer3.apply_op(buf1_op).unwrap();
buffer3.apply_op(buf2_op).unwrap();
buffer1.apply_op(buf2_op.clone());
buffer1.apply_op(buf3_op.clone());
buffer2.apply_op(buf1_op.clone());
buffer2.apply_op(buf3_op);
buffer3.apply_op(buf1_op);
buffer3.apply_op(buf2_op);

assert_eq!(buffer1.text(), "a12c34e56");
assert_eq!(buffer2.text(), "a12c34e56");
Expand Down Expand Up @@ -756,7 +756,7 @@ fn test_random_concurrent_edits(mut rng: StdRng) {
replica_id,
ops.len()
);
buffer.apply_ops(ops).unwrap();
buffer.apply_ops(ops);
}
}
_ => {}
Expand Down
Loading
Loading