Skip to content

0.7.2#275

Open
Bistard wants to merge 23 commits intomasterfrom
dev
Open

0.7.2#275
Bistard wants to merge 23 commits intomasterfrom
dev

Conversation

@Bistard
Copy link
Owner

@Bistard Bistard commented Feb 26, 2025

Pull Request Template

Environment Changes

  • ...

Breaking Changes

  • ...

Features

  • ...

Fixes

  • ...

Checklist

  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Summary by Sourcery

Refactor the command registration process to improve flexibility and add list manipulation commands. The changes involve using shortcutOptions for command shortcuts and introducing new commands for splitting, sinking, and lifting list items.

New Features:

  • Introduce list-related commands such as splitListItem, sinkListItem, and liftListItem to enhance list manipulation within the editor.

Enhancements:

  • Refactor command registration to use shortcutOptions for defining shortcuts, weights, and conditions, instead of passing shortcuts as separate arguments.
  • Remove the disposable return type from the shortcut registration methods.

@Bistard Bistard self-assigned this Feb 26, 2025
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Feb 26, 2025

Reviewer's Guide by Sourcery

This pull request refactors the editor commands registration to support shortcut options, improves command execution, fixes a typo in editorView.ts, and renames joinForward to JoinForward in editorCommands.ts. The changes involve modifications to several files, including editorCommands.ts, command.ts, shortcutRegistrant.ts, shortcutService.ts, editorView.ts, and others. The new implementation uses shortcut options to define shortcuts, weights, and conditions for commands. The command execution is also improved by prioritizing commands and allowing clients to indicate if a shortcut is handled.

Sequence diagram for command execution with shortcut

sequenceDiagram
    participant KeyboardService
    participant ShortcutService
    participant ShortcutRegistrant
    participant CommandService
    participant Command

    KeyboardService->>ShortcutService: onKeydown(event)
    ShortcutService->>ShortcutRegistrant: findShortcut(pressed)
    ShortcutRegistrant-->>ShortcutService: candidates
    loop For each candidate
      ShortcutService->>ContextService: contextMatchExpr(candidate.when)
      alt context matches
        ShortcutService->>CommandService: executeCommand(candidate.commandID, ...args)
        CommandService->>Command: run(...args)
        Command-->>CommandService: return true/false
        alt ret === true
          CommandService-->>ShortcutService: break
        end
      end
    end
Loading

Updated class diagram for EditorCommandExtension

classDiagram
  class EditorCommandExtension {
    - _commandSet: Set<string>
    - _commandKeybinding: Map<number, string>
    + registerCommand(command: Command): void
    + registerBasicEditorCommands(extension: IEditorCommandExtension, logService: ILogService, getArguments: () => EditorCommandArguments): void
  }
  class Command {
    + id: string
  }
  EditorCommandExtension -- Command : uses
  note for EditorCommandExtension "Deprecated, FIX"
Loading

Updated class diagram for ShortcutRegistrant

classDiagram
    class ShortcutRegistrant {
        - _shortcuts: Map<number, ShortcutRegistration[]>
        + register2<ID extends string>(commandID: ID, registration: IShortcutRegistration<ID>): void
    }

    class ShortcutRegistration {
        + commandArgs: any[] | IO<any[]>
        + when: ContextKeyExpression
        + weight: ShortcutWeight
        + shortcut: Shortcut | Shortcut[]
    }

    ShortcutRegistrant -- ShortcutRegistration : uses
    note for ShortcutRegistration "commandArgs can be a function (IO<any[]>)"
Loading

File-Level Changes

Change Details Files
Refactor editor commands registration to support shortcut options and improve command execution.
  • Modified registerBasicEditorCommands to accept a getArguments function for command arguments.
  • Updated command registration to include shortcutOptions for defining shortcuts, weights, and conditions.
  • Replaced direct shortcut registration with the new shortcutOptions in __registerToggleMarkCommands, __registerHeadingCommands, and __registerBasicCommands.
  • Removed the shortcuts parameter from the registerCommand method in EditorCommandExtension.
  • Modified ShortcutRegistrant to support multiple shortcuts per command and to handle command arguments more flexibly.
  • Updated ShortcutService to execute commands based on priority and to allow clients to indicate if a shortcut is handled.
  • Introduced buildEditorCommand to streamline command creation.
  • Added EditorCommandBase as a base class for editor commands.
  • Added EditorCommandArguments type alias for editor command arguments.
  • Removed disposable returns from shortcut registration.
  • Added commandList.contrib.ts to register list-related commands.
  • Added editorCommand.ts to define EditorCommandBase and buildEditorCommand.
  • Updated editorContextKeys.ts to include isEditorEditable context key.
  • Updated proseMirror.ts to export ProseReplaceAroundStep.
src/editor/contrib/command/editorCommands.ts
src/editor/contrib/command/command.ts
src/workbench/services/shortcut/shortcutRegistrant.ts
src/workbench/services/shortcut/shortcutService.ts
src/editor/contrib/command/commandList.contrib.ts
src/editor/contrib/command/editorCommand.ts
src/editor/common/editorContextKeys.ts
src/editor/common/proseMirror.ts
Fix a typo in editorView.ts.
  • Changed onDidFocus to onDidBlur in the get onDidBlur method.
src/editor/view/editorView.ts
Rename joinForward to JoinForward in editorCommands.ts.
  • Renamed joinForward to JoinForward in editorCommands.ts.
src/editor/contrib/command/editorCommands.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Bistard - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider renaming __registerOtherCommands to registerBasicCommands for clarity and consistency.
  • The removal of the shortcuts parameter from registerCommand in EditorCommandExtension requires careful review to ensure all commands are still accessible via keyboard shortcuts.
  • The addition of commandArgs: getArguments to shortcutOptions is a good way to provide context to commands, but ensure getArguments is performant.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant