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

Add a shortcut for interrupting the program #3967

Merged
merged 6 commits into from
Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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: 8 additions & 0 deletions app/gui/controller/engine-protocol/src/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ trait API {
fn modify_visualisation
(&self, visualisation_id: Uuid, visualisation_config: VisualisationConfiguration) -> ();

/// Interrupt the program execution.
#[MethodInput=InterruptInput, rpc_name="executionContext/interrupt"]
fn interrupt(&self, context_id: ContextId) -> ();

/// Restart the program execution.
#[MethodInput=RecomputeInput, rpc_name="executionContext/recompute"]
fn recompute(&self, context_id: ContextId) -> ();

/// Obtain the full suggestions database.
#[MethodInput=GetSuggestionsDatabaseInput, rpc_name="search/getSuggestionsDatabase"]
fn get_suggestions_database(&self) -> response::GetSuggestionDatabase;
Expand Down
2 changes: 2 additions & 0 deletions app/gui/docs/product/shortcuts.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ broken and require further investigation.
| <kbd>ctrl</kbd>+<kbd>w</kbd> | Close the application (Windows, Linux) |
| :warning: <kbd>ctrl</kbd>+<kbd>p</kbd> | Toggle profiling mode |
| <kbd>escape</kbd> | Cancel current action. For example, drop currently dragged connection. |
| <kbd>cmd</kbd>+<kbd>shift</kbd>+<kbd>t</kbd> | Terminate the program execution |
| <kbd>cmd</kbd>+<kbd>shift</kbd>+<kbd>r</kbd> | Re-execute the program |

#### Navigation

Expand Down
10 changes: 5 additions & 5 deletions app/gui/src/controller/graph/executed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ pub enum Notification {
#[derive(Clone, CloneRef, Debug)]
pub struct Handle {
#[allow(missing_docs)]
pub logger: Logger,
pub logger: Logger,
/// A handle to basic graph operations.
graph: Rc<RefCell<controller::Graph>>,
graph: Rc<RefCell<controller::Graph>>,
/// Execution Context handle, its call stack top contains `graph`'s definition.
execution_ctx: model::ExecutionContext,
pub execution_ctx: model::ExecutionContext,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field was private for a reason: otherwise, someone could manually push a frame there without changing the graph field, which should always point to the definition on top of the frame.

Please, make it private and add interrupt and restart methods to this handle.

/// The handle to project controller is necessary, as entering nodes might need to switch
/// modules, and only the project can provide their controllers.
project: model::Project,
project: model::Project,
/// The publisher allowing sending notification to subscribed entities. Note that its outputs
/// is merged with publishers from the stored graph and execution controllers.
notifier: crate::notification::Publisher<Notification>,
notifier: crate::notification::Publisher<Notification>,
}

impl Handle {
Expand Down
8 changes: 8 additions & 0 deletions app/gui/src/model/execution_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,14 @@ pub trait API: Debug {
let detach_actions = visualizations.into_iter().map(move |v| self.detach_visualization(v));
futures::future::join_all(detach_actions).boxed_local()
}

/// Interrupt the program execution.
#[allow(clippy::needless_lifetimes)] // Note: Needless lifetimes
fn interrupt<'a>(&'a self) -> BoxFuture<'a, FallibleResult>;

/// Restart the program execution.
#[allow(clippy::needless_lifetimes)] // Note: Needless lifetimes
fn restart<'a>(&'a self) -> BoxFuture<'a, FallibleResult>;
}

// Note: Needless lifetimes
Expand Down
8 changes: 8 additions & 0 deletions app/gui/src/model/execution_context/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,14 @@ impl model::execution_context::API for ExecutionContext {
Err(InvalidVisualizationId(visualization_id).into())
}
}

fn interrupt(&self) -> BoxFuture<FallibleResult> {
futures::future::ready(Ok(())).boxed_local()
}

fn restart(&self) -> BoxFuture<FallibleResult> {
futures::future::ready(Ok(())).boxed_local()
}
}


Expand Down
16 changes: 16 additions & 0 deletions app/gui/src/model/execution_context/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,22 @@ impl model::execution_context::API for ExecutionContext {
debug!("Dispatching visualization update through the context {}", self.id());
self.model.dispatch_visualization_update(visualization_id, data)
}

fn interrupt(&self) -> BoxFuture<FallibleResult> {
async move {
self.language_server.client.interrupt(&self.id).await?;
Ok(())
}
.boxed_local()
}

fn restart(&self) -> BoxFuture<FallibleResult> {
async move {
self.language_server.client.recompute(&self.id).await?;
Ok(())
}
.boxed_local()
Comment on lines +301 to +306
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. is restarting automatically interrupting?
  2. Is interrupting pausing until its restarted? It should.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes and yes

}
}

impl Drop for ExecutionContext {
Expand Down
22 changes: 22 additions & 0 deletions app/gui/src/presenter/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,24 @@ impl Model {
}
})
}

fn execution_context_interrupt(&self) {
let controller = self.graph_controller.clone_ref();
executor::global::spawn(async move {
if let Err(err) = controller.execution_ctx.interrupt().await {
error!("Error interrupting execution context: {err}");
}
})
}

fn execution_context_restart(&self) {
let controller = self.graph_controller.clone_ref();
executor::global::spawn(async move {
if let Err(err) = controller.execution_ctx.restart().await {
error!("Error restarting execution context: {err}");
}
})
}
}


Expand Down Expand Up @@ -246,6 +264,10 @@ impl Project {
view.values_updated <+ values_computed;

eval_ view.save_project_snapshot(model.save_project_snapshot());

eval_ view.execution_context_interrupt(model.execution_context_interrupt());

eval_ view.execution_context_restart(model.execution_context_restart());
}

let graph_controller = self.model.graph_controller.clone_ref();
Expand Down
6 changes: 6 additions & 0 deletions app/gui/view/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ ensogl::define_endpoints! {
disable_debug_mode(),
/// A set of value updates has been processed and rendered.
values_updated(),
/// Interrupt the running program.
execution_context_interrupt(),
/// Restart the program execution.
execution_context_restart(),
}

Output {
Expand Down Expand Up @@ -834,6 +838,8 @@ impl application::View for View {
(Press, "", "cmd y", "redo"),
(Press, "!debug_mode", DEBUG_MODE_SHORTCUT, "enable_debug_mode"),
(Press, "debug_mode", DEBUG_MODE_SHORTCUT, "disable_debug_mode"),
(Press, "", "cmd shift t", "execution_context_interrupt"),
(Press, "", "cmd shift r", "execution_context_restart"),
]
.iter()
.map(|(a, b, c, d)| Self::self_shortcut_when(*a, *c, *d, *b))
Expand Down