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

Not possible to include vendor #84

Closed
remcotolsma opened this issue Sep 21, 2018 · 7 comments
Closed

Not possible to include vendor #84

remcotolsma opened this issue Sep 21, 2018 · 7 comments

Comments

@remcotolsma
Copy link

It looks like it's not possible to include the vendor folder in the make-pot command:

We have a few plugins where we require to include the vendor folder, how can we do this?

@swissspidy
Copy link
Member

You're right, it's currently not possible to override built-in excludes.

However, it was a conscious decision to only exclude a specific, limited set of files and folders.

What is your use case exactly?

vendor is typically used by Composer. In your theme/plugin you can add third-party libraries as dependencies and they will end up in that folder. These dependencies should obviously have different text domains than your project and you shouldn't have to care about translations of a dependency anyway.

@remcotolsma
Copy link
Author

remcotolsma commented Sep 21, 2018

We have split a WordPress plugin into a number of Composer packages, but they share the same text domain. Probably not the best design, but for now we can't change this very easily. I also don't know how it works with multiple text domains in one plugin and the https://translate.wordpress.org/projects/wp-plugins/ translation platform. For now fixed it with a custom make-pot command:

<?php

namespace Pronamic\WordPress\Pay;

class MakePotCommand extends \WP_CLI\I18n\MakePotCommand {
	public function __construct() {
		parent::__construct();

		// https://github.com/wp-cli/i18n-command/blob/v2.0.1/src/MakePotCommand.php#L36-L44
		$this->exclude = array_diff( $this->exclude, array(
			'vendor',
		) );

		$this->exclude = array_merge( $this->exclude, array(
			'build',
			'deploy',
			'documentation',
			'etc',
			'repositories',
			'wordpress',
			'wp-content',
		) );

		$this->include = array(
			'admin',
			'includes',
			'templates',
			'vendor',
			'views',
		);
	}
}

// https://github.com/wp-cli/i18n-command/blob/v2.0.1/i18n-command.php
\WP_CLI::add_command( 'pronamic i18n make-pot', '\Pronamic\WordPress\Pay\MakePotCommand' );
// wp pronamic i18n make-pot . languages/pronamic_ideal.pot --slug="pronamic-ideal"

@swissspidy
Copy link
Member

Have you tried out using a different folder for your plugin's dependencies in the meantime? For this, the dependencies simply need to require composer/installers. After that, you can use something like this in your plugin's composer.json file:

{
  "extra":{
    "installer-paths": {
      "lib": ["remcotolsma/some-package", "remcotolsma/some-other-package"]
    }
  }
}

This would be a bit easier than having to maintain a custom command.

Of course this only solves the problem for vendor.

I would have to think about how to best handle this if it turns out to be a major use case for other paths as well.

@schlessera
Copy link
Member

@swissspidy How about having --include take precedence over --exclude. This way, adding vendor to --include would override the predefined --exclude. I think this would cover pretty much any reasonable scenario.

@swissspidy
Copy link
Member

We could give it a go, sure.

Ideally we merge #83 first so we have some unit tests where we can more easily catch regressions.

@remcotolsma
Copy link
Author

@swissspidy Thanks for the composer/installers suggestion, we will try this.

@schlessera 👍

It would also be nice if the --include and --exclude arguments work similar to, for example, the grep or rsync --include and --exclude arguments.

I also noted that if you for example exclude src it will exclude the src folder in all folder levels. It's not possible to only exclude the root src folder? Probably also related to:

Leading and trailing slashes are ignored, i.e. /my/directory/ is the same as my/directory.

Is there not an existing library which can handle the --include and --exclude arguments correctly and similar to other CLI tools? Then we also don't have to write unit tests for the include/exclude options.

@schlessera
Copy link
Member

@remcotolsma With the merged PR #87 , you can now add --include=vendor to include the vendor folder into the processing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants