Skip to content

Implement command framework#816

Merged
josuave merged 14 commits intomasterfrom
command-framework
Jul 17, 2024
Merged

Implement command framework#816
josuave merged 14 commits intomasterfrom
command-framework

Conversation

@dremin
Copy link
Copy Markdown
Collaborator

@dremin dremin commented Jun 14, 2024

Introduces an ICairoCommand interface. Cairo and extensions can register commands in the same way shell extensions are registered. Commands consist of an action and metadata that describes the command--its identifier, label, description, parameters, and availability. Commands can can dynamically change their availability, label, etc based on changes to settings or other state.

Introduces a CommandService, similar to ExtensionService, that manages the list of and lifecycle of commands. It initializes all commands and disposes of them.

The CommandService is also what is responsible for invoking commands. Commands are invoked using their unique identifier, and can accept a key/value pair list of parameters.

I've created several commands so far, mostly representing the items in the Cairo menu and a few others. Before I mark this PR as ready, I plan to continue refactoring to replace existing code with commands then removing the old implementations.

As an example usage of commands, I've converted the Cairo menu to be dynamically generated by a list of commands. The Places menu has also been re-implemented around commands. The goal here is to make the menus fully customizable in the future.

There are interfaces, such as ICairoShellFolderCommandInfo and ICairoShellItemCommandInfo, which allow commands to declare their availability and label within shell context menus. Those commands are dynamically shown in menus.

Ultimately once the commands are all implemented, we will be in a place to deconstruct the rest of the Cairo Desktop project (i.e. desktop, taskbar, stacks, etc) now that the code is de-tangled.

Fixes #144

@dremin dremin added Core Features core to Cairo Refactor labels Jun 14, 2024
@dremin dremin requested a review from josuave June 14, 2024 05:49
@josuave josuave marked this pull request as ready for review June 14, 2024 18:48
@josuave josuave marked this pull request as draft June 14, 2024 18:52
Added shell-related commands
Created new interfaces for commands to declare ShellFolder/ShellItem support
Removed old SupportingClasses commands implementations
@dremin
Copy link
Copy Markdown
Collaborator Author

dremin commented Jun 24, 2024

In the latest push, I've added interfaces which commands can implement to indicate their support for displaying in shell context menus. All of the old implementations have been removed and replaced with this new dynamic system.

The builds will fail until cairoshell/ManagedShell#109 merges.

@dremin
Copy link
Copy Markdown
Collaborator Author

dremin commented Jun 25, 2024

@josuave One thing I am on the fence regarding is the Start() method in commands. I originally added this to provide an "out" in case there is a circular dependency when using injection, so a command could get a service from within the Start method instead of via injection. However, I haven't run into this within practical usage, and am beginning to think it is unnecessary. Do you agree?

@josuave
Copy link
Copy Markdown
Collaborator

josuave commented Jun 25, 2024

@dremin i know what your saying. I would not implement anything yet unless needed to do the job right now. Keep it simple and to the point. Handling what if scenarios will usually back you into a corner and get your hands tied.

@dremin
Copy link
Copy Markdown
Collaborator Author

dremin commented Jun 26, 2024

@josuave Thanks! I went ahead and removed it.

I think this is ready now.

@dremin dremin marked this pull request as ready for review June 26, 2024 01:07
@dremin
Copy link
Copy Markdown
Collaborator Author

dremin commented Jul 14, 2024

@josuave Not sure if you saw but this is ready for review :)

public struct CommandInvokedEventArgs
{
public ICairoCommandInfo CommandInfo;
public (string name, object value)[] Parameters;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Due to this being a public part of the api or sdk it may be worth having a real type here

public string Name;
public string Description;
public bool IsRequired;
public object Value;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not a fan of the boxing, but I don't have another solution at the moment


namespace CairoDesktop.Application.Interfaces
{
public interface ICairoCommand : IDisposable
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if we need to inherit IDisposable here considering that not all commands will need it. I would propose that we ignorantly test the command in the command service to check if is.


foreach (var command in _commands)
{
command.Dispose();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if(command is IDisposable disposable)
{
    disposable.dispose();
}

@josuave josuave merged commit 3d04441 into master Jul 17, 2024
@josuave josuave deleted the command-framework branch July 17, 2024 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core Features core to Cairo Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Impliment Command Framework

2 participants