-
Notifications
You must be signed in to change notification settings - Fork 323
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
Changes from 17 commits
f49457a
877d2a9
5001a5b
527ed4d
348c7a6
cbc3d1a
f00c012
bbb4352
d473bd1
11e293c
40f2bb7
74a692b
bdf8a5a
aab4ec5
6bb1dfd
468d25b
c1ffc83
4faf47f
bad2e6e
4588e57
7ca79ad
b4b2c98
21397f5
ee8fec7
ed4af57
8376deb
a5cafba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 === | ||
// ====================== | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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: Rc<Cell<ExecutionEnvironment>>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it need to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. |
||
} | ||
|
||
impl ExecutionContext { | ||
|
@@ -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, | ||
} | ||
} | ||
|
||
|
@@ -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() | ||
} | ||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -284,6 +285,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:?}"); | ||
} | ||
} | ||
} | ||
|
||
|
||
|
@@ -381,23 +400,26 @@ impl Project { | |
|
||
eval_ view.execution_context_restart(model.execution_context_restart()); | ||
|
||
eval graph_view.execution_environment((mode) model.execution_environment_changed(mode)); | ||
|
||
eval_ view.toggle_read_only(model.toggle_read_only()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double
-