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

use mnapoli/silly for cli applications #279

Merged
merged 26 commits into from
Sep 9, 2017
Merged

Conversation

glensc
Copy link
Member

@glensc glensc commented Jul 27, 2017

We have Symfony\Console as dependency since introduction of Phinx.

For long time wanted to convert the console scripts using Symfony\Console, they were even moved to classes (#137) with having in mind porting to Symfony\Console one day.

So today found Silly CLI micro-framework based on Symfony Console. and couldn't resist.

Symfony\Console benefits:

  • unified commandline arguments passing (--verbose, --quiet)
  • colored output!

probably could easily add debug messages with verbose increasing. currently no script is able to print more information if wanted to debug script execution.

new library dependencies:

  - Installing psr/container (1.0.0): Loading from cache
  - Installing container-interop/container-interop (1.2.0): Downloading (100%)         
  - Installing php-di/invoker (1.3.3): Downloading (100%)         
  - Installing mnapoli/silly (1.5.1): Downloading (100%)     

@glensc glensc changed the title WIP: use mnapoli/silly for cli applications use mnapoli/silly for cli applications Sep 8, 2017
not required, but silences EA notice.

From official documentation:

Note: For portability, it is strongly recommended that you always use the 'b' flag when opening files with fopen().

Note: Again, for portability, it is also strongly recommended that you re-write code that uses or relies upon the 't' mode so that it uses the correct line endings and 'b' mode instead.
note the commandline syntax changed
now requires email:download to be as first argument

also support for executing from web has been dropped.
if need to execute from web, create your own wrapper.
➔ rgrep -r const.DEFAULT_COMMAND src/Command/|nf|LC_ALL=C sort
'extension:enable';
'mail-queue:process';
'mail-queue:truncate';
'mail:download';
'mail:route';
'reminders:check';
'system:monitor';
@glensc
Copy link
Member Author

glensc commented Sep 8, 2017

Due the way Symfony\Console works, scripts that use non-option arguments, their invocation has been changed. Commands that used only options or none, for them it was possible to setup default command to be run without arguments. For these invocation has not changed.

Before After
bin/check_reminders.php [--debug] bin/check_reminders.php [--debug]
bin/download_emails.php [username] [hostname] [mailbox] bin/download_emails.php mail:download [username] [hostname] [mailbox]
bin/monitor.php [-q] bin/monitor.php [-q|--quiet]
bin/process_all_emails.php [filename] bin/process_all_emails.php mail:route [filename]
bin/process_mail_queue.php bin/process_mail_queue.php
bin/truncate_mail_queue.php bin/truncate_mail_queue.php [-q|--quiet] [--interval=]

UPATE: this is no longer valid, see #279 (comment)

@glensc
Copy link
Member Author

glensc commented Sep 8, 2017

perhaps should just have single entrypoint for those cli commands (except bin/upgrade.php and bin/irc-bot.php because they are different) and deprecate current ones.

what the name would be?

  • bin/cli.php
  • bin/eventum.php
  • bin/console.php
  • ...your idea here?

@glensc glensc requested a review from balsdorf September 8, 2017 20:41
@glensc glensc added this to the 3.3.0 milestone Sep 8, 2017
@glensc
Copy link
Member Author

glensc commented Sep 9, 2017

looks like with symfony/console 3.2 it's possible to enable $isSingleCommand:

http://symfony.com/doc/3.2/components/console/single_command_tool.html

and given that example don't even need silly library much (ok, it does provide DI and simple interface to define all arguments and options):

#!/usr/bin/env php
<?php
require __DIR__.'/vendor/autoload.php';

use Symfony\Component\Console\Application;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

(new Application('echo', '1.0.0'))
  ->register('echo')
      ->addArgument('foo', InputArgument::OPTIONAL, 'The directory')
      ->addOption('bar', null, InputOption::VALUE_REQUIRED)
      ->setCode(function(InputInterface $input, OutputInterface $output) {
          // output arguments and options
      })
  ->getApplication()
  ->setDefaultCommand('echo', true) // Single command application
  ->run();

so just need to add the $isSingleCommand argument to setDefaultCommand calls.

glensc added a commit to glensc/eventum that referenced this pull request Sep 9, 2017
@glensc glensc merged commit 5ff93b3 into eventum:master Sep 9, 2017
@glensc glensc deleted the cli-app branch September 9, 2017 20:07
@balsdorf
Copy link
Contributor

Sorry I didn't get a chance to review this right away but I think it is all good. Our cli scripts have been rather painful.

I like the idea of a single entrypoint as well for a future improvement, bin/eventum.php would be my first choice.

@cpinfold
Copy link
Contributor

cpinfold commented Sep 11, 2017 via email

@glensc
Copy link
Member Author

glensc commented Sep 11, 2017

dislikes:

  • bin/eventum.php we kind of already have eventum cli took named eventum.phar, could cause confusion
  • bin/process_jobs.php - not all commands "process jobs", bin/monitor.php for example currently is for checking system health, bin/extension.php is for managing eventum extensions

processing multiple commands, i had this idea too, but mostly because i invoke same command repeatedly for different projects in serial manner:

bin/download_emails.php eventum@example.org example.org INBOX.support
bin/download_emails.php eventum@example.org example.org INBOX

@balsdorf the cli scripts haven't been pain since 3.0.9 (Feb 6, 2016, #137), i.e they were organized into Command classes. this PR just added Symfony\Console support to add perhaps better commandline parser and colors.

@balsdorf
Copy link
Contributor

Good point on bin/eventum.php. Django uses manage.py but I can see how that wouldn't always be appropriate. I'm fine with console.php, command.php or manage.php but I don't think any name is going to be 100% accurate unfortunately.

@cpinfold You actually can run multiple commands in one cronjob, just separate the commands with a ';'.

* * * * * bin/download_emails.php eventum@example.org example.org INBOX.support; bin/download_emails.php eventum@example.org example.org INBOX

@cpinfold
Copy link
Contributor

cpinfold commented Sep 18, 2017 via email

glensc added a commit that referenced this pull request Oct 8, 2017
pld-gitsync pushed a commit to pld-linux/eventum that referenced this pull request Oct 8, 2017
glensc added a commit that referenced this pull request Oct 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants