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

Add more CLI options (#19) #47

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

atrifat
Copy link
Contributor

@atrifat atrifat commented Dec 11, 2023

This PR will add more CLI options (#19 ) by introducing four new parameters: --created-at, --kind, --tags, --help. This PR also uses vanilla/garden-cli as CLI dependency instead of implementing manual parsing of CLI arguments as shown in previous implementation.

Previously, CLI tools nostr-php only supports --content, --relay, --key parameters.

nostr-php --help will give command outputs as follows:

usage: nostr-php [<options>]

Send nostr events to relays

OPTIONS
  --content, -c      Content (Required)
  --created-at, -a   Event created_at in unixtime (Optional).
  --help, -?         Display this help.
  --key, -p          Private key file location to use (Required).
  --kind, -k         Event kinds (Optional, Default: 1).
  --relay, -r        Relays to publish the events (Required).
  --tags, -t         Comma separated tags (Optional). Example:
                     t:nostr,t:travel,e:eventId

nostr-php command without any arguments will give outputs as follows:

[error] The content is empty

Usage:

To get complete help for commands:

nostr-php --help

Basic Usage:

nostr-php --content "Hello world!" --key /home/path/to/nostr-private.key --relay wss://nostr.pleb.network

Thank you.

@Sebastix
Copy link
Member

Thanks @atrifat ! Nice work.

What are the biggest advantages of using a new dependency / package?

The cli client is just a small addition for demonstrating basic command in my opinion.
I haven't written much cli applications in php, for the ones I've made I've used https://symfony.com/components/Console. If I would like to build a PHP cli client I would use this framework (I'm sure there a other good ones also) and use nostr-php as a package in it.

My suggestion would be to leave vanilla/garden-cli out (the less dependencies we use the better) and keep it very simple with plain php.

@atrifat
Copy link
Contributor Author

atrifat commented Dec 12, 2023

Thank you for your feedback @Sebastix .

What are the biggest advantages of using a new dependency / package?

The biggest advantage is simply that we can simplify the process of parsing and validation of CLI arguments. Instead of we have to manually reinvent or reimplement features by making additional function/class to parse the CLI arguments which prone to errors due to our limited experiences in making CLI apps. I'm sure that there are many other libraries to help with this (including symfony/console) thus open to change the implementation as long as it is simple enough.

$cli->description('Send nostr events to relays')
            ->opt('content:c', 'Content (Required)')
            ->opt('key:p', 'Private key file location to use (Required).')
            ->opt('relay:r', 'Relays to publish the events (Required).')
            ->opt('kind:k', 'Event kinds (Optional, Default: 1).', false, 'integer')
            ->opt('created-at:a', 'Event created_at in unixtime (Optional).', false, 'integer')
            ->opt('tags:t', 'Comma separated tags (Optional). Example: t:nostr,t:travel,e:eventId', false);

I think vanilla/garden-cli has simpler or lightweight implementation (based on their codebase) than full-blown features CLI libraries like symfony/console thus it becomes the main reason of library choice. symfony/console might be better choice if we write many CLI functions.

My suggestion would be to leave vanilla/garden-cli out (the less dependencies we use the better) and keep it very simple with plain php

I think i will look into that and probably make different version of PR later. But still, this approach might end up with situation where we have to make our own CLI parsers.

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.

2 participants