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

Ability to change the execution environment between design and live. #6341

Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f49457a
Immplement shortcut handling.
MichaelMauderer Apr 4, 2023
877d2a9
Implement controller handling.
MichaelMauderer Apr 6, 2023
5001a5b
./run fmt, ./run lint
MichaelMauderer Apr 6, 2023
527ed4d
Fix clickable area size.
MichaelMauderer Apr 13, 2023
348c7a6
Rebase to develop and merge changes.
MichaelMauderer Apr 18, 2023
cbc3d1a
Integrate with new existing functionality.
MichaelMauderer Apr 18, 2023
f00c012
Refactoring & code style.
MichaelMauderer Apr 18, 2023
bbb4352
Fix naming of parameters.
MichaelMauderer Apr 18, 2023
d473bd1
Revert unintended changes.
MichaelMauderer Apr 18, 2023
11e293c
Changelog.
MichaelMauderer Apr 18, 2023
40f2bb7
Review feedback.
MichaelMauderer Apr 21, 2023
74a692b
Removed another instance of "mode"
MichaelMauderer Apr 21, 2023
bdf8a5a
Unify dummy data use.
MichaelMauderer Apr 21, 2023
aab4ec5
Merge branch 'develop' into wip/MichaelMauderer/Ability_to_change_the…
MichaelMauderer Apr 21, 2023
6bb1dfd
Fix merge issues.
MichaelMauderer Apr 21, 2023
468d25b
Remove whitespace.
MichaelMauderer Apr 21, 2023
c1ffc83
Restore debug functionality.
MichaelMauderer Apr 21, 2023
4faf47f
Merge branch 'develop' into wip/MichaelMauderer/Ability_to_change_the…
MichaelMauderer Apr 24, 2023
bad2e6e
Resolve merge conflicts.
MichaelMauderer Apr 24, 2023
4588e57
Review feedback.
MichaelMauderer Apr 24, 2023
7ca79ad
QA feedback.
MichaelMauderer Apr 25, 2023
b4b2c98
Merge branch 'develop' into wip/MichaelMauderer/Ability_to_change_the…
MichaelMauderer Apr 25, 2023
21397f5
Revert debug logging.
MichaelMauderer Apr 25, 2023
ee8fec7
Fix shortcuts. Revert read only refactoring.
MichaelMauderer Apr 25, 2023
ed4af57
Remove TRACE_ALL
vitvakatu Apr 25, 2023
8376deb
Merge branch 'develop' into wip/MichaelMauderer/Ability_to_change_the…
vitvakatu Apr 25, 2023
a5cafba
Fix compilation and formatting
vitvakatu Apr 25, 2023
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@
quickly understand each button's function.
- [File associations are created on Windows and macOS][6077]. This allows
opening Enso files by double-clicking them in the file explorer.
- [The IDE UI element for selecting the execution mode of the project is now
sending messages to the backend.][6341].

#### EnsoGL (rendering engine)

Expand Down Expand Up @@ -194,6 +196,7 @@
[5895]: https://github.com/enso-org/enso/pull/6130
[6035]: https://github.com/enso-org/enso/pull/6035
[6097]: https://github.com/enso-org/enso/pull/6097
[6097]: https://github.com/enso-org/enso/pull/6341

#### Enso Standard Library

Expand Down
14 changes: 7 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion app/gui/controller/engine-protocol/src/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ trait API {

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

/// Obtain the full suggestions database.
#[MethodInput=GetSuggestionsDatabaseInput, rpc_name="search/getSuggestionsDatabase"]
Expand Down Expand Up @@ -205,6 +205,11 @@ trait API {
/// VCS snapshot if no `commit_id` is provided.
#[MethodInput=VcsRestoreInput, rpc_name="vcs/restore"]
fn restore_vcs(&self, root: Path, commit_id: Option<String>) -> response::RestoreVcs;

/// Set the execution environment of the context for future evaluations.
#[MethodInput=SetModeInput, rpc_name="executionContext/setExecutionEnvironment"]
fn set_execution_environment(&self, context_id: ContextId, execution_environment: ExecutionEnvironment) -> ();

}}


Expand Down
66 changes: 66 additions & 0 deletions app/gui/controller/engine-protocol/src/language_server/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,72 @@ pub struct LibraryComponentGroup {
}



// =============================
// === Execution Environment ===
// =============================

/// The execution environment which controls the global execution of functions with side effects.
///
/// For more information, see
/// https://github.com/enso-org/design/blob/main/epics/basic-libraries/write-action-control/design.md.
#[derive(Hash, Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display)]
pub enum ExecutionEnvironment {
/// Allows editing the graph, but the `Output` context is disabled, so it prevents accidental
/// changes.
Design,
/// Unrestricted, live editing of data.
Live,
Comment on lines +1167 to +1173
Copy link
Member

Choose a reason for hiding this comment

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

