From 6aba602a3414b8f530ab602db04761ee1104fb45 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 20 Apr 2023 17:17:18 +0300 Subject: [PATCH] Read-only mode for controllers (#6259) Closes #6181 - Added a read-only flag for the project model (now controlled by a temporary shortcut). - Renaming the project, editing the code, connecting and disconnecting nodes, and collapsing, and navigating through collapsed nodes are forbidden and will result in the error message in the console. - Some of the above actions produce undesired effects on the IDE, which will be fixed later. This PR is focused on restricting actual AST modifications. - Moving nodes (and updating metadata in general) no longer causes reevaluation https://user-images.githubusercontent.com/6566674/231616408-4f334bb7-1985-43ba-9953-4c0998338a9b.mp4 --- .../engine-protocol/src/language_server.rs | 2 +- .../src/language_server/tests.rs | 5 +- app/gui/src/controller/graph/executed.rs | 176 +++++++++++++++--- app/gui/src/controller/module.rs | 2 +- app/gui/src/controller/searcher.rs | 1 + app/gui/src/model/module.rs | 4 +- app/gui/src/model/module/plain.rs | 42 +++-- app/gui/src/model/module/synchronized.rs | 42 ++++- app/gui/src/model/project.rs | 13 ++ app/gui/src/model/project/synchronized.rs | 52 ++++-- app/gui/src/model/registry.rs | 7 +- app/gui/src/presenter/project.rs | 8 + app/gui/src/presenter/searcher.rs | 2 +- app/gui/src/test.rs | 19 +- app/gui/tests/language_server.rs | 2 +- app/gui/view/src/project.rs | 3 + 16 files changed, 307 insertions(+), 73 deletions(-) diff --git a/app/gui/controller/engine-protocol/src/language_server.rs b/app/gui/controller/engine-protocol/src/language_server.rs index 574933da6da6..5132b45cb46d 100644 --- a/app/gui/controller/engine-protocol/src/language_server.rs +++ b/app/gui/controller/engine-protocol/src/language_server.rs @@ -113,7 +113,7 @@ trait API { /// have permission to edit the resources for which edits are sent. This failure may be partial, /// in that some edits are applied and others are not. #[MethodInput=ApplyTextFileEditInput, rpc_name="text/applyEdit"] - fn apply_text_file_edit(&self, edit: FileEdit) -> (); + fn apply_text_file_edit(&self, edit: FileEdit, execute: bool) -> (); /// Create a new execution context. Return capabilities executionContext/canModify and /// executionContext/receivesUpdates containing freshly created ContextId diff --git a/app/gui/controller/engine-protocol/src/language_server/tests.rs b/app/gui/controller/engine-protocol/src/language_server/tests.rs index 94d216eddff5..336dfca8348a 100644 --- a/app/gui/controller/engine-protocol/src/language_server/tests.rs +++ b/app/gui/controller/engine-protocol/src/language_server/tests.rs @@ -547,7 +547,7 @@ fn test_execution_context() { let path = main.clone(); let edit = FileEdit { path, edits, old_version, new_version }; test_request( - |client| client.apply_text_file_edit(&edit), + |client| client.apply_text_file_edit(&edit, &true), "text/applyEdit", json!({ "edit" : { @@ -572,7 +572,8 @@ fn test_execution_context() { ], "oldVersion" : "d3ee9b1ba1990fecfd794d2f30e0207aaa7be5d37d463073096d86f8", "newVersion" : "6a33e22f20f16642697e8bd549ff7b759252ad56c05a1b0acc31dc69" - } + }, + "execute": true }), unit_json.clone(), (), diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 3635a8128760..96f286d77d38 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -44,6 +44,10 @@ pub struct NotEvaluatedYet(double_representation::node::Id); #[fail(display = "The node {} does not resolve to a method call.", _0)] pub struct NoResolvedMethod(double_representation::node::Id); +#[allow(missing_docs)] +#[derive(Debug, Fail, Clone, Copy)] +#[fail(display = "Operation is not permitted in read only mode")] +pub struct ReadOnly; // ==================== @@ -216,19 +220,25 @@ impl Handle { /// This will cause pushing a new stack frame to the execution context and changing the graph /// controller to point to a new definition. /// - /// Fails if method graph cannot be created (see `graph_for_method` documentation). + /// ### Errors + /// - Fails if method graph cannot be created (see `graph_for_method` documentation). + /// - Fails if the project is in read-only mode. pub async fn enter_method_pointer(&self, local_call: &LocalCall) -> FallibleResult { - debug!("Entering node {}.", local_call.call); - let method_ptr = &local_call.definition; - let graph = controller::Graph::new_method(&self.project, method_ptr); - let graph = graph.await?; - self.execution_ctx.push(local_call.clone()).await?; - debug!("Replacing graph with {graph:?}."); - self.graph.replace(graph); - debug!("Sending graph invalidation signal."); - self.notifier.publish(Notification::EnteredNode(local_call.clone())).await; - - Ok(()) + if self.project.read_only() { + Err(ReadOnly.into()) + } else { + debug!("Entering node {}.", local_call.call); + let method_ptr = &local_call.definition; + let graph = controller::Graph::new_method(&self.project, method_ptr); + let graph = graph.await?; + self.execution_ctx.push(local_call.clone()).await?; + debug!("Replacing graph with {graph:?}."); + self.graph.replace(graph); + debug!("Sending graph invalidation signal."); + self.notifier.publish(Notification::EnteredNode(local_call.clone())).await; + + Ok(()) + } } /// Attempts to get the computed value of the specified node. @@ -246,15 +256,21 @@ impl Handle { /// Leave the current node. Reverse of `enter_node`. /// - /// Fails if this execution context is already at the stack's root or if the parent graph + /// ### Errors + /// - Fails if this execution context is already at the stack's root or if the parent graph /// cannot be retrieved. + /// - Fails if the project is in read-only mode. pub async fn exit_node(&self) -> FallibleResult { - let frame = self.execution_ctx.pop().await?; - let method = self.execution_ctx.current_method(); - let graph = controller::Graph::new_method(&self.project, &method).await?; - self.graph.replace(graph); - self.notifier.publish(Notification::SteppedOutOfNode(frame.call)).await; - Ok(()) + if self.project.read_only() { + Err(ReadOnly.into()) + } else { + let frame = self.execution_ctx.pop().await?; + let method = self.execution_ctx.current_method(); + let graph = controller::Graph::new_method(&self.project, &method).await?; + self.graph.replace(graph); + self.notifier.publish(Notification::SteppedOutOfNode(frame.call)).await; + Ok(()) + } } /// Interrupt the program execution. @@ -264,9 +280,16 @@ impl Handle { } /// Restart the program execution. + /// + /// ### Errors + /// - Fails if the project is in read-only mode. pub async fn restart(&self) -> FallibleResult { - self.execution_ctx.restart().await?; - Ok(()) + if self.project.read_only() { + Err(ReadOnly.into()) + } else { + self.execution_ctx.restart().await?; + Ok(()) + } } /// Get the current call stack frames. @@ -308,15 +331,29 @@ impl Handle { } /// Create connection in graph. + /// + /// ### Errors + /// - Fails if the project is in read-only mode. pub fn connect(&self, connection: &Connection) -> FallibleResult { - self.graph.borrow().connect(connection, self) + if self.project.read_only() { + Err(ReadOnly.into()) + } else { + self.graph.borrow().connect(connection, self) + } } /// Remove the connections from the graph. Returns an updated edge destination endpoint for /// disconnected edge, in case it is still used as destination-only edge. When `None` is /// returned, no update is necessary. + /// + /// ### Errors + /// - Fails if the project is in read-only mode. pub fn disconnect(&self, connection: &Connection) -> FallibleResult> { - self.graph.borrow().disconnect(connection, self) + if self.project.read_only() { + Err(ReadOnly.into()) + } else { + self.graph.borrow().disconnect(connection, self) + } } } @@ -368,6 +405,8 @@ pub mod tests { use crate::model::execution_context::ExpressionId; use crate::test; + use crate::test::mock::Fixture; + use controller::graph::SpanTree; use engine_protocol::language_server::types::test::value_update_with_type; use wasm_bindgen_test::wasm_bindgen_test; use wasm_bindgen_test::wasm_bindgen_test_configure; @@ -454,4 +493,95 @@ pub mod tests { notifications.expect_pending(); } + + // Test that moving nodes is possible in read-only mode. + #[wasm_bindgen_test] + fn read_only_mode_does_not_restrict_moving_nodes() { + use model::module::Position; + + let fixture = crate::test::mock::Unified::new().fixture(); + let Fixture { executed_graph, graph, .. } = fixture; + + let nodes = executed_graph.graph().nodes().unwrap(); + let node = &nodes[0]; + + let pos1 = Position::new(500.0, 250.0); + let pos2 = Position::new(300.0, 150.0); + + graph.set_node_position(node.id(), pos1).unwrap(); + assert_eq!(graph.node(node.id()).unwrap().position(), Some(pos1)); + graph.set_node_position(node.id(), pos2).unwrap(); + assert_eq!(graph.node(node.id()).unwrap().position(), Some(pos2)); + } + + // Test that certain actions are forbidden in read-only mode. + #[wasm_bindgen_test] + fn read_only_mode() { + fn run(code: &str, f: impl FnOnce(&Handle)) { + let mut data = crate::test::mock::Unified::new(); + data.set_code(code); + let fixture = data.fixture(); + fixture.read_only.set(true); + let Fixture { executed_graph, .. } = fixture; + f(&executed_graph); + } + + + // === Editing the node. === + + let default_code = r#" +main = + foo = 2 * 2 +"#; + run(default_code, |executed| { + let nodes = executed.graph().nodes().unwrap(); + let node = &nodes[0]; + assert!(executed.graph().set_expression(node.info.id(), "5 * 20").is_err()); + }); + + + // === Collapsing nodes. === + + let code = r#" +main = + foo = 2 + bar = foo + 6 + baz = 2 + foo + bar + caz = baz / 2 * baz +"#; + run(code, |executed| { + let nodes = executed.graph().nodes().unwrap(); + // Collapse two middle nodes. + let nodes_range = vec![nodes[1].id(), nodes[2].id()]; + assert!(executed.graph().collapse(nodes_range, "extracted").is_err()); + }); + + + // === Connecting nodes. === + + let code = r#" +main = + 2 + 2 + 5 * 5 +"#; + run(code, |executed| { + let nodes = executed.graph().nodes().unwrap(); + let sum_node = &nodes[0]; + let product_node = &nodes[1]; + + assert_eq!(sum_node.expression().to_string(), "2 + 2"); + assert_eq!(product_node.expression().to_string(), "5 * 5"); + + let context = &span_tree::generate::context::Empty; + let sum_tree = SpanTree::<()>::new(&sum_node.expression(), context).unwrap(); + let sum_input = + sum_tree.root_ref().leaf_iter().find(|n| n.is_argument()).unwrap().crumbs; + let connection = Connection { + source: controller::graph::Endpoint::new(product_node.id(), []), + destination: controller::graph::Endpoint::new(sum_node.id(), sum_input), + }; + + assert!(executed.connect(&connection).is_err()); + }); + } } diff --git a/app/gui/src/controller/module.rs b/app/gui/src/controller/module.rs index 987e5afa33b9..8cfc7da81608 100644 --- a/app/gui/src/controller/module.rs +++ b/app/gui/src/controller/module.rs @@ -173,7 +173,7 @@ impl Handle { ) -> FallibleResult { let ast = parser.parse(code.to_string(), id_map).try_into()?; let metadata = default(); - let model = Rc::new(model::module::Plain::new(path, ast, metadata, repository)); + let model = Rc::new(model::module::Plain::new(path, ast, metadata, repository, default())); Ok(Handle { model, language_server, parser }) } diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 51bef4934cff..54ca7285a3be 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -97,6 +97,7 @@ pub struct CannotCommitExpression { } + // ===================== // === Notifications === // ===================== diff --git a/app/gui/src/model/module.rs b/app/gui/src/model/module.rs index 144b55be03be..86d51c70243d 100644 --- a/app/gui/src/model/module.rs +++ b/app/gui/src/model/module.rs @@ -744,7 +744,9 @@ pub mod test { repository: Rc, ) -> Module { let ast = parser.parse_module(&self.code, self.id_map.clone()).unwrap(); - let module = Plain::new(self.path.clone(), ast, self.metadata.clone(), repository); + let path = self.path.clone(); + let metadata = self.metadata.clone(); + let module = Plain::new(path, ast, metadata, repository, default()); Rc::new(module) } } diff --git a/app/gui/src/model/module/plain.rs b/app/gui/src/model/module/plain.rs index 4bc74485bb2e..3869f7b640b6 100644 --- a/app/gui/src/model/module/plain.rs +++ b/app/gui/src/model/module/plain.rs @@ -26,6 +26,17 @@ use std::collections::hash_map::Entry; +// ============== +// === Errors === +// ============== + +#[allow(missing_docs)] +#[derive(Debug, Clone, Copy, Fail)] +#[fail(display = "Attempt to edit a read-only module")] +pub struct EditInReadOnly; + + + // ============== // === Module === // ============== @@ -41,6 +52,7 @@ pub struct Module { content: RefCell, notifications: notification::Publisher, repository: Rc, + read_only: Rc>, } impl Module { @@ -50,36 +62,44 @@ impl Module { ast: ast::known::Module, metadata: Metadata, repository: Rc, + read_only: Rc>, ) -> Self { Module { content: RefCell::new(ParsedSourceFile { ast, metadata }), notifications: default(), path, repository, + read_only, } } /// Replace the module's content with the new value and emit notification of given kind. /// - /// Fails if the `new_content` is so broken that it cannot be serialized to text. In such case + /// ### Errors + /// - Fails if the `new_content` is so broken that it cannot be serialized to text. In such case /// the module's state is guaranteed to remain unmodified and the notification will not be /// emitted. + /// - Fails if the module is read-only. Metadata-only changes are allowed in read-only mode. #[profile(Debug)] fn set_content(&self, new_content: Content, kind: NotificationKind) -> FallibleResult { if new_content == *self.content.borrow() { debug!("Ignoring spurious update."); return Ok(()); } - trace!("Updating module's content: {kind:?}. New content:\n{new_content}"); - let transaction = self.repository.transaction("Setting module's content"); - transaction.fill_content(self.id(), self.content.borrow().clone()); - - // We want the line below to fail before changing state. - let new_file = new_content.serialize()?; - let notification = Notification::new(new_file, kind); - self.content.replace(new_content); - self.notifications.notify(notification); - Ok(()) + if self.read_only.get() && kind != NotificationKind::MetadataChanged { + Err(EditInReadOnly.into()) + } else { + trace!("Updating module's content: {kind:?}. New content:\n{new_content}"); + let transaction = self.repository.transaction("Setting module's content"); + transaction.fill_content(self.id(), self.content.borrow().clone()); + + // We want the line below to fail before changing state. + let new_file = new_content.serialize()?; + let notification = Notification::new(new_file, kind); + self.content.replace(new_content); + self.notifications.notify(notification); + Ok(()) + } } /// Use `f` to update the module's content. diff --git a/app/gui/src/model/module/synchronized.rs b/app/gui/src/model/module/synchronized.rs index 9598f4cbc9f9..502663277b5a 100644 --- a/app/gui/src/model/module/synchronized.rs +++ b/app/gui/src/model/module/synchronized.rs @@ -165,6 +165,7 @@ impl Module { language_server: Rc, parser: Parser, repository: Rc, + read_only: Rc>, ) -> FallibleResult> { let file_path = path.file_path().clone(); info!("Opening module {file_path}"); @@ -176,7 +177,9 @@ impl Module { let source = parser.parse_with_metadata(opened.content); let digest = opened.current_version; let summary = ContentSummary { digest, end_of_file }; - let model = model::module::Plain::new(path, source.ast, source.metadata, repository); + let metadata = source.metadata; + let ast = source.ast; + let model = model::module::Plain::new(path, ast, metadata, repository, read_only); let this = Rc::new(Module { model, language_server }); let content = this.model.serialized_content()?; let first_invalidation = this.full_invalidation(&summary, content); @@ -238,7 +241,7 @@ impl Module { self.content().replace(parsed_source); let summary = ContentSummary::new(&content); let change = TextEdit::from_prefix_postfix_differences(&content, &source.content); - self.notify_language_server(&summary, &source, vec![change]).await?; + self.notify_language_server(&summary, &source, vec![change], true).await?; let notification = Notification::new(source, NotificationKind::Reloaded); self.notify(notification); Ok(()) @@ -423,7 +426,8 @@ impl Module { }; //id_map goes first, because code change may alter its position. let edits = vec![id_map_change, code_change]; - let notify_ls = self.notify_language_server(&summary.summary, &new_file, edits); + let summary = &summary.summary; + let notify_ls = self.notify_language_server(summary, &new_file, edits, true); profiler::await_!(notify_ls, _profiler) } NotificationKind::MetadataChanged => { @@ -431,7 +435,8 @@ impl Module { range: summary.metadata_engine_range().into(), text: new_file.metadata_slice().to_string(), }]; - let notify_ls = self.notify_language_server(&summary.summary, &new_file, edits); + let summary = &summary.summary; + let notify_ls = self.notify_language_server(summary, &new_file, edits, false); profiler::await_!(notify_ls, _profiler) } NotificationKind::Reloaded => Ok(ParsedContentSummary::from_source(&new_file)), @@ -450,7 +455,7 @@ impl Module { debug!("Handling full invalidation: {ls_content:?}."); let range = Range::new(Location::default(), ls_content.end_of_file); let edits = vec![TextEdit { range: range.into(), text: new_file.content.clone() }]; - self.notify_language_server(ls_content, &new_file, edits) + self.notify_language_server(ls_content, &new_file, edits, true) } fn edit_for_snipped( @@ -518,7 +523,7 @@ impl Module { .into_iter() .flatten() .collect_vec(); - self.notify_language_server(&ls_content.summary, &new_file, edits) + self.notify_language_server(&ls_content.summary, &new_file, edits, true) } /// This is a helper function with all common logic regarding sending the update to @@ -529,6 +534,7 @@ impl Module { ls_content: &ContentSummary, new_file: &SourceFile, edits: Vec, + execute: bool, ) -> impl Future> + 'static { let summary = ParsedContentSummary::from_source(new_file); let edit = FileEdit { @@ -538,7 +544,7 @@ impl Module { new_version: Sha3_224::new(new_file.content.as_bytes()), }; debug!("Notifying LS with edit: {edit:#?}."); - let ls_future_reply = self.language_server.client.apply_text_file_edit(&edit); + let ls_future_reply = self.language_server.client.apply_text_file_edit(&edit, &execute); async move { ls_future_reply.await?; Ok(summary) @@ -634,7 +640,7 @@ pub mod test { f: impl FnOnce(&FileEdit) -> json_rpc::Result<()> + 'static, ) { let this = self.clone(); - client.expect.apply_text_file_edit(move |edits| { + client.expect.apply_text_file_edit(move |edits, _execute| { let content_so_far = this.current_ls_content.get(); let result = f(edits); let new_content = apply_edits(content_so_far, edits); @@ -755,9 +761,11 @@ pub mod test { // * there is an initial invalidation after opening the module // * replacing AST causes invalidation // * localized text edit emits similarly localized synchronization updates. + // * modifying the code fails if the read-only mode is enabled. let initial_code = "main =\n println \"Hello World!\""; let mut data = crate::test::mock::Unified::new(); data.set_code(initial_code); + let read_only: Rc> = default(); // We do actually care about sharing `data` between `test` invocations, as it stores the // Parser which is time-consuming to construct. let test = |runner: &mut Runner| { @@ -784,19 +792,33 @@ pub mod test { Ok(()) }); }); + fixture.read_only.set(read_only.get()); let parser = data.parser.clone(); let module = fixture.synchronized_module(); let new_content = "main =\n println \"Test\""; let new_ast = parser.parse_module(new_content, default()).unwrap(); - module.update_ast(new_ast).unwrap(); + let res = module.update_ast(new_ast); + if read_only.get() { + assert!(res.is_err()); + } else { + assert!(res.is_ok()); + } runner.perhaps_run_until_stalled(&mut fixture); let change = TextChange { range: (20..24).into(), text: "Test 2".to_string() }; - module.apply_code_change(change, &Parser::new(), default()).unwrap(); + let res = module.apply_code_change(change, &Parser::new(), default()); + if read_only.get() { + assert!(res.is_err()); + } else { + assert!(res.is_ok()); + } runner.perhaps_run_until_stalled(&mut fixture); }; + read_only.set(false); + Runner::run(test); + read_only.set(true); Runner::run(test); } diff --git a/app/gui/src/model/project.rs b/app/gui/src/model/project.rs index 2d6fcd699884..74eb3a032521 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -39,6 +39,15 @@ pub trait API: Debug { /// Project's qualified name fn qualified_name(&self) -> project::QualifiedName; + /// Whether the read-only mode is enabled for the project. + /// + /// Read-only mode forbids certain operations, like renaming the project or editing the code + /// through the IDE. + fn read_only(&self) -> bool; + + /// Set the read-only mode for the project. + fn set_read_only(&self, read_only: bool); + /// Get Language Server JSON-RPC Connection for this project. fn json_rpc(&self) -> Rc; @@ -267,4 +276,8 @@ pub mod test { path.qualified_module_name(name.clone()) }); } + + pub fn expect_read_only(project: &mut MockAPI, read_only: Rc>) { + project.expect_read_only().returning_st(move || read_only.get()); + } } diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index 714df7e69aa2..bf6435d54a38 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -224,6 +224,11 @@ async fn update_modules_on_file_change( #[fail(display = "Project Manager is unavailable.")] pub struct ProjectManagerUnavailable; +#[allow(missing_docs)] +#[derive(Clone, Copy, Debug, Fail)] +#[fail(display = "Project renaming is not available in read-only mode.")] +pub struct RenameInReadOnly; + /// A wrapper for an error with information that user tried to open project with unsupported /// engine's version (which is likely the cause of the problems). #[derive(Debug, Fail)] @@ -295,6 +300,7 @@ pub struct Project { pub parser: Parser, pub notifications: notification::Publisher, pub urm: Rc, + pub read_only: Rc>, } impl Project { @@ -339,6 +345,7 @@ impl Project { parser, notifications, urm, + read_only: default(), }; let binary_handler = ret.binary_event_handler(); @@ -620,13 +627,13 @@ impl Project { &self, path: module::Path, ) -> impl Future>> { - let language_server = self.language_server_rpc.clone_ref(); + let ls = self.language_server_rpc.clone_ref(); let parser = self.parser.clone_ref(); let urm = self.urm(); - let repository = urm.repository.clone_ref(); + let repo = urm.repository.clone_ref(); + let read_only = self.read_only.clone_ref(); async move { - let module = - module::Synchronized::open(path, language_server, parser, repository).await?; + let module = module::Synchronized::open(path, ls, parser, repo, read_only).await?; urm.module_opened(module.clone()); Ok(module) } @@ -702,18 +709,23 @@ impl model::project::API for Project { } fn rename_project(&self, name: String) -> BoxFuture { - async move { - let old_name = self.properties.borrow_mut().name.project.clone_ref(); - let referent_name = name.to_im_string(); - let project_manager = self.project_manager.as_ref().ok_or(ProjectManagerUnavailable)?; - let project_id = self.properties.borrow().id; - let project_name = ProjectName::new_unchecked(name); - project_manager.rename_project(&project_id, &project_name).await?; - self.properties.borrow_mut().name.project = referent_name.clone_ref(); - self.execution_contexts.rename_project(old_name, referent_name); - Ok(()) + if self.read_only() { + std::future::ready(Err(RenameInReadOnly.into())).boxed_local() + } else { + async move { + let old_name = self.properties.borrow_mut().name.project.clone_ref(); + let referent_name = name.to_im_string(); + let project_manager = + self.project_manager.as_ref().ok_or(ProjectManagerUnavailable)?; + let project_id = self.properties.borrow().id; + let project_name = ProjectName::new_unchecked(name); + project_manager.rename_project(&project_id, &project_name).await?; + self.properties.borrow_mut().name.project = referent_name.clone_ref(); + self.execution_contexts.rename_project(old_name, referent_name); + Ok(()) + } + .boxed_local() } - .boxed_local() } fn project_content_root_id(&self) -> Uuid { @@ -727,6 +739,14 @@ impl model::project::API for Project { fn urm(&self) -> Rc { self.urm.clone_ref() } + + fn read_only(&self) -> bool { + self.read_only.get() + } + + fn set_read_only(&self, read_only: bool) { + self.read_only.set(read_only); + } } @@ -864,7 +884,7 @@ mod test { let write_capability = Some(write_capability); let open_response = response::OpenTextFile { content, current_version, write_capability }; expect_call!(client.open_text_file(path=path.clone()) => Ok(open_response)); - client.expect.apply_text_file_edit(|_| Ok(())); + client.expect.apply_text_file_edit(|_, _| Ok(())); expect_call!(client.close_text_file(path) => Ok(())); } diff --git a/app/gui/src/model/registry.rs b/app/gui/src/model/registry.rs index d6c6ae4ab56c..f71a886d90ed 100644 --- a/app/gui/src/model/registry.rs +++ b/app/gui/src/model/registry.rs @@ -165,9 +165,10 @@ mod test { use super::*; use crate::executor::test_utils::TestWithLocalPoolExecutor; + use model::module::Plain; type ModulePath = model::module::Path; - type Registry = super::Registry; + type Registry = super::Registry; #[test] fn getting_module() { @@ -177,7 +178,7 @@ mod test { let ast = ast::Ast::one_line_module(line).try_into().unwrap(); let path = ModulePath::from_mock_module_name("Test"); let urm = default(); - let state = Rc::new(model::module::Plain::new(path.clone(), ast, default(), urm)); + let state = Rc::new(Plain::new(path.clone(), ast, default(), urm, default())); let registry = Registry::default(); let expected = state.clone_ref(); @@ -198,7 +199,7 @@ mod test { let path1 = ModulePath::from_mock_module_name("Test"); let path2 = path1.clone(); let urm = default(); - let state1 = Rc::new(model::module::Plain::new(path1.clone_ref(), ast, default(), urm)); + let state1 = Rc::new(Plain::new(path1.clone_ref(), ast, default(), urm, default())); let state2 = state1.clone_ref(); let registry1 = Rc::new(Registry::default()); let registry2 = registry1.clone_ref(); diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index dfb5569b68bf..e45ad2dd4397 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -199,6 +199,12 @@ impl Model { self.ide_controller.set_component_browser_private_entries_visibility(!visibility); } + fn toggle_read_only(&self) { + let read_only = self.controller.model.read_only(); + self.controller.model.set_read_only(!read_only); + info!("New read only state: {}.", self.controller.model.read_only()); + } + fn restore_project_snapshot(&self) { let controller = self.controller.clone_ref(); let breadcrumbs = self.view.graph().model.breadcrumbs.clone_ref(); @@ -374,6 +380,8 @@ impl Project { eval_ view.execution_context_interrupt(model.execution_context_interrupt()); eval_ view.execution_context_restart(model.execution_context_restart()); + + eval_ view.toggle_read_only(model.toggle_read_only()); } let graph_controller = self.model.graph_controller.clone_ref(); diff --git a/app/gui/src/presenter/searcher.rs b/app/gui/src/presenter/searcher.rs index 1cf9a2834215..c7b3fb2b0206 100644 --- a/app/gui/src/presenter/searcher.rs +++ b/app/gui/src/presenter/searcher.rs @@ -134,7 +134,7 @@ impl Model { match self.suggestion_for_entry_id(entry_id) { Ok(suggestion) => if let Err(error) = self.controller.preview_suggestion(suggestion) { - warn!("Failed to preview suggestion {entry_id:?} because of error: {error:?}."); + warn!("Failed to preview suggestion {entry_id:?} because of error: {error}."); }, Err(err) => warn!("Error while previewing suggestion: {err}."), } diff --git a/app/gui/src/test.rs b/app/gui/src/test.rs index 9fee40d904c7..3d0e287af907 100644 --- a/app/gui/src/test.rs +++ b/app/gui/src/test.rs @@ -135,6 +135,7 @@ pub mod mock { pub suggestions: HashMap, pub context_id: model::execution_context::Id, pub parser: parser::Parser, + pub read_only: Rc>, code: String, id_map: ast::IdMap, metadata: crate::model::module::Metadata, @@ -172,6 +173,7 @@ pub mod mock { context_id: CONTEXT_ID, root_definition: definition_name(), parser: parser::Parser::new(), + read_only: default(), } } @@ -184,8 +186,14 @@ pub mod mock { let path = self.module_path.clone(); let metadata = self.metadata.clone(); let repository = urm.repository.clone_ref(); - let module = Rc::new(model::module::Plain::new(path, ast, metadata, repository)); - urm.module_opened(module.clone()); + let module = Rc::new(model::module::Plain::new( + path, + ast, + metadata, + repository, + self.read_only.clone_ref(), + )); + urm.module_opened(module.clone_ref()); module } @@ -242,6 +250,7 @@ pub mod mock { // Root ID is needed to generate module path used to get the module. model::project::test::expect_root_id(&mut project, crate::test::mock::data::ROOT_ID); model::project::test::expect_suggestion_db(&mut project, suggestion_database); + model::project::test::expect_read_only(&mut project, self.read_only.clone_ref()); let json_rpc = language_server::Connection::new_mock_rc(json_client); model::project::test::expect_json_rpc(&mut project, json_rpc); let binary_rpc = binary::Connection::new_mock_rc(binary_client); @@ -301,6 +310,7 @@ pub mod mock { position_in_code, ) .unwrap(); + let read_only = self.read_only.clone_ref(); executor.run_until_stalled(); Fixture { executor, @@ -313,6 +323,7 @@ pub mod mock { project, searcher, ide, + read_only, } } @@ -360,6 +371,7 @@ pub mod mock { pub executed_graph: controller::ExecutedGraph, pub suggestion_db: Rc, pub project: model::Project, + pub read_only: Rc>, pub ide: controller::Ide, pub searcher: controller::Searcher, #[deref] @@ -384,7 +396,8 @@ pub mod mock { let path = self.data.module_path.clone(); let ls = self.project.json_rpc(); let repository = self.project.urm().repository.clone_ref(); - let module_future = model::module::Synchronized::open(path, ls, parser, repository); + let ro = self.read_only.clone_ref(); + let module_future = model::module::Synchronized::open(path, ls, parser, repository, ro); // We can `expect_ready`, because in fact this is synchronous in test conditions. // (there's no real asynchronous connection beneath, just the `MockClient`) let module = module_future.boxed_local().expect_ready().unwrap(); diff --git a/app/gui/tests/language_server.rs b/app/gui/tests/language_server.rs index 9321b72ddb56..a19e31e7ae40 100644 --- a/app/gui/tests/language_server.rs +++ b/app/gui/tests/language_server.rs @@ -240,7 +240,7 @@ async fn ls_text_protocol_test() { let new_version = Sha3_224::new(b"Hello, world!"); let path = move_path.clone(); let edit = FileEdit { path, edits, old_version, new_version: new_version.clone() }; - client.apply_text_file_edit(&edit).await.expect("Couldn't apply edit."); + client.apply_text_file_edit(&edit, &true).await.expect("Couldn't apply edit."); let saving_result = client.save_text_file(&move_path, &new_version).await; saving_result.expect("Couldn't save file."); diff --git a/app/gui/view/src/project.rs b/app/gui/view/src/project.rs index 587b14daa64b..753ee17ecc59 100644 --- a/app/gui/view/src/project.rs +++ b/app/gui/view/src/project.rs @@ -103,6 +103,7 @@ ensogl::define_endpoints! { execution_context_interrupt(), /// Restart the program execution. execution_context_restart(), + toggle_read_only(), } Output { @@ -689,6 +690,8 @@ impl application::View for View { (Press, "debug_mode", DEBUG_MODE_SHORTCUT, "disable_debug_mode"), (Press, "", "cmd shift t", "execution_context_interrupt"), (Press, "", "cmd shift r", "execution_context_restart"), + // TODO(#6179): Remove this temporary shortcut when Play button is ready. + (Press, "", "ctrl shift b", "toggle_read_only"), ] .iter() .map(|(a, b, c, d)| Self::self_shortcut_when(*a, *c, *d, *b))