-
Notifications
You must be signed in to change notification settings - Fork 1k
Support alternative alias syntax for cross-platform compatibility #6155
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
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…s handling Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
- Update validate_alias_type to use find_alias_key for proper YAML lookup - Fix build_aliases to remove old key structure before adding updated alias - Ensure validation happens before modifying aliases array - Handle both @foo and aliases: formats when removing old keys Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
The build_aliases method creates aliases with normalized keys (without @), but for backward compatibility with existing YAML files and test expectations, we need to add the @ prefix back when writing to YAML files. This ensures that: - Existing YAML files with @foo: format remain compatible - New aliases are written with @ prefix - Tests that expect @foo keys in YAML pass without warnings Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…iases Two issues fixed: 1. Alias updates now preserve existing fields - when updating an alias with --set-path, it keeps the existing user field instead of removing it 2. Runtime aliases (WP_CLI_RUNTIME_ALIAS) now normalize keys by removing @ prefix for consistency with internal storage Changes: - build_aliases() now preserves existing alias data during updates and merges with new values - get_aliases() in Configurator normalizes runtime alias keys by removing @ prefix Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Three improvements based on code review: 1. Fixed validate_alias_type to properly detect group aliases - checks for numeric keys instead of @-prefixed values since groups are now stored as ['foo', 'bar'] without @ prefix 2. Fixed indentation inconsistency in build_aliases elseif block 3. Changed process_aliases to write aliases using the new 'aliases:' format instead of @foo: format for better cross-platform compatibility and consistency Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
Windows PowerShell treats
@as an array expansion operator, requiring awkward quoting:wp "@foo" core version. This PR adds--alias=fooas an alternative to@fooand supportsaliases:YAML syntax alongside@foo:.Changes
YAML Configuration:
Configurator::add_alias()- Extracts alias parsing logic, normalizes both@foo:andaliases: { foo: ... }formatsConfigurator::merge_yml()- Processes newaliases:keyConfigurator::get_aliases()- Normalizes runtime aliases (fromWP_CLI_RUNTIME_ALIASenvironment variable) by removing@prefix for consistency@prefixCLI_Alias_Command::process_aliases()- Converts all aliases to newaliases:format when writing YAML files for better cross-platform compatibility and consistencyCLI Flag:
--alias=<name>toconfig-spec.phpRunner::init_config()- Checks for@foopositional arg before parsing, then checks for--alias=fooflag (takes precedence if both provided)Runner::run_alias_group()- Uses--alias=foointernally, displays@foofor consistencyRunner::run_ssh_command()- Filters--aliasand@aliasargs from SSH commandsDisplay & Management:
CLI_Alias_Command::list_()- Adds@prefix back for displayCLI_Alias_Command::normalize_alias()- Strips@prefix for storageCLI_Alias_Command::find_alias_key()- Helper to locate aliases in raw YAML (handles both@fooandaliases:formats)CLI_Alias_Command::validate_alias_type()- Updated to properly detect group aliases by checking for numeric array keys instead of @-prefixed valuesCLI_Alias_Command::build_aliases()- Fixed to preserve existing alias fields during updates and merge with new values, ensuring partial updates don't lose dataCompletions- Fixed to display@prefix for aliases in bash completionBug Fixes:
@foosyntax. The@foopositional argument is now detected and removed from$argvbefore argument parsing, preventing it from being misinterpreted as a command.@foo:andaliases: { foo: }formats--set-pathnow preserves the existinguserfield)aliases:format consistently for better cross-platform compatibilityWP_CLI_RUNTIME_ALIASenvironment variable for consistency with internal storage@prefix for user-facing outputvalidate_alias_type()to prevent preg_grep errorsExamples
YAML:
CLI:
Both syntaxes are fully interchangeable for reading. All newly written/updated aliases use the new
aliases:format for consistency and better cross-platform compatibility. All existing functionality preserved.Original prompt
This section details on the original issue you should resolve
<issue_title>Support alternative alias syntax for better cross-platform compat</issue_title>
<issue_description>The
wp-clicommand configuration has some mismatches with Windows Powershell.e.g. the
ALIAS_REGEXconstant used to configure remote instances because it uses the@symbol as a prefix.@is used (like the$character as a variable prefix) to expand the variable (converts a string variable in an array).wp-cli/php/WP_CLI/Configurator.php
Line 47 in b179c7a
Would that be possible to allow overriding the constant with some filter?
Alternative question, why are we using
@prodstructure to define the aliases in thewp-cli.ymlinstead of an array of values likedisabled_commandsorrequire?</issue_description><agent_instructions>Add a new aliases configuration entry in the
wp-cli.ymlfileAllow to use
--alias=fooinstead of@footo use an aliasKeep the classic arg notation working
Works properly with the local and global configs merged together even if both styles are used
Here's a previous attempt from a few years ago to give you some extra context: