-
Notifications
You must be signed in to change notification settings - Fork 16
add api to skip adding commands to undo queue #394
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
base: main
Are you sure you want to change the base?
Conversation
| execute(cmd, false); | ||
| } | ||
|
|
||
| public void execute(AbstractCommand<T> cmd, boolean skipUndo) { |
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.
This seems a good fix, but there are many internal uses of CommandManager::execute, which are only exposed via ActionCmdFactory (i.e. ActionCmdFactory::decorate calls ActionCmdDecorate::apply that calls viewModel.getCommandManager().execute(new DecorateCmd(Objects.requireNonNull(decorations)));).
So this API change would only work for direct usage of execute, which I don't think is happening, given that the CommandManager is typically used only internally by the RTA view model, and not by developers, isn't it?
| */ | ||
| package com.gluonhq.richtextarea.viewmodel; | ||
|
|
||
| public interface EditActionCmd extends ActionCmd { |
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.
You don't need a new interface. ActionCmd could have:
public interface ActionCmd {
void apply(RichTextAreaViewModel viewModel);
default void apply(RichTextAreaViewModel viewModel, boolean skipUndo) {
apply(viewModel);
}
...
And then you could add the new apply where needed (ActionCmdXXX)
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.
DecorateAction and subclasses don't expose this.
BasicAction::execute doesn't expose this.
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.
DecorateAction doesn’t directly use the CommandManager. DecorateAction is extended by TextDecorateAction and ParagraphDecorateAction with both the later using ACTION_CMD_FACTORY.decorate internally. Should skipUndo be added as a constructor parameter to DecorateAction ?
Fixes #364