infograf768 / j4localise

developing a version of com_localise adapted to Joomla 4
GNU General Public License v2.0
3 stars 1 forks source link

PHP8 error when saving some ini files #42

Closed fontanil closed 2 years ago

fontanil commented 3 years ago

Hi I'm trying to translate Akeeba Tickets System (com_ats.ini admin side) and I can't save the file, I get this message: 0 The arguments array must contain 2 items, 1 given Debug and error reporting to maximum, there is one error when I open the list of files administrator/components/com_localise/Model/TranslationModel.php on line 317 (and line 321) I had no error with com_ats.sys.ini file. Regards, fontanil

Localise 5.0.0 beta 5 PHP 8.0.10

infograf768 commented 3 years ago

@fontanil I do not get this error with php 7.2.4 (after reformatting the en-GB strings). Have to upgrade my MAMP to get php8. Can you test with a php 7 version?

@Valc Can you test? here is the pack: https://www.akeeba.com/download/ats/4-0-4.html

ALSO @Valc we have another problem Installing that package on our com_localise 5.0.0-dev does not reformat the Akeeba strings which are installed in the core folders (and therefore they do not display in translations view..

The reason is that the langmetadata.xml already exists in the language folder, therefore the Akeeba strings are not reformatted. See the reformat() method in the LanguagesModel file.

                $file = constant('LOCALISEPATH_' . strtoupper($client)) . '/language/' . $folder . '/' . $folder . '.xml';
                $path = constant('LOCALISEPATH_' . strtoupper($client)) . '/language';

                if (File::exists($file))

also line 351 en-GB is included in the array array('.svn', 'CVS','.DS_Store','__MACOSX','pdf_fonts','overrides', 'en-GB')

fontanil commented 3 years ago

I tried on PHP 7.4: no more error when I save com_ats.ini Other errors did not disappear

Nicholas said that a Joomla! 4 specific version will be released (I don't know when).

infograf768 commented 3 years ago

Other errors did not disappear

Can you ellaborate?

fontanil commented 3 years ago

As I said: Debug and error reporting to maximum, there is one error when I open the list of files administrator/components/com_localise/Model/TranslationModel.php on line 317 (and line 321) French administration. Clic on "traductions" then select "administrateur", "french" image

infograf768 commented 3 years ago

Sorry, lost in translation I mean after reformating the files to :

mod_atsstats.ini mod_atsstats.sys.ini plg_actionlog_ats.ini plg_actionlog_ats.sys.ini plg_ats_customfields.ini plg_ats_customfields.sys.ini plg_ats_gravatar.ini plg_ats_gravatar.sys.ini plg_finder_ats.ini plg_finder_ats.sys.ini plg_search_ats.ini plg_search_ats.sys.ini plg_user_ats.ini plg_user_ats.sys.ini com_ats.ini com_ats.sys.ini lib_fof40.ini

you can save the file with php 7.4 but still have the errors, right?

lines are if ($line[0] == '#') and elseif ($line[0] == ';')

We have to find out what has changed in php8...

infograf768 commented 3 years ago

Installed php 8.0.8 and tested saving com_ats.ini. I get the same error as you did. The arguments array must contain 2 items, 1 given

What is weird is that this only happens with that file and only when using the edit view and not the raw (source) view.

I don't get the errors administrator/components/com_localise/Model/TranslationModel.php on line 317 (and line 321)

Valc commented 3 years ago

@infograf768 I will take a look ASAP

Valc commented 3 years ago

ALSO @Valc we have another problem Installing that package on our com_localise 5.0.0-dev does not reformat the Akeeba strings which are installed in the core folders (and therefore they do not display in translations view..

The reason is that the langmetadata.xml already exists in the language folder, therefore the Akeeba strings are not reformatted. See the reformat() method in the LanguagesModel file.

Yep, i think we need to keep in mind than a non core package can to be installed in any moment, so:

I have tested this code and now is simply focused to search on core language folders files with the "xx-XX" prefix to reformat them when present.

The "localise" and "xml" cases was already under control and it continue under control.

Note than we does not search at "non core" folders (type; administrator/componets/com_pim/language).

It is also posible from a recursive search from "site" path getting "language" folders from standar locations. I am using this one to start:

            // Recursive search of folders than contains "language" and the path.
            // False positives as "languages" or "com_languages" also are matching.
            $exclude = array('overrides', '.svn', 'CVS', '.DS_Store', '__MACOSX');
            $folders = Folder::folders($location, 'language', true, true, $exclude);

"$location" is "JPATH_SITE"

If you wanna test i can start a new PR to catch all the standar paths. Here the mod only for core language folders.

    /**
     * Reformat j3 lang files.
     *
     * @return  bool True on success
     *
     * @since   5.0
     */
    public function reformat()
    {
        $return  = false;
        $renamed = false;

        // Reformat ini files in extensions language folder
        $scans = LocaliseHelper::getScans();

        foreach ($scans as $scan)
        {
            $prefix     = $scan['prefix'];
            $suffix     = $scan['suffix'];
            $type       = $scan['type'];
            $client     = ucfirst($scan['client']);
            $path       = $scan['path'];
            $folder     = $scan['folder'];
            $extensions = Folder::folders($path);

            foreach ($extensions as $extension)
            {
                if (Folder::exists("$path$extension/language"))
                {
                    $tags = Folder::folders("$path$extension/language");

                    foreach ($tags as $tag)
                    {
                        $extlangpath = $path . $extension . '/language/' . $tag;
                        $inifiles    = Folder::files($extlangpath, '.ini$');

                        foreach ($inifiles as $inifile)
                        {
                            $newinifile = str_replace($tag . '.', '', $inifile);
                            rename( $extlangpath . '/' . $inifile,  $extlangpath . '/' . $newinifile);

                            if (strpos($inifile, $tag . '.') !== false)
                            {
                                $shortpath  = str_replace(JPATH_ROOT . '/', '', $path);

                                Factory::getApplication()->enqueueMessage(Text::sprintf('COM_LOCALISE_REFORMAT_EXTENSION_SUCCESS', $shortpath . $extension . '/language/' . $tag . '/'), 'message');

                                $return = true;
                            }
                        }
                    }
                }
            }
        }

        // End scanning for extensions language folders
        // Start core language folders

        $clients = array('site', 'administrator');

        if (LocaliseHelper::hasInstallation())
        {
            $clients[] = 'installation';
        }

        foreach ($clients as $client)
        {
            $folders = Folder::folders(
                constant('LOCALISEPATH_' . strtoupper($client)) . '/language',
                    '.',
                    false,
                    false,
                    array('.svn', 'CVS','.DS_Store','__MACOSX','pdf_fonts','overrides')
            );

            foreach ($folders as $folder)
            {
                $path = constant('LOCALISEPATH_' . strtoupper($client)) . '/language';

                $files   = Folder::files($path . '/' . $folder);
                $newPath = $path . '/' . $folder;
                $renamed = false;

                foreach ($files as $file)
                {
                    if ($file === $folder . '.xml')
                    {
                        rename($newPath . '/' . $file, $newPath . '/langmetadata.xml');
                        $renamed = true;
                    }

                    if ($file === $folder . '.localise.php')
                    {
                        rename($newPath . '/' . $file, $newPath . '/localise.php');
                        $renamed = true;
                    }
                }

                $inifiles = Folder::files($path . '/' . $folder, '.ini$');

                foreach ($inifiles as $inifile)
                {
                    $has_tag = substr($inifile, 0, strlen($folder)+1);

                    if ($inifile === $folder . '.ini')
                    {
                        rename($newPath . '/' . $inifile,  $newPath . '/joomla.ini');
                        $renamed = true;
                    }
                    else if ($has_tag === $folder . '.')
                    {
                        $newinifile = str_replace($folder . '.', '', $inifile);
                        rename($newPath . '/' . $inifile,  $newPath . '/' . $newinifile);
                        $renamed = true;
                    }
                }

                if ($renamed)
                {
                    $return = true;
                    Factory::getApplication()->enqueueMessage(Text::sprintf('COM_LOCALISE_REFORMAT_SUCCESS', $folder, $client), 'message');
                }
            }
        }

        if (!$renamed)
        {
            Factory::getApplication()->enqueueMessage(Text::_('COM_LOCALISE_REFORMAT_NONE'));
            $return = false;
        }

        return $return;
    }

EDITED Not noticed before than not core language folders ara also under control from the first block of code lines, so, not new PR seems be required due already working fine there. rename( $extlangpath . '/' . $inifile, $extlangpath . '/' . $newinifile);

Valc commented 3 years ago

About the other issue i get a " PHP Warning: vsprintf()" on PHP 7.4.24

[Fri Oct 22 18:06:36.497170 2021] [php7:warn] [pid 2631] [client 127.0.0.1:44300] PHP Warning:  vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123, referer: https://fertra.ddns.net/administrator/index.php?option=com_localise&view=translation&layout=edit&id=4197&client=administrator&tag=es-ES&filename=com_ats&storage=global
[Fri Oct 22 18:06:36.497292 2021] [php7:warn] [pid 2631] [client 127.0.0.1:44300] PHP Warning:  vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123, referer: https://fertra.ddns.net/administrator/index.php?option=com_localise&view=translation&layout=edit&id=4197&client=administrator&tag=es-ES&filename=com_ats&storage=global
[Fri Oct 22 18:06:36.551893 2021] [php7:warn] [pid 2631] [client 127.0.0.1:44300] PHP Warning:  vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123, referer: https://fertra.ddns.net/administrator/index.php?option=com_localise&view=translation&layout=edit&id=4197&client=administrator&tag=es-ES&filename=com_ats&storage=global
[Fri Oct 22 18:06:36.551996 2021] [php7:warn] [pid 2631] [client 127.0.0.1:44300] PHP Warning:  vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123, referer: https://fertra.ddns.net/administrator/index.php?option=com_localise&view=translation&layout=edit&id=4197&client=administrator&tag=es-ES&filename=com_ats&storage=global
Valc commented 3 years ago

@infograf768 @fontanil

The next 2 string seems are issued due the amount of "%s" are trying to handle.

AKEEBA_COMMON_UPDATE_DEVREL_INFO="Presumably you are using the development release because our support asked you to in order to test a new feature or verify a bug fix. Before installing version %s please make sure that it was released after %s, the date when your currently installed development release was published. Otherwise any new features or bug fixes will be removed. If you are not sure please click on the “More information” button below."

AKEEBA_COMMON_PHPVERSIONTOOOLD_WARNING_BODY="Your site is running on PHP %s which has stopped receiving security updates since %s. Using this on a live site is dangerous: unpatched security issues can get your site hacked. Moreover, we will only support obsolete versions of PHP for nine months since their end-of-life date. Therefore we will discontinue support for your PHP version on %s. We strongly advise you to ask your host to upgrade your site to PHP %s or later."

For me does not seems a com_localise issue: searching that keys at Akeeba code surerly will be founded the amount of vars is handling realy each one.

To test i have edited and modified a bit the line "123" of my /libraries/src/Language/Text.php file to:

$final_string = vsprintf($first_part, $string_parts);

if(!$final_string) { die($string); }


After delete both from language file to test,  no more warnings at my log. 
Valc commented 3 years ago

lines are if ($line[0] == '#') and elseif ($line[0] == ';')

We have to find out what has changed in php8...

@infograf768 About this one maybe is issued due not patch applied at the release of com_localise than @fontanil is using:

Firts task is get the "line" 0 and then eval it byif ($line[0] == '#'), etc, etc.

                while (!$stream->eof())
                {
                    $line = $stream->gets();

Not sure if this patch is applied on last one com_localise released, but before that "$line" code part was called at the end of the bucle and files without empty lines at the end was adding "ghost extra strings", so, it can to be the same but due not "line" called at the first bucle step to eval.

https://github.com/infograf768/j4localise/pull/35/commits/e79721dc0aec38a4720316cebeaebe1860589dda https://github.com/infograf768/j4localise/pull/35#issuecomment-932792262

infograf768 commented 3 years ago

After delete both from language file to test, no more warnings at my log.

I confirm taking off the first %s in AKEEBA_COMMON_UPDATE_DEVREL_INFO value lets save the file in normal edit (after deleting the other string). But this makes no sense at all as saving a 3rd party string with variables is not used by com_localise as a sprintf... As it works fine in Source mode (and in php 7.x), I do not think the 2 strings are the culprit.

Also, there are other strings in that file containing multiple variables.

infograf768 commented 3 years ago

About https://github.com/infograf768/j4localise/issues/42#issuecomment-949911037

This patch is not in beta5 but in -dev. As soon as we correct the issue with 3rd party installing lang files with lang tag in core folder, I will release a beta6.

Valc commented 3 years ago

After delete both from language file to test, no more warnings at my log.

I confirm taking off the first %s in AKEEBA_COMMON_UPDATE_DEVREL_INFO value lets save the file in normal edit (after deleting the other string). But this makes no sense at all as saving a 3rd party string with variables is not used by com_localise as a sprintf... As it works fine in Source mode (and in php 7.x), I do not think the 2 strings are the culprit.

Also, there are other strings in that file containing multiple variables.

After try to: grep -r -i "AKEEBA_COMMON_UPDATE_DEVREL_INFO" /var/www/html/ft > ~/Descargas/archivo3.txt

To get matches at Akeeba code, only the matches at the language ini files are returning, so, seem than that key is not present at Akeeba code yet.

This one does not point to the amount of "%s" at the string, for me point more than that amount does not mathes with the called by the code... but without be able to found from where that key is called from Akeeba code is not easy to determinate.

infograf768 commented 3 years ago

Patch proposed in https://github.com/infograf768/j4localise/issues/42#issuecomment-949782319 works great!

Please propose PR.

infograf768 commented 3 years ago

To get matches at Akeeba code, only the matches at the language ini files are returning, so, seem than that key is not present at Akeeba code yet.

com_localise does not care about Akeeba code imho. It just deals with the strings. The problem only shows in php8 and in normal edit mode...

fontanil commented 3 years ago

Hi, I seem to have got the same error in PHP 8 with com_localise.ini Please try, my server actually use PHP 7.4

Valc commented 3 years ago

To get matches at Akeeba code, only the matches at the language ini files are returning, so, seem than that key is not present at Akeeba code yet.

com_localise does not care about Akeeba code imho. It just deals with the strings. The problem only shows in php8 and in normal edit mode...

Job time now :) For me this one is not a com_localise bug. From some part that language file is "parsed" from Joomla API and then jump the warning. But i can take a look more in deep later.

infograf768 commented 3 years ago

I seem to have got the same error in PHP 8 with com_localise.ini

I do confirm this with php 8. :(

Valc commented 3 years ago
To test i have edited and modified a bit the line "123" of my /libraries/src/Language/Text.php file to:

$final_string = vsprintf($first_part, $string_parts);

if(!$final_string)
{
die($string);
}

I am back at home in 5 or 6 hours, so, you can advance testing what keys are jumping as warning after save com_localise.

fontanil commented 3 years ago

I recently had an error on a module when reading an array with, for example, $line[0] (empty result) on PHP 8 and needed to change to $line['field_name'] to get the value of the field. 'field_name' was the real name of the field. I'm not a coder so I don't know if it could be the same problem reading a string.

infograf768 commented 3 years ago

Found out something

If I take off the coma , in both strings concerned in com_ats.ini, it saves OK...

In AKEEBA_COMMON_PHPVERSIONTOOOLD_WARNING_BODY it is in Moreover, we will only In AKEEBA_COMMON_UPDATE_DEVREL_INFO it is init was released after %s, the date

As we have no issue saving from Source, I get more and more convinced that it is in com_localise code.

Valc commented 3 years ago

@infograf768 At com_localise are the next ones:

Warning: vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123
object(SimpleXMLElement)#8090 (1) { [0]=> string(193) "COM_LOCALISE_NOTICE_GITHUB_FILE_ADDED
A new file <strong>%1$s</strong> present in the Github %2$s branch, has been added to the local %3$s/language/en-GB folder." }
Warning: vsprintf(): Too few arguments in /var/www/html/ft/libraries/src/Language/Text.php on line 123
object(SimpleXMLElement)#8090 (1) { [0]=> string(396) "COM_LOCALISE_WARNING_DISABLED_ALLOW_DEVELOP_WITHOUT_LOCAL_SET
Note that the client %1$s is using a version of language files ('%2$s') different from the local installed instance ('%3$s') with 'Enable development reference language' disabled. Please, if you wanna continue with the feature disabled is suggested choose this client from the 'Location' filter to solve the issue." }

I am trying to undertand the passSprintf function at libraries/src/Language/Text.php file, that is called by "Text::_()" and there is no way:

  1. If no "comma" present at the string is returning directly false.. Why? It is not posible pass '%s' as valid sprintf at strings without commas? why is mandatory explode the strings by commas?
  2. One time commas are catched, then simply seems it does not know how to handle all cases.
  3. For me the wrong there is than the code is trying to Replace custom named placeholders with sprintf style placeholders
  4. This one or similar is not applied there:preg_match('/%([0-9]+\$)?s/', $the_full_string, $give_me_all_matches)

Without commas in Not evaluated: A new file <strong>%1$s</strong> present in the Github %2$s branch has been added to the local %3$s/language/en-GB folder.

Comma after first match "passed" or "allowed"

A new file <strong>%1$s</strong> present in the Github, %2$s branch has been added to the local %3$s/language/en-GB folder.

Comma after first 2 or more matches (actual case) is a log warning

So, no idea about why at edit view is crashing and at raw is not crashing, but this one is about required grammar commas use within the string... that are rightly used: the crash is comming from `passSprintf' code.

The fact is than "moving commas" the issue is solved, but i think we canot solve the issue from our side "moving commas" or "handling grammar commas from our code"

That com_localise keys and strings are actualy on a valid format type key="string", and i does not understant at all why all the keys within a language file need to be under a "precheck" about if are type "sprintf" or not, when no one is calling that keys from "sprintf" in that moment. The normal seems that when a key is directly called from "sprintf" then jump all the required warning if the string is not ok, no?

For me that "precheck" applied from passSprintf is actualy issued or crashed due the use of commas as exploder for the array of matches than seems are expecting "only one match by first and next comma".

infograf768 commented 3 years ago

I looked at https://www.php.net/manual/en/function.vsprintf.php

The main change it seems is :

8.0.0 | This function no longer returns false on failure.

would that explain our issue?

infograf768 commented 3 years ago

Note: PR to reformat 3rd party strings installed in core lang folders is now merged.

infograf768 commented 3 years ago

BTW, this is not limited to 3rd party ini files. We have the same issue with core files and commas and php 8 To test I modified com_joomlaupdate.ini by adding a comma. I edited it and tried to save. COM_JOOMLAUPDATE_UPDATE_LOG_START="Update started by user %2$s (%1$s), old version is %3$s."

Valc commented 3 years ago

I looked at https://www.php.net/manual/en/function.vsprintf.php

The main change it seems is :

8.0.0 | This function no longer returns false on failure.

would that explain our issue?

Yep, it is posible: Me is testing at PHP 7.4 and it is returning false for sure.

I can not update to 8.0 to test due my server have webs working there.

But the "false" returning by "passSprintf" before exec "vsprintf" code line is simply to say "this string is not Text::sprintf", and there are a lot of cases than that precheck is returning that type of "false", when realy that string can to use Text::sprintf from some part of the code.

At the end all those are warnings at precheck, due i think than when Text::sprintf is directly calling a key, the result handling the string is the expected, if the amount of vars within the string matches with the amoun of required "%x$s" at code (no more and no less than the required by code)

Valc commented 3 years ago

BTW, this is not limited to 3rd party ini files. We have the same issue with core files and commas and php 8 To test I modified com_joomlaupdate.ini by adding a comma. I edited it and tried to save. COM_JOOMLAUPDATE_UPDATE_LOG_START="Update started by user %2$s (%1$s), old version is %3$s."

Yes, each "wrong comma" will do it. I think this should be solved by Joomla Project code due it is not a com_localise issue.

Testing as sample the next fake cases should return real mistakes due our code or due our language files, when Text::sprintf is calling the key with a fake sample string:

Deleting a var from the string: COM_JOOMLAUPDATE_UPDATE_LOG_START="Update started by user %2$s (%1$s) old version is ." Adding a var to the string: COM_JOOMLAUPDATE_UPDATE_LOG_START="Update started by user %2$s (%1$s) old version is %3$s %4$s."

infograf768 commented 3 years ago

I made a beta6 install to concentrate on this issue if someone accepts to help. It should also solve the stream issue @fontanil had. com_localise-5.0.0-beta6.zip

Use a clean test site and install a new language. Check that English United Kingdom is selected as reference in the OPtions. Make sure memory limit is high in your php.ini (256M is good).

Valc commented 3 years ago

@fontanil

I suggest you contact with Nicholas about if is posible use the '%X$s' format when the string have more than one var to pass.

For example: SAMPLE_KEY="This key have one %s to handle and will be ok."

AKEEBA_COMMON_UPDATE_DEVREL_INFO="Presumably you are using the development release because our support asked you to in order to test a new feature or verify a bug fix. Before installing version %1$s please make sure that it was released after %2$s, the date when your currently installed development release was published. Otherwise any new features or bug fixes will be removed. If you are not sure please click on the “More information” button below."

Source: https://www.w3schools.com/php/func_string_vsprintf.asp

The vsprintf() function writes a formatted string to a variable.

Unlike sprintf(), the arguments in vsprintf(), are placed in an array. The array elements will be inserted at the percent (%) signs in the main string. This function works "step-by-step". At the first % sign, the first array element is inserted, at the second % sign, the second array element is inserted, etc.

Note: If there are more % signs than arguments, you must use placeholders. A placeholder is inserted after the % sign, and consists of the argument- number and "\$". See example two.

fontanil commented 3 years ago

Remember that the same error occurs with com_localise.ini where %1$s and %2$s are used

Valc commented 3 years ago

Remember that the same error occurs with com_localise.ini where %1$s and %2$s are used

Yes, i know: this one is generaly speaking: use a suggested placefolder format can not solve the "commas issue" :)

Valc commented 3 years ago

@bakual, please, bring a bit of light here: the "commas case" at "passSprintf" function is a Joomla bug? https://github.com/infograf768/j4localise/issues/42#issuecomment-950276070

fontanil commented 3 years ago

com_localise.sys.ini is using commas and I get no error on saving it. The problem lies elsewhere, in my opinion.

Valc commented 3 years ago

com_localise.sys.ini is using commas and I get no error on saving it. The problem lies elsewhere, in my opinion.

Yes, the issue is not due use commas or not, is due use commas at "wrong location under the passSprintf checks" (explained some coments above)

Of course, we need to use commas within the strings when is required, no matter the location of the comma within the string, so, the problem can not be solved by "deleting commas within valid strings".

https://github.com/infograf768/j4localise/issues/42#issuecomment-950156470

Bakual commented 3 years ago

As long as you don't have a comma in the language KEY, it should work fine I think. The comma is only used as a separator from the KEY to the values. But I honestly doubt this is even used somewhere. That code looks like a hacky solution introduced years ago by Peter van Westen 😄

The error message you get indicates that a method is called with not enough arguments. From memory this is something that changed between PHP7 and 8. PHP 8 is more restrictive with such errors that PHP 7 was. But you should see in the error logs on which exact line this happend.

Valc commented 3 years ago

As long as you don't have a comma in the language KEY, it should work fine I think. The comma is only used as a separator from the KEY to the values. But I honestly doubt this is even used somewhere. That code looks like a hacky solution introduced years ago by Peter van Westen smile

The error message you get indicates that a method is called with not enough arguments. From memory this is something that changed between PHP7 and 8. PHP 8 is more restrictive with such errors that PHP 7 was. But you should see in the error logs on which exact line this happend.

Thanks @Bakual :) Here my log output at PHP 7.4 https://github.com/infograf768/j4localise/issues/42#issuecomment-950156470

For me that passSprintf "precheck" is realy unrequred due is bugged.

fontanil commented 3 years ago

I used Jdump for J4 and the last string is COM_LOCALISE_NOTICE_GITHUB_FILE_ADDED="A new file %1$s present in the Github %2$s branch, has been added to the local %3$s/language/en-GB folder." $first_parst is null: image

Valc commented 3 years ago

And whats happens if you delete the comma from the string? A new file %1$s present in the Github %2$s branch has been added to the local %3$s/language/en-GB folder. or if you move the comma just after the first placeholder? A new file %1$s, present in the Github %2$s branch has been added to the local %3$s/language/en-GB folder.

fontanil commented 3 years ago

Sorry, the variable is not the good one, it's $first_part, not $first_parts and it's not empty image So I don't understand why $final_string = vsprintf($first_part, $string_parts); displays an error, no variable is empty or null

Valc commented 3 years ago

I think is due the amount of placehoders by "comma exploded": it is not under control by that code on ALL posible cases.

The "parts" are ok, but not the "first_part": seems than sometimes one or the other is counting wrong the placeholders due seems is "comma case dependant"

fontanil commented 3 years ago

The first_part is correct If I delete the comma, an error occurs on COM_LOCALISE_WARNING_DISABLED_ALLOW_DEVELOP_WITHOUT_LOCAL_SET="Note that the client %1$s is using a version of language files ('%2$s') different from the local installed instance ('%3$s') with 'Enable development reference language' disabled. Please, if you wanna continue with the feature disabled is suggested choose this client from the 'Location' filter to solve the issue."

0 The arguments array must contain 3 items, 1 given If I delete the comma (after "Please ") in that string, and and I leave the comma in "COM_LOCALISE_NOTICE_GITHUB_FILE_ADDED", no more error!

Valc commented 3 years ago

The first_part is correct If I delete the comma, an error occurs on COM_LOCALISE_WARNING_DISABLED_ALLOW_DEVELOP_WITHOUT_LOCAL_SET="Note that the client %1$s is using a version of language files ('%2$s') different from the local installed instance ('%3$s') with 'Enable development reference language' disabled. Please, if you wanna continue with the feature disabled is suggested choose this client from the 'Location' filter to solve the issue."

0 The arguments array must contain 3 items, 1 given If I delete the comma (after "Please "in that string, no more error!

Yes, this one is another string case: I have moved the comma just after first place holder again. COM_LOCALISE_WARNING_DISABLED_ALLOW_DEVELOP_WITHOUT_LOCAL_SET="Note that the client %1$s, is using a version of language files ('%2$s') different from the local installed instance ('%3$s') with 'Enable development reference language' disabled. Please, if you wanna continue with the feature disabled is suggested choose this client from the 'Location' filter to solve the issue."

No matter the string, the issue is try to handle more than one place holders by comma, as it is the string case again.

fontanil commented 3 years ago

I deleted the comma in that string (with ftp) and opened the file in Localise, found the line, added the comma ("Please,") and saved without error !

Valc commented 3 years ago

If I delete the comma (after "Please ") in that string, and and I leave the comma in "COM_LOCALISE_NOTICE_GITHUB_FILE_ADDED", no more error!

If this is as you say.. this can become a bit crazy to found, but the issue continue out of the com_localise code, i think :D

Valc commented 3 years ago

I deleted the comma in that string (with ftp) and opened the file in Localise, found the line, added the comma ("Please,") and saved without error !

And what language is loading? the en-GB or the fr-FR? have you test to modify both language files? Not sure about what language is called from that function, i think en-GB.

infograf768 commented 3 years ago

See https://github.com/joomla/joomla-cms/issues/35002

fontanil commented 3 years ago

Actually the administration is set to en-GB and I try to translate into french.

Valc commented 3 years ago

See joomla/joomla-cms#35002

Great. At this moment i have not more ideas or solutions than the already explained.

infograf768 commented 3 years ago

I really think that passSprintf is the culprit. Its code is just not accepted by php8. If I understand well, that code was made to let have variables in a string when using a simple JText::_() call instead of a JText::sprintf(). I have no idea who uses such case.

Maybe we could just kill the method and modify the _ function.