Undo redo fixes and improvements #6950
Conversation
# Conflicts: # app/gui/view/src/project.rs
mwu-tow
requested review from
MichaelMauderer,
wdanilo,
farmaazon and
kazcw
as code owners
| from Standard.Table import all | ||
| from Standard.Database import all | ||
| from Standard.AWS import all | ||
| import Standard.Visualization | ||
|
|
||
| main = | ||
| operator1 = "Press TAB key to create a new node" | ||
|
|
There was a problem hiding this comment.
Can we in such cases maintain the code indentation? We can before passing this to engine just cut the left indentation to the common one, making this file looks nicer. What do you think of this idea?
There was a problem hiding this comment.
There are pros and cons to both approaches. Personally, I prefer to keep the indentation as-is, because it makes it easier to:
- copy-paste code as-is to test it;
- easily update code without reformatting.
As this is used only in tests (which are generally not long and separated from other code), the irregular indention doesn't bother me much.
But this is not a strong opinion and generally matter of taste.
Some compromise might be to the code to separate files, and then use include to load them. This way, the code is still separated, but the indentation is regular. On the other hand, it becomes impossible to understand the code at a glance, because you have to jump between files.
At this time I'd keep it as-is, as this convention is also used in many other tests. If we decide we want to use other style, I'd rather have it a separate PR that changes all tests at once, so that we don't have a mix of styles.
There was a problem hiding this comment.
I'm ok with leaving it this way! Thanks for the explanation!
| debug!("Skipping this snapshot, as module's state was already saved.") | ||
| } | ||
| }) | ||
| with(self.frame.borrow_mut(), |mut data| data.store_module_content(id, content)) |
There was a problem hiding this comment.
you can use self.frame.with_borrowed_mut(|...| ...) instead.
There was a problem hiding this comment.
@wdanilo
As far as I know, there is no such method on RefCell, it seems to be provided only for things defined with thread_local.
There was a problem hiding this comment.
This method is defined in prelude!
app/gui/view/src/project.rs
Outdated
| /// The name of the command used to undo the last action. | ||
| /// | ||
| /// It is used to discern undo command in `current_shortcut` output, allowing other components to | ||
| /// handle it in a special way. | ||
| const UNDO_COMMAND_NAME: &str = "undo"; |
There was a problem hiding this comment.
It is used to discern undo command in
current_shortcutoutput,
I don't see any such usage of this constant.
Co-authored-by: Adam Obuchowicz <adam.obuchowicz@enso.org>
mwu-tow
added
the
CI: No changelog needed
QA Report: 🔴
|
|
@farmaazon really good catches! |
Both are fixed. |
mwu-tow
changed the title
Pull Request Description
This PR adds facilities for controllers to be aware of what shortcut command is currently being processed. This allows grouping consequences of single user action into a single transaction without hard-coding it separately for all the separate paths case-by-case, which turned out to be challenging and error-prone.
Additionally, a number of minor fixes were carried over from #6877:
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.