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

PHP 8 compatibility (Gettext 4.x branch) #266

Merged
merged 1 commit into from Mar 10, 2021

Conversation

MichaelHoste
Copy link
Contributor

It just removes one char for PHP 8 compatibility (at least for our specs).

The error fixed is Argument #1 ($carry) must be passed by reference, value given


Note

We still use the Gettext 4.x branch as dependency in translation/laravel (for https://translation.io) to be able to keep using the Blade Extractor.

Do you know if someone already made a Blade Scanner compatible with 5.x?

Just for information, how hard do you think it would be to create this scanner by using the code of your 4.x extractor?

Thanks for this great library!

@oscarotero oscarotero merged commit 3ab1651 into php-gettext:4.x Mar 10, 2021
@oscarotero
Copy link
Member

Hi, thanks for fixing this!

About Blade Scanner for Gettext 5.x, it should be very easy to replicate what we have in 4.x because it simply compile the template (here: https://github.com/php-gettext/Gettext/blob/4.x/src/Extractors/Blade.php#L29) and then run the PHP code scanner (here: https://github.com/php-gettext/Gettext/blob/4.x/src/Extractors/Blade.php#L32)

In 5.x, I imagine something like the following (not tested):

class BladeScanner extends PhpScanner
{
    public function scanString(string $string, string $filename): void
    {
        // create a compiler somehow
        $compiler = new BladeCompiler();

        // convert Blade code to regular PHP code
        $code = $compiler->compileString($string);

        return parent::scanString($code, filename);
    }
}

We should provide a way to customize the BladeCompiler (as we have in 4.x with $options['facade'] and other options like a path for cache, etc. But basically, it's just that.

If you want to work on this, it would be great, i can create the BladeScanner repository and add you as a contributor.

@MichaelHoste
Copy link
Contributor Author

Thank you for the prompt new release @oscarotero !

And thank you for the extra information about the Blade scanner.

I may give it a try in the future to be able to bump the dependency, but I guess I may have to change a lot of other small things in our code like $translations::fromPoFile or $translation->toPoFile(). That's not overly complicated but I don't have the time right now.

If you want to work on this, it would be great, i can create the BladeScanner repository and add you as a contributor.

Maybe not today but when I'll have some code to show, that would be great, thanks! :-)

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.

None yet

2 participants