Create new node when opening searcher and no node is selected. #3645
Conversation
…Source_when_opening_Seacher_for_a_new_Node_#182633544 # Conflicts: # CHANGELOG.md # app/gui/src/presenter/searcher.rs
MichaelMauderer
requested review from
wdanilo,
farmaazon and
mwu-tow
as code owners
| @@ -416,6 +416,7 @@ pub mod tests { | |||
| // Setup the controller. | |||
| let mut fixture = crate::test::mock::Unified::new().fixture(); | |||
| let Fixture { executed_graph, execution, executor, .. } = &mut fixture; | |||
| executor.run_until_stalled(); | |||
There was a problem hiding this comment.
If it is required at this point, shan't it be called always when constructing the fixture?
app/gui/src/controller/searcher.rs
Outdated
| fn with_new_node( | ||
| parameters: SearcherParams, | ||
| graph: &presenter::Graph, | ||
| graph_editor: &GraphEditor, | ||
| graph_controller: &controller::Graph, | ||
| ) -> FallibleResult<Self> { |
There was a problem hiding this comment.
Here is a bit of responsibilities mixing. So far, controllers does not know anything about views - it was the presenter who know both, set up and call them directly. There was an idea to make controllers, and presenter separate crates (view is already one).
app/gui/src/controller/searcher.rs
Outdated
| let mut new_node = NewNodeInfo::new_pushed_back(DEFAULT_INPUT_EXPRESSION); | ||
| new_node.metadata = Some(metadata); | ||
| new_node.introduce_pattern = false; | ||
| let created_node = graph_controller.add_node(new_node)?; |
There was a problem hiding this comment.
I think creating new node should not be a part of "Mode" constructor, as "Mode" is rather a description according to docs. A data structure, not an object in terms of OOP.
| let mut graph_info = GraphInfo::from_definition(graph_definition.item); | ||
| graph_info.add_node(&node, LocationHint::End)?; | ||
| module.ast = module.ast.set_traversing(&graph_definition.crumbs, graph_info.ast())?; | ||
| let position = | ||
| self.graph.graph().node(self.mode.node_id())?.metadata.and_then(|md| md.position); | ||
| let metadata = NodeMetadata { position, ..default() }; |
There was a problem hiding this comment.
When adding an example, we still create a new node. Are we sure the old one is removed? Maybe we can just update the existing node expression instead of adding a new one?
There was a problem hiding this comment.
By default, the newly created node is cleared unless, we prevent that by calling prevent_revert on the edit guard.
The only case that could result in a weird state is, if we edit a node and choose an example. Then the original node would be reverted to pre-edit and the example is inserted. But I think examples are only available for empty prompts and are not meant to be used with other nodes (at the moment).
app/gui/src/controller/searcher.rs
Outdated
| fn init_input(&self) { | ||
| if let Mode::NewNode { source_node, .. } = self.mode.deref() { | ||
| if source_node.is_none() { | ||
| if let Err(e) = self.set_input("".to_string()) { | ||
| tracing::error!( | ||
| "Failed to clear input when creating searcher for a new node: {:?}", | ||
| e | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why do we need this? Does the view make some input update before being cleared? IMO, whatever reason, it should be handled rather by presenter (to not propagate view input to searcher or AST changes to view).
There was a problem hiding this comment.
The input would otherwise be set to whatever is sued as the default value of a new node. I added a comment and moved this to the presenter.
MichaelMauderer
added
the
CI: Ready to merge
…Source_when_opening_Seacher_for_a_new_Node_#182633544
MichaelMauderer
added
the
CI: Ready to merge
mergify
bot
deleted the
wip/michaelmauderer/Create_a_New_Node_in_Source_when_opening_Seacher_for_a_new_Node_#182633544
branch
Pull Request Description
So far, when opening the searcher with no node selected, an empty input without an associated AST node was created. This input was manipulated by the user and the final expression from the input was used to create a new node when the user confirmed their input. This PR changes this, so that when the searcher is opened without a selected node, a new AST node is created right away with some placeholder content, and this node is updated when the user confirms their input.
The only change visible to the user, is that if the text editor is opened during editing, a new node will appear in the source when the searcher is opened to create a new node. All other behaviour should stay the same.
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide buildand./run ide watch.