Skip to content

Use all caps pure-tokens in command "Usage:" #184

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

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

gmbnomis
Copy link
Contributor

@gmbnomis gmbnomis commented Dec 22, 2024

In the command page's "Usage:", the lower case name was used in case of a pure-token. However, the all caps token should be used instead. For MIGRATE, this makes an even bigger difference (see empty-string):

Before:

Usage:
    MIGRATE host port 〈 key | empty-string 〉 destination-db timeout [ copy ] [ replace ] [ AUTH auth | AUTH2 username password ] [ KEYS keys ] [ [ KEYS keys ] ... ]

Now:

Usage:
    MIGRATE host port 〈 key | "" 〉 destination-db timeout [ COPY ] [ REPLACE ] [ AUTH auth | AUTH2 username password ] [ KEYS keys ] [ [ KEYS keys ] ... ]

This addresses a part of #116

By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.

In the command page's "Usage:", the lower case `name` was used in case of a `pure-token`.
However, the all caps `token` should be used instead. For MIGRATE, this makes an even bigger
difference (see `empty-string`):

Before:

```
Usage:
    MIGRATE host port 〈 key | empty-string 〉 destination-db timeout [ copy ] [ replace ] [ AUTH auth | AUTH2 username password ] [ KEYS keys ] [ [ KEYS keys ] ... ]
```

Now:

```
Usage:
    MIGRATE host port 〈 key | "" 〉 destination-db timeout [ COPY ] [ REPLACE ] [ AUTH auth | AUTH2 username password ] [ KEYS keys ] [ [ KEYS keys ] ... ]
```

Signed-off-by: Simon Baatz <[email protected]>
Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

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

LGTM

@stockholmux
Copy link
Member

This looks good.

I get a little itchy when there are changes to the command reference parser due to the possibility of unintended consequences, but I ran a diff vs the command rendering on current iteration of the website and indeed this appears to only affect spacing (fine/irrelevant) and the intended capitalization.

@stockholmux
Copy link
Member

@madolson would you mind giving this a review as well?

@madolson madolson merged commit 7ffac1a into valkey-io:main Jan 7, 2025
1 check passed
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.

3 participants