  1. I believe we have the strings "design" and "live" hardcoded somewhere in the previous PR.
  2. I don't think that hardcoding that as enum is a good option, as these 2 options are temporary and we will allow users defining custom modes. So we should keep the current mode in string-like struct instead.
  3. This will also make all of helper functions simpler, such as list_all_imstrings.
  4. Function list_all is similar to a function in previous PR, which was already listing all available execution environments, we should unify these codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here they need to be hard-codes as enum, as this is the current LS protocol. The UI elements can handle arbitrary labels. So, would suggest this to be refactored appropriately once custom modes are implemented.

}

impl Default for ExecutionEnvironment {
fn default() -> Self {
ExecutionEnvironment::Design
}
}

impl ExecutionEnvironment {
/// List all available execution environments.
pub fn list_all() -> Vec<Self> {
vec![ExecutionEnvironment::Design, ExecutionEnvironment::Live]
}

/// List all available execution environments as ImStrings. Useful for UI.
pub fn list_all_as_imstrings() -> Vec<ImString> {
Self::list_all().iter().map(|env| (*env).into()).collect()
}
}

impl From<ExecutionEnvironment> for ImString {
fn from(env: ExecutionEnvironment) -> Self {
ImString::new(env.to_string())
}
}

impl TryFrom<&str> for ExecutionEnvironment {
type Error = ();

fn try_from(value: &str) -> core::result::Result<Self, Self::Error> {
match value.to_lowercase().as_str() {
"design" => Ok(ExecutionEnvironment::Design),
"live" => Ok(ExecutionEnvironment::Live),
_ => Err(()),
}
}
}

impl ExecutionEnvironment {
/// Returns whether the output context is enabled for this execution environment.
pub fn output_context_enabled(&self) -> bool {
match self {
Self::Design => false,
Self::Live => true,
}
}
}



// ======================
// === Test Utilities ===
// ======================
Expand Down
10 changes: 10 additions & 0 deletions app/gui/src/controller/graph/executed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::model::execution_context::VisualizationId;
use crate::model::execution_context::VisualizationUpdateData;

use double_representation::name::QualifiedName;
use engine_protocol::language_server::ExecutionEnvironment;
use engine_protocol::language_server::MethodPointer;
use span_tree::generate::context::CalledMethodInfo;
use span_tree::generate::context::Context;
Expand Down Expand Up @@ -355,6 +356,15 @@ impl Handle {
self.graph.borrow().disconnect(connection, self)
}
}

/// Set the execution environment.
pub async fn set_execution_environment(
&self,
execution_environment: ExecutionEnvironment,
) -> FallibleResult {
self.execution_ctx.set_execution_environment(execution_environment).await?;
Ok(())
}
}


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 @@ -6,6 +6,7 @@ use double_representation::identifier::Identifier;
use double_representation::name::project;
use double_representation::name::QualifiedName;
use engine_protocol::language_server;
use engine_protocol::language_server::ExecutionEnvironment;
use engine_protocol::language_server::ExpressionUpdate;
use engine_protocol::language_server::ExpressionUpdatePayload;
use engine_protocol::language_server::MethodPointer;
Expand Down Expand Up @@ -503,6 +504,13 @@ pub trait API: Debug {

/// Adjust method pointers after the project rename action.
fn rename_method_pointers(&self, old_project_name: String, new_project_name: String);

/// Set the execution environment of the context.
#[allow(clippy::needless_lifetimes)]
fn set_execution_environment<'a>(
&'a self,
execution_environment: ExecutionEnvironment,
) -> BoxFuture<'a, FallibleResult>;
}

// Note: Needless lifetimes
Expand Down
14 changes: 14 additions & 0 deletions app/gui/src/model/execution_context/plain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::model::execution_context::Visualization;
use crate::model::execution_context::VisualizationId;
use crate::model::execution_context::VisualizationUpdateData;

use engine_protocol::language_server::ExecutionEnvironment;
use engine_protocol::language_server::MethodPointer;
use engine_protocol::language_server::VisualisationConfiguration;
use futures::future::LocalBoxFuture;
Expand Down Expand Up @@ -61,6 +62,8 @@ pub struct ExecutionContext {
pub is_ready: crate::sync::Synchronized<bool>,
/// Component groups defined in libraries imported into the execution context.
pub component_groups: RefCell<Rc<Vec<ComponentGroup>>>,
/// Execution environment of the context.
pub execution_environment: Cell<ExecutionEnvironment>,
}

impl ExecutionContext {
Expand All @@ -72,13 +75,15 @@ impl ExecutionContext {
let computed_value_info_registry = default();
let is_ready = default();
let component_groups = default();
let execution_environment = default();
Self {
entry_point,
stack,
visualizations,
computed_value_info_registry,
is_ready,
component_groups,
execution_environment,
}
}

Expand Down Expand Up @@ -273,6 +278,15 @@ impl model::execution_context::API for ExecutionContext {
local_call.definition = update_method_pointer(&mut local_call.definition)
});
}

fn set_execution_environment(
&self,
environment: ExecutionEnvironment,
) -> BoxFuture<FallibleResult> {
info!("Setting execution environment to {environment:?}.");
self.execution_environment.set(environment);
futures::future::ready(Ok(())).boxed_local()
}
}


Expand Down
23 changes: 22 additions & 1 deletion app/gui/src/model/execution_context/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::model::execution_context::VisualizationId;
use crate::model::execution_context::VisualizationUpdateData;

use engine_protocol::language_server;
use engine_protocol::language_server::ExecutionEnvironment;



Expand Down Expand Up @@ -298,7 +299,11 @@ impl model::execution_context::API for ExecutionContext {
async move {
self.language_server
.client
.recompute(&self.id, &language_server::InvalidatedExpressions::All)
.recompute(
&self.id,
&language_server::InvalidatedExpressions::All,
&Some(self.model.execution_environment.get()),
)
.await?;
Ok(())
}
Expand All @@ -308,6 +313,22 @@ impl model::execution_context::API for ExecutionContext {
fn rename_method_pointers(&self, old_project_name: String, new_project_name: String) {
self.model.rename_method_pointers(old_project_name, new_project_name);
}

fn set_execution_environment(
&self,
execution_environment: ExecutionEnvironment,
) -> BoxFuture<FallibleResult> {
self.model.execution_environment.set(execution_environment);
async move {
info!("Setting execution environment to {execution_environment:?}.");
self.language_server
.client
.set_execution_environment(&self.id, &execution_environment)
.await?;
Ok(())
}
.boxed_local()
}
}

impl Drop for ExecutionContext {
Expand Down
2 changes: 1 addition & 1 deletion app/gui/src/presenter/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ use crate::presenter::graph::state::State;
use double_representation::context_switch::Context;
use double_representation::context_switch::ContextSwitch;
use double_representation::context_switch::ContextSwitchExpression;
use engine_protocol::language_server::ExecutionEnvironment;
use engine_protocol::language_server::SuggestionId;
use enso_frp as frp;
use futures::future::LocalBoxFuture;
use ide_view as view;
use ide_view::graph_editor::component::node as node_view;
use ide_view::graph_editor::component::visualization as visualization_view;
use ide_view::graph_editor::EdgeEndpoint;
use view::graph_editor::ExecutionEnvironment;
use view::graph_editor::WidgetUpdates;


Expand Down
35 changes: 29 additions & 6 deletions app/gui/src/presenter/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::executor::global::spawn_stream_handler;
use crate::presenter;
use crate::presenter::graph::ViewNodeId;

use engine_protocol::language_server::ExecutionEnvironment;
use enso_frp as frp;
use ensogl::system::js;
use ide_view as view;
Expand Down Expand Up @@ -287,6 +288,24 @@ impl Model {
view.show_graph_editor();
})
}

fn execution_environment_changed(
&self,
execution_environment: &ide_view::execution_environment_selector::ExecutionEnvironment,
) {
if let Ok(execution_environment) = execution_environment.as_str().try_into() {
let graph_controller = self.graph_controller.clone_ref();
executor::global::spawn(async move {
if let Err(err) =
graph_controller.set_execution_environment(execution_environment).await
{
error!("Error setting execution environment: {err}");
}
});
} else {
error!("Invalid execution environment: {execution_environment:?}");
}
}
}


Expand Down Expand Up @@ -385,22 +404,26 @@ impl Project {
eval_ view.execution_context_restart(model.execution_context_restart());

view.set_read_only <+ view.toggle_read_only.map(f_!(model.toggle_read_only()));
eval graph_view.execution_environment((mode) model.execution_environment_changed(mode));

eval_ view.toggle_read_only(model.toggle_read_only());
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this line is needed? We already have

view.set_read_only <+ view.toggle_read_only.map(f_!(model.toggle_read_only()));

slightly above. I see that read-only mode is broken in a weird way (after toggling it twice removing nodes works, but dragging edges doesn't)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a merge error and the line is removed now.

}

let graph_controller = self.model.graph_controller.clone_ref();

self.init_analytics()
.init_execution_modes()
.init_execution_environments()
.setup_notification_handler()
.attach_frp_to_values_computed_notifications(graph_controller, values_computed)
}

/// Initialises execution modes. Currently a dummy implementqation to be replaced during
/// implementation of #5930.
fn init_execution_modes(self) -> Self {
/// Initialises execution environment.
fn init_execution_environments(self) -> Self {
let graph = &self.model.view.graph();
let entries = Rc::new(vec!["design".to_string(), "live".to_string()]);
graph.set_available_execution_modes(entries);
let entries = Rc::new(ExecutionEnvironment::list_all_as_imstrings());
graph.set_available_execution_environments(entries);
let default_mode = ExecutionEnvironment::default();
graph.set_execution_environment(default_mode);
self
}

Expand Down
Loading