From 84ee08229a1b268e4eb6176236bece7a876462bf Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Wed, 5 Apr 2023 17:20:03 +0400 Subject: [PATCH 1/9] wip --- app/gui/src/controller/module.rs | 2 +- app/gui/src/model/module.rs | 4 +++- app/gui/src/model/module/plain.rs | 10 ++++++++++ app/gui/src/model/module/synchronized.rs | 4 +++- app/gui/src/model/project.rs | 4 ++++ app/gui/src/model/project/synchronized.rs | 14 +++++++++++++- app/gui/src/model/registry.rs | 6 ++++-- app/gui/src/presenter/project.rs | 8 ++++++++ app/gui/src/test.rs | 8 +++++--- app/gui/view/src/project.rs | 2 ++ 10 files changed, 53 insertions(+), 9 deletions(-) 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/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..01dfa43319ca 100644 --- a/app/gui/src/model/module/plain.rs +++ b/app/gui/src/model/module/plain.rs @@ -25,6 +25,10 @@ use parser::Parser; use std::collections::hash_map::Entry; +#[derive(Debug, Clone, Fail)] +#[fail(display = "Attempt to edit a read-only module.")] +pub struct ReadOnlyError; + // ============== // === Module === @@ -41,6 +45,7 @@ pub struct Module { content: RefCell, notifications: notification::Publisher, repository: Rc, + read_only: Rc>, } impl Module { @@ -50,12 +55,14 @@ 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, } } @@ -70,6 +77,9 @@ impl Module { debug!("Ignoring spurious update."); return Ok(()); } + if self.read_only.get() && kind != NotificationKind::MetadataChanged { + return Err(ReadOnlyError.into()); + } 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()); diff --git a/app/gui/src/model/module/synchronized.rs b/app/gui/src/model/module/synchronized.rs index 9598f4cbc9f9..8743e1338e1f 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,8 @@ 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 model = + model::module::Plain::new(path, source.ast, source.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); diff --git a/app/gui/src/model/project.rs b/app/gui/src/model/project.rs index 2d6fcd699884..43a2414c97a4 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -39,6 +39,10 @@ pub trait API: Debug { /// Project's qualified name fn qualified_name(&self) -> project::QualifiedName; + fn read_only(&self) -> bool; + + fn set_read_only(&self, read_only: bool); + /// Get Language Server JSON-RPC Connection for this project. fn json_rpc(&self) -> Rc; diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index 74408a2d1985..ae25004f09df 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -287,6 +287,7 @@ pub struct Project { pub parser: Parser, pub notifications: notification::Publisher, pub urm: Rc, + pub read_only: Rc>, } impl Project { @@ -331,6 +332,7 @@ impl Project { parser, notifications, urm, + read_only: default(), }; let binary_handler = ret.binary_event_handler(); @@ -616,9 +618,11 @@ impl Project { let parser = self.parser.clone_ref(); let urm = self.urm(); let repository = 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?; + module::Synchronized::open(path, language_server, parser, repository, read_only) + .await?; urm.module_opened(module.clone()); Ok(module) } @@ -717,6 +721,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); + } } diff --git a/app/gui/src/model/registry.rs b/app/gui/src/model/registry.rs index d6c6ae4ab56c..ad2cf120ac33 100644 --- a/app/gui/src/model/registry.rs +++ b/app/gui/src/model/registry.rs @@ -177,7 +177,8 @@ 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(model::module::Plain::new(path.clone(), ast, default(), urm, default())); let registry = Registry::default(); let expected = state.clone_ref(); @@ -198,7 +199,8 @@ 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(model::module::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 d5901b5922bb..4b40e5929dd5 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); + error!("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/test.rs b/app/gui/src/test.rs index 9fee40d904c7..3fc9fd9e3b80 100644 --- a/app/gui/src/test.rs +++ b/app/gui/src/test.rs @@ -184,8 +184,9 @@ 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, default())); + urm.module_opened(module.clone_ref()); module } @@ -384,7 +385,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 module_future = + model::module::Synchronized::open(path, ls, parser, repository, default()); // 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/view/src/project.rs b/app/gui/view/src/project.rs index 587b14daa64b..9d9a4f3815df 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,7 @@ 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"), + (Press, "", "cmd shift b", "toggle_read_only"), ] .iter() .map(|(a, b, c, d)| Self::self_shortcut_when(*a, *c, *d, *b)) From 30a85ce6270df895234706db7ee3cae57698b961 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Tue, 11 Apr 2023 14:08:29 +0400 Subject: [PATCH 2/9] wip --- .../engine-protocol/src/language_server.rs | 2 +- .../src/language_server/tests.rs | 2 +- app/gui/src/controller/graph/executed.rs | 44 +++++++++++++++++-- app/gui/src/model/module/synchronized.rs | 17 ++++--- app/gui/src/model/project.rs | 2 + app/gui/src/model/project/synchronized.rs | 10 ++++- app/gui/tests/language_server.rs | 2 +- 7 files changed, 65 insertions(+), 14 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..2fe99e8ba2e6 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" : { diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 671f4ccc556a..00425c46dccc 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,8 +220,13 @@ 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 { + if self.project.read_only() { + return Err(ReadOnly.into()); + } debug!("Entering node {}.", local_call.call); let method_ptr = &local_call.definition; let graph = controller::Graph::new_method(&self.project, method_ptr); @@ -249,10 +258,16 @@ 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 there's no information about target method pointer (e.g. because node value hasn't + /// ### Errors + /// - Fails if there's no information about target method pointer (e.g. because node value + /// hasn't /// been yet computed by the engine) or if method graph cannot be created (see /// `graph_for_method` documentation). + /// - Fails if the project is in read-only mode. pub async fn enter_node(&self, node: double_representation::node::Id) -> FallibleResult { + if self.project.read_only() { + return Err(ReadOnly.into()); + } let computed_value = self.node_computed_value(node)?; let method_pointer = computed_value.method_call.as_ref().ok_or(NoResolvedMethod(node))?; let definition = method_pointer.clone(); @@ -262,9 +277,14 @@ 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 { + if self.project.read_only() { + return Err(ReadOnly.into()); + } let frame = self.execution_ctx.pop().await?; let method = self.execution_ctx.current_method(); let graph = controller::Graph::new_method(&self.project, &method).await?; @@ -280,7 +300,13 @@ impl Handle { } /// Restart the program execution. + /// + /// ### Errors + /// - Fails if the project is in read-only mode. pub async fn restart(&self) -> FallibleResult { + if self.project.read_only() { + return Err(ReadOnly.into()); + } self.execution_ctx.restart().await?; Ok(()) } @@ -324,14 +350,26 @@ impl Handle { } /// Create connection in graph. + /// + /// ### Errors + /// - Fails if the project is in read-only mode. pub fn connect(&self, connection: &Connection) -> FallibleResult { + if self.project.read_only() { + return Err(ReadOnly.into()); + } 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> { + if self.project.read_only() { + return Err(ReadOnly.into()); + } self.graph.borrow().disconnect(connection, self) } } diff --git a/app/gui/src/model/module/synchronized.rs b/app/gui/src/model/module/synchronized.rs index 8743e1338e1f..02c0ae3d3652 100644 --- a/app/gui/src/model/module/synchronized.rs +++ b/app/gui/src/model/module/synchronized.rs @@ -240,7 +240,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(()) @@ -425,7 +425,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 notify_ls = + self.notify_language_server(&summary.summary, &new_file, edits, true); profiler::await_!(notify_ls, _profiler) } NotificationKind::MetadataChanged => { @@ -433,7 +434,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 notify_ls = + self.notify_language_server(&summary.summary, &new_file, edits, false); profiler::await_!(notify_ls, _profiler) } NotificationKind::Reloaded => Ok(ParsedContentSummary::from_source(&new_file)), @@ -452,7 +454,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( @@ -520,7 +522,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 @@ -531,6 +533,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 { @@ -540,7 +543,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) @@ -636,7 +639,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); diff --git a/app/gui/src/model/project.rs b/app/gui/src/model/project.rs index 43a2414c97a4..4fcff8b1c4df 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -39,8 +39,10 @@ pub trait API: Debug { /// Project's qualified name fn qualified_name(&self) -> project::QualifiedName; + // TODO fn read_only(&self) -> bool; + // TODO fn set_read_only(&self, read_only: bool); /// Get Language Server JSON-RPC Connection for this project. diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index ae25004f09df..eea9f6df15e8 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -216,6 +216,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)] @@ -698,6 +703,9 @@ impl model::project::API for Project { } fn rename_project(&self, name: String) -> BoxFuture { + if self.read_only() { + return std::future::ready(Err(RenameInReadOnly.into())).boxed_local(); + } async move { let referent_name = name.as_str().try_into()?; let project_manager = self.project_manager.as_ref().ok_or(ProjectManagerUnavailable)?; @@ -866,7 +874,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/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."); From 88acbfdd1f1805351c70c649aeb9a164bc6a22da Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Tue, 11 Apr 2023 17:16:39 +0400 Subject: [PATCH 3/9] wip --- .../controller/engine-protocol/src/language_server/tests.rs | 3 ++- app/ide-desktop/lib/client/watch.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) 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 2fe99e8ba2e6..336dfca8348a 100644 --- a/app/gui/controller/engine-protocol/src/language_server/tests.rs +++ b/app/gui/controller/engine-protocol/src/language_server/tests.rs @@ -572,7 +572,8 @@ fn test_execution_context() { ], "oldVersion" : "d3ee9b1ba1990fecfd794d2f30e0207aaa7be5d37d463073096d86f8", "newVersion" : "6a33e22f20f16642697e8bd549ff7b759252ad56c05a1b0acc31dc69" - } + }, + "execute": true }), unit_json.clone(), (), diff --git a/app/ide-desktop/lib/client/watch.ts b/app/ide-desktop/lib/client/watch.ts index fdc61ed181a3..9bf9a563b4a5 100644 --- a/app/ide-desktop/lib/client/watch.ts +++ b/app/ide-desktop/lib/client/watch.ts @@ -121,6 +121,7 @@ process.on('SIGINT', () => { for (;;) { console.log('Spawning Electron process.') + console.log('Args: ' + ELECTRON_ARGS) const electronProcess = childProcess.spawn('electron', ELECTRON_ARGS, { stdio: 'inherit', shell: true, From 393c112a714016f59d42b921b5172e9edb659b02 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Wed, 12 Apr 2023 16:29:25 +0400 Subject: [PATCH 4/9] wip --- app/gui/src/model/module/plain.rs | 19 ++++++++++++++----- app/gui/src/model/project.rs | 7 +++++-- app/gui/src/presenter/project.rs | 2 +- app/gui/view/src/project.rs | 3 ++- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/gui/src/model/module/plain.rs b/app/gui/src/model/module/plain.rs index 01dfa43319ca..67cfcab61a27 100644 --- a/app/gui/src/model/module/plain.rs +++ b/app/gui/src/model/module/plain.rs @@ -25,9 +25,16 @@ use parser::Parser; use std::collections::hash_map::Entry; -#[derive(Debug, Clone, Fail)] -#[fail(display = "Attempt to edit a read-only module.")] -pub struct ReadOnlyError; + +// ============== +// === Errors === +// ============== + +#[allow(missing_docs)] +#[derive(Debug, Clone, Copy, Fail)] +#[fail(display = "Attempt to edit a read-only module")] +pub struct EditInReadOnly; + // ============== @@ -68,9 +75,11 @@ impl Module { /// 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() { @@ -78,7 +87,7 @@ impl Module { return Ok(()); } if self.read_only.get() && kind != NotificationKind::MetadataChanged { - return Err(ReadOnlyError.into()); + return Err(EditInReadOnly.into()); } trace!("Updating module's content: {kind:?}. New content:\n{new_content}"); let transaction = self.repository.transaction("Setting module's content"); diff --git a/app/gui/src/model/project.rs b/app/gui/src/model/project.rs index 4fcff8b1c4df..c4150ac81f3b 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -39,10 +39,13 @@ pub trait API: Debug { /// Project's qualified name fn qualified_name(&self) -> project::QualifiedName; - // TODO + /// 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; - // TODO + /// 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. diff --git a/app/gui/src/presenter/project.rs b/app/gui/src/presenter/project.rs index 4b40e5929dd5..9d697f19587f 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -202,7 +202,7 @@ impl Model { fn toggle_read_only(&self) { let read_only = self.controller.model.read_only(); self.controller.model.set_read_only(!read_only); - error!("New read only state: {}", self.controller.model.read_only()); + info!("New read only state: {}.", self.controller.model.read_only()); } fn restore_project_snapshot(&self) { diff --git a/app/gui/view/src/project.rs b/app/gui/view/src/project.rs index 9d9a4f3815df..d637c3a919ef 100644 --- a/app/gui/view/src/project.rs +++ b/app/gui/view/src/project.rs @@ -690,7 +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"), - (Press, "", "cmd shift b", "toggle_read_only"), + // TODO(#6179): Remove this temporary shortcut when Play button is ready. + (Press, "", "ctrl alt b", "toggle_read_only"), ] .iter() .map(|(a, b, c, d)| Self::self_shortcut_when(*a, *c, *d, *b)) From 76e0fc1efda67154af176ec252b2a39c7cc47957 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 13 Apr 2023 04:38:02 +0400 Subject: [PATCH 5/9] wip --- app/gui/src/controller/graph/executed.rs | 93 ++++++++++++++++++++++++ app/gui/src/controller/searcher.rs | 5 ++ app/gui/src/model/module/synchronized.rs | 20 ++++- app/gui/src/model/project.rs | 4 + app/gui/src/presenter/project.rs | 2 +- app/gui/src/presenter/searcher.rs | 2 +- app/gui/src/test.rs | 18 ++++- app/gui/view/src/project.rs | 2 +- 8 files changed, 138 insertions(+), 8 deletions(-) diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 00425c46dccc..950dd8adc7b4 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -422,6 +422,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; @@ -508,4 +510,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/searcher.rs b/app/gui/src/controller/searcher.rs index 51bef4934cff..9c3f055b5cfc 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -96,6 +96,11 @@ pub struct CannotCommitExpression { mode: Mode, } +#[allow(missing_docs)] +#[derive(Debug, Clone, Copy, Fail)] +#[fail(display = "Attempt to preview a suggestion in read-only mode")] +pub struct SuggestionPreviewInReadOnly; + // ===================== // === Notifications === diff --git a/app/gui/src/model/module/synchronized.rs b/app/gui/src/model/module/synchronized.rs index 02c0ae3d3652..f165ef1f5075 100644 --- a/app/gui/src/model/module/synchronized.rs +++ b/app/gui/src/model/module/synchronized.rs @@ -760,9 +760,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| { @@ -789,19 +791,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 c4150ac81f3b..5e383bfc16d0 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -276,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/presenter/project.rs b/app/gui/src/presenter/project.rs index 9d697f19587f..cbcd23387efb 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -202,7 +202,7 @@ impl Model { 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()); + error!("New read only state: {}.", self.controller.model.read_only()); } fn restore_project_snapshot(&self) { 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 3fc9fd9e3b80..e5b421bdc890 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,13 @@ 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, default())); + let module = Rc::new(model::module::Plain::new( + path, + ast, + metadata, + repository, + self.read_only.clone_ref(), + )); urm.module_opened(module.clone_ref()); module } @@ -243,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); @@ -302,6 +310,7 @@ pub mod mock { position_in_code, ) .unwrap(); + let read_only = self.read_only.clone_ref(); executor.run_until_stalled(); Fixture { executor, @@ -314,6 +323,7 @@ pub mod mock { project, searcher, ide, + read_only, } } @@ -361,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] @@ -385,8 +396,9 @@ 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 read_only = self.read_only.clone_ref(); let module_future = - model::module::Synchronized::open(path, ls, parser, repository, default()); + model::module::Synchronized::open(path, ls, parser, repository, read_only); // 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/view/src/project.rs b/app/gui/view/src/project.rs index d637c3a919ef..753ee17ecc59 100644 --- a/app/gui/view/src/project.rs +++ b/app/gui/view/src/project.rs @@ -691,7 +691,7 @@ impl application::View for View { (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 alt b", "toggle_read_only"), + (Press, "", "ctrl shift b", "toggle_read_only"), ] .iter() .map(|(a, b, c, d)| Self::self_shortcut_when(*a, *c, *d, *b)) From f406abb8a59565f5485069c3f25ffca31523fd85 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 13 Apr 2023 04:41:46 +0400 Subject: [PATCH 6/9] wip --- app/gui/src/controller/searcher.rs | 4 ---- app/ide-desktop/lib/client/watch.ts | 1 - 2 files changed, 5 deletions(-) diff --git a/app/gui/src/controller/searcher.rs b/app/gui/src/controller/searcher.rs index 9c3f055b5cfc..54ca7285a3be 100644 --- a/app/gui/src/controller/searcher.rs +++ b/app/gui/src/controller/searcher.rs @@ -96,10 +96,6 @@ pub struct CannotCommitExpression { mode: Mode, } -#[allow(missing_docs)] -#[derive(Debug, Clone, Copy, Fail)] -#[fail(display = "Attempt to preview a suggestion in read-only mode")] -pub struct SuggestionPreviewInReadOnly; // ===================== diff --git a/app/ide-desktop/lib/client/watch.ts b/app/ide-desktop/lib/client/watch.ts index 52b057ab30b6..81b89bd5d66d 100644 --- a/app/ide-desktop/lib/client/watch.ts +++ b/app/ide-desktop/lib/client/watch.ts @@ -121,7 +121,6 @@ process.on('SIGINT', () => { for (;;) { console.log('Spawning Electron process.') - console.log('Args: ' + ELECTRON_ARGS) const electronProcess = childProcess.spawn('electron', ELECTRON_ARGS, { stdio: 'inherit', shell: true, From c76654863741280d7b25d4ec0b98bb5a474d5000 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 13 Apr 2023 04:46:55 +0400 Subject: [PATCH 7/9] lints --- app/gui/src/model/project.rs | 2 +- app/gui/src/presenter/project.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/gui/src/model/project.rs b/app/gui/src/model/project.rs index 5e383bfc16d0..74eb3a032521 100644 --- a/app/gui/src/model/project.rs +++ b/app/gui/src/model/project.rs @@ -279,5 +279,5 @@ pub mod test { 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/presenter/project.rs b/app/gui/src/presenter/project.rs index cbcd23387efb..9d697f19587f 100644 --- a/app/gui/src/presenter/project.rs +++ b/app/gui/src/presenter/project.rs @@ -202,7 +202,7 @@ impl Model { fn toggle_read_only(&self) { let read_only = self.controller.model.read_only(); self.controller.model.set_read_only(!read_only); - error!("New read only state: {}.", self.controller.model.read_only()); + info!("New read only state: {}.", self.controller.model.read_only()); } fn restore_project_snapshot(&self) { From 78db7ebd72db013b8ccd8f411f59c482837cc73e Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 13 Apr 2023 14:48:49 +0400 Subject: [PATCH 8/9] address review comments and fix formatting --- app/gui/src/controller/graph/executed.rs | 64 +++++++++++++---------- app/gui/src/model/module/plain.rs | 23 ++++---- app/gui/src/model/module/synchronized.rs | 13 ++--- app/gui/src/model/project/synchronized.rs | 32 ++++++------ app/gui/src/model/registry.rs | 9 ++-- app/gui/src/test.rs | 5 +- 6 files changed, 76 insertions(+), 70 deletions(-) diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index 950dd8adc7b4..a81435e846da 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -225,19 +225,20 @@ impl Handle { /// - Fails if the project is in read-only mode. pub async fn enter_method_pointer(&self, local_call: &LocalCall) -> FallibleResult { if self.project.read_only() { - return Err(ReadOnly.into()); + 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(()) } - 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. @@ -266,13 +267,15 @@ impl Handle { /// - Fails if the project is in read-only mode. pub async fn enter_node(&self, node: double_representation::node::Id) -> FallibleResult { if self.project.read_only() { - return Err(ReadOnly.into()); + Err(ReadOnly.into()) + } else { + let computed_value = self.node_computed_value(node)?; + let method_pointer = + computed_value.method_call.as_ref().ok_or(NoResolvedMethod(node))?; + let definition = method_pointer.clone(); + let local_call = LocalCall { call: node, definition }; + self.enter_method_pointer(&local_call).await } - let computed_value = self.node_computed_value(node)?; - let method_pointer = computed_value.method_call.as_ref().ok_or(NoResolvedMethod(node))?; - let definition = method_pointer.clone(); - let local_call = LocalCall { call: node, definition }; - self.enter_method_pointer(&local_call).await } /// Leave the current node. Reverse of `enter_node`. @@ -283,14 +286,15 @@ impl Handle { /// - Fails if the project is in read-only mode. pub async fn exit_node(&self) -> FallibleResult { if self.project.read_only() { - return Err(ReadOnly.into()); + 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(()) } - 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. @@ -355,9 +359,10 @@ impl Handle { /// - Fails if the project is in read-only mode. pub fn connect(&self, connection: &Connection) -> FallibleResult { if self.project.read_only() { - return Err(ReadOnly.into()); + Err(ReadOnly.into()) + } else { + self.graph.borrow().connect(connection, self) } - self.graph.borrow().connect(connection, self) } /// Remove the connections from the graph. Returns an updated edge destination endpoint for @@ -368,9 +373,10 @@ impl Handle { /// - Fails if the project is in read-only mode. pub fn disconnect(&self, connection: &Connection) -> FallibleResult> { if self.project.read_only() { - return Err(ReadOnly.into()); + Err(ReadOnly.into()) + } else { + self.graph.borrow().disconnect(connection, self) } - self.graph.borrow().disconnect(connection, self) } } diff --git a/app/gui/src/model/module/plain.rs b/app/gui/src/model/module/plain.rs index 67cfcab61a27..3869f7b640b6 100644 --- a/app/gui/src/model/module/plain.rs +++ b/app/gui/src/model/module/plain.rs @@ -87,18 +87,19 @@ impl Module { return Ok(()); } if self.read_only.get() && kind != NotificationKind::MetadataChanged { - return Err(EditInReadOnly.into()); + 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(()) } - 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 f165ef1f5075..502663277b5a 100644 --- a/app/gui/src/model/module/synchronized.rs +++ b/app/gui/src/model/module/synchronized.rs @@ -177,8 +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, read_only); + 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); @@ -425,8 +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, true); + let summary = &summary.summary; + let notify_ls = self.notify_language_server(summary, &new_file, edits, true); profiler::await_!(notify_ls, _profiler) } NotificationKind::MetadataChanged => { @@ -434,8 +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, false); + 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)), diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index eea9f6df15e8..ce7077782d9f 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -619,15 +619,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, read_only) - .await?; + let module = module::Synchronized::open(path, ls, parser, repo, read_only).await?; urm.module_opened(module.clone()); Ok(module) } @@ -704,18 +702,20 @@ impl model::project::API for Project { fn rename_project(&self, name: String) -> BoxFuture { if self.read_only() { - return std::future::ready(Err(RenameInReadOnly.into())).boxed_local(); - } - async move { - let referent_name = name.as_str().try_into()?; - 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; - Ok(()) + std::future::ready(Err(RenameInReadOnly.into())).boxed_local() + } else { + async move { + let referent_name = name.as_str().try_into()?; + 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; + Ok(()) + } + .boxed_local() } - .boxed_local() } fn project_content_root_id(&self) -> Uuid { diff --git a/app/gui/src/model/registry.rs b/app/gui/src/model/registry.rs index ad2cf120ac33..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,8 +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, default())); + let state = Rc::new(Plain::new(path.clone(), ast, default(), urm, default())); let registry = Registry::default(); let expected = state.clone_ref(); @@ -199,8 +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, default())); + 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/test.rs b/app/gui/src/test.rs index e5b421bdc890..3d0e287af907 100644 --- a/app/gui/src/test.rs +++ b/app/gui/src/test.rs @@ -396,9 +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 read_only = self.read_only.clone_ref(); - let module_future = - model::module::Synchronized::open(path, ls, parser, repository, read_only); + 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(); From 5efd78c4b68358c9368e8d1d6a00c550718a52da Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 13 Apr 2023 18:11:37 +0400 Subject: [PATCH 9/9] fix --- app/gui/src/controller/graph/executed.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/gui/src/controller/graph/executed.rs b/app/gui/src/controller/graph/executed.rs index a81435e846da..ab4e5e845ab7 100644 --- a/app/gui/src/controller/graph/executed.rs +++ b/app/gui/src/controller/graph/executed.rs @@ -309,10 +309,11 @@ impl Handle { /// - Fails if the project is in read-only mode. pub async fn restart(&self) -> FallibleResult { if self.project.read_only() { - return Err(ReadOnly.into()); + Err(ReadOnly.into()) + } else { + self.execution_ctx.restart().await?; + Ok(()) } - self.execution_ctx.restart().await?; - Ok(()) } /// Get the current call stack frames.