Correct namespace containing hyphen

[harmenjanssen]

To allow vendors with dashes in their name, such as "grrr-amsterdam".

Right now, this fails silently, the plugin just isn't recognized by the PluginManager.

[LukeTowers]

This pull request has been made to the wrong branch. Please review the contributing guidelines as all PRs need to be made to the develop branch.

I'll fix it for you this time, but please ensure you make any future PRs to the develop branch, not the master branch.

[LukeTowers]

@harmenjanssen can you provide an example of a plugin that causes this issue to occur?

[harmenjanssen]

Certainly, it's this one: https://packagist.org/packages/grrr-amsterdam/oc-styleguide-plugin

(note that it's WIP)

[LukeTowers]

@harmenjanssen ah, I see. The more robust fix to apply here is to add a filter to $vars['name'] in https://github.com/composer/installers/blob/master/src/Composer/Installers/OctoberInstaller.php#L35 to remove all non-alphanumeric characters, or at the very least hyphens.

[harmenjanssen]

@LukeTowers Ah, right, that would be the better solution.

It's $vars['vendor'] in my case, right?
I'll add my PR to Composer Installers, thanks!

[LukeTowers]

@harmenjanssen I'm not sure tbh, play around with it and see if you can figure it out.

[harmenjanssen]

I finally got around to proposing a pull request over at Composer Installers, see composer/installers#397.

So I'm closing this one, hopefully the issue will be resolved soon.

[harmenjanssen]

@LukeTowers Composer Installers has merged the PR, but I still also need this feature to ensure the PHP namespace resolves correctly. 🙂
The files are loaded correctly and Grrr-Amsterdam will end up in the plugins directory, but the hyphen still ends up in generated namespaces.

Will you consider reopening reconsider this?

[LukeTowers]

@harmenjanssen then you've done your PR wrong. Your PR was supposed to remove the - entirely so I don't see how you have a hyphen involved at all anymore.

[harmenjanssen]

Argh, you're absolutely right, I misinterpreted a colleague's bug report.
The problem is my fix in Composer/Installers hasn't been tagged yet, so new October installations won't include the latest version. Something we can overcome for now by manually requiring a later version in composer.json.

Anyways, sorry for bothering, I will close this for good now. 🙂