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

symfony-console arguments handling + WordPress Timber bridge #60

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

drzraf
Copy link

@drzraf drzraf commented Jul 20, 2018

  • This commit uses symfony-console for manage arguments specific to Twig-Gettext-Extractor which used to be broken.
  • The $input is also passed to the extractor for possible futur use (eg: handling --debug)
  • Better Twig loaded. Twig Environment use a ChainLoader which is initialized according to file/paths passed to the command line.
  • It's possible to add a template directory the Twig-way and compile for one (or multiple templates).
  • The list of Twig templates to compile can now be passed through stdin, using --files stdin
  • Twig_SimpleFilter is fixed. true is not an accepted value anymore. Empty value callback avoid the cryptic Twig error: Function 1() does not exist
  • A --require flag is added to add custom PHP code before compilation.
  • Using the above, a brigde with WordPress+Timber is added. It loads Timber Twig extensions and add a couple of WordPress specific parameters to xgettext.

Copy link
Owner

@umpirsky umpirsky left a comment

Choose a reason for hiding this comment

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

Thanks for this, look promising. 👍

Please update https://github.com/umpirsky/Twig-Gettext-Extractor/#setup since old setup does not work any more.

{
$this->environment = $environment;
$this->reset();
$this->input = $input;
Copy link
Owner

Choose a reason for hiding this comment

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

$this->input is not defined in Extractor.

@@ -5,6 +5,7 @@
* This file is part of the Twig Gettext utility.
*
* (c) Саша Стаменковић <[email protected]>
* (c) 2018 Raphaël Droz <[email protected]>
Copy link
Owner

Choose a reason for hiding this comment

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

Please add this as author, not a copyright holder.

composer.json Outdated
@@ -18,7 +18,8 @@
"symfony/filesystem": "~3.0|~4.0",
"symfony/translation": "~3.0|~4.0",
"symfony/form": "~3.0|~4.0",
"symfony/asset": "~2.8|~3.0|~4.0"
"symfony/asset": "~2.8|~3.0|~4.0",
"symfony/console": "^4.1"
Copy link
Owner

Choose a reason for hiding this comment

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

Please also support 4.0 with ~4.0. No reason not to.

Copy link
Owner

Choose a reason for hiding this comment

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

Ignore, I'll fix this in #61

}
}
$file_loader = new Twig_Loader_Array($files);
$loaders->addLoader($file_loader);
Copy link
Owner

Choose a reason for hiding this comment

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

$loaders->addLoader(new Twig_Loader_Array($files));

@@ -32,6 +32,11 @@ class Extractor
*/
protected $parameters;

/**
* @var `Symfony\Component\Console\Input\ArgvInput
Copy link
Owner

Choose a reason for hiding this comment

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

Remove ```. Use InputInterface instead.

{
$this->environment = $environment;
$this->reset();
$this->input = $input;
Copy link
Owner

Choose a reason for hiding this comment

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

Hang on, $this->input is never used, why do we have it here?

Copy link
Author

Choose a reason for hiding this comment

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

I initially intended to use some input arguments (like debug) inside the Extractor.
Finally it was not the case, but that may useful later.
I've no problem with dropping it btw

Copy link
Owner

Choose a reason for hiding this comment

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

Let's drop it.

* This commit uses symfony-console for manage arguments specific to Twig-Gettext-Extractor which used to be broken.
* The $input is also passed to the extractor for possible futur use (eg: handling --debug)
* Better Twig loaded. Twig Environment use a ChainLoader which is initialized according to file/paths passed to the command line.
* It's possible to add a template directory the Twig-way and compile for one (or multiple templates).
* The list of Twig templates to compile can now be passed through stdin, using --files stdin
* Twig_SimpleFilter is fixed. true is not an accepted value anymore. Empty value callback avoid the cryptic Twig error: `Function 1() does not exist`
* A --require flag is added to add custom PHP code before compilation.
* Using the above, a brigde with WordPress+Timber is added. It loads Timber Twig extensions and add a couple of WordPress specific parameters to xgettext.
* integrate umpirsky#53 (debugging)
@umpirsky
Copy link
Owner

Please update with latest master, travis fails now.

@umpirsky
Copy link
Owner

@drzraf What is the status of the PR? Looks like we only need to update README to be able to setup Poedit.

@drzraf
Copy link
Author

drzraf commented Jul 22, 2018

I updated the command in README.md, but not the text that appears inside the screenshot (new screenshot is needed). Could you?

@umpirsky
Copy link
Owner

umpirsky commented Jul 23, 2018

Sure, let's fix tests first.

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