Skip to content
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

Updates help generation for literal arguments #537

Merged
merged 3 commits into from Apr 3, 2024

Conversation

DerEchtePilz
Copy link
Collaborator

For #536
Now displays the literal value instead of the node name

For #536
Now displays the literal value instead of the node name
Copy link
Owner

@JorelAli JorelAli left a comment

Choose a reason for hiding this comment

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

Looks good - just a few minor changes please.

@DerEchtePilz DerEchtePilz merged commit 118704e into dev/dev Apr 3, 2024
3 checks passed
@DerEchtePilz DerEchtePilz deleted the dev/literal-argument-help branch April 3, 2024 13:46
willkroboth added a commit that referenced this pull request Apr 29, 2024
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
willkroboth added a commit that referenced this pull request Apr 29, 2024
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
willkroboth added a commit that referenced this pull request Apr 30, 2024
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
willkroboth added a commit that referenced this pull request May 1, 2024
Add tests for #454, #537, and f9c5fce. Just generally ensure code coverage for help generation.

Pulled repetitive logic into `CommandHelpTests#assertHelpTopicCreated` and `TestBase#enableServer`

Also, small change: The alias list in an alias help topic had a redundant `ChatColor.WHITE` character
willkroboth added a commit that referenced this pull request May 13, 2024
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
willkroboth added a commit that referenced this pull request May 14, 2024
- Reordered constructor arguments to more closely match the order in `ExecutableCommand`
  - This branch is now definitely backward incompatible (though not majorly)
- Replaced `List<String> argsAsStr` with `Node rootNode`
  - Added `RegisteredCommand.Node` class
    - Has an `argsAsStr` method as a replacement for the parameter of `RegisteredCommand`
    - Simplified representation of the Brigadier's `CommandNode` structure for the command
  - Removed `AbstractArgument#getHelpString` and `RegisteredCommand#arguments` (Added by #537)
    - The `RegisteredCommand.Node` structure is used to generate usage
    - `Literal` and `MultiLiteral` have separate logic for creating thier `RegisteredCommand.Node, wherein they define a different help string
    - TODO: #537 didn't have any tests, so I'm only guessing this new implementation works the same. In general, add more tests for usage generation.
- Removed `ExecutableCommand#getArgumentsAsStrings` and `AbstractArgument#appendToCommandPaths`
  - Added `AbstractArgument.NodeInformation` to help pass around enough information to generate Brigadier nodes and `RegisteredCommand.Node`s in one `AbstractArgument` tree traversal.
  - Modified the signatures of a few methods to facilitate this, including overriding methods
  - TODO: This broke `Previewable` arguments, so I'll have to tweak those
- Changed `CommandAPIHandler#registeredCommands` from an `ArrayList` to a `LinkedHashMap`
  - Instead of creating one `RegisteredCommand` object for each command path, `RegisteredCommand`s are merged when they share thier command name and namespace
  - One call to `ExecutableCommand#register` creates one `RegisteredCommand` object (if it has a namespace, an unnamespaced copy is created too)
  - `CommandAPIPlatform#postCommandRegistration` was reverted to original signature
  - NOTE: `CommandAPI#getRegisteredCommands` still returns a `List<RegisteredCommand>`
- Updated `CommandAPICommandRegisteredCommandTests` and `CommandTreeRegisteredCommandTests`
  - Added `RegisteredCommandTestBase` to handle common utility methods
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.

None yet

2 participants