SimpleMachines / SMF

Simple Machines Forum — SMF in short — is free and open-source community forum software, delivering professional grade features in a package that allows you to set up your own online community within minutes!
https://www.simplemachines.org/
Other
583 stars 250 forks source link

Ugrade hardcoded new language String #4889

Closed wintstar closed 6 years ago

wintstar commented 6 years ago

Forum version: SMF 2.1 Beta 4 (more detailed) Current SMF version: SMF 2.1 Beta 3 GD version: bundled (2.1.0 compatible) MySQLi engine: MariaDB MySQLi version: 10.1.29-MariaDB Alternative PHP Cache: 5.1.2 PHP: 7.1.11 (more detailed) Server version: Apache/2.4.33 (Win32) OpenSSL/1.0.2o PHP/7.1.11 Browser: Firefox Quantum 61.0.1 (64-bit) Last Pull Request of Today 4873

I have update my german language strings, but one Text is not translatet: Completed in

https://github.com/SimpleMachines/SMF2.1/blob/8902c394cf9228726a66db41053fed3131a262ff/other/upgrade.php#L4313

upgrade_new_strings

The string seems to exist: $txt['upgrade_completed_time2'] = 'Completed in %1$d';

frandominguezl commented 6 years ago

I'm not able to test it now. Can you tell me if this works?

setInnerHTML(document.getElementById("upgradeCompleted"), "', sprintf($txt['upgrade_completed_time2'], totalTime), '");';

wintstar commented 6 years ago

Text is no longer displayed, and the upgrade script stops at total progress 14% and progress 0.2%.

frandominguezl commented 6 years ago

Actually this would be harder to achieve. If you change the line to:

setInnerHTML(document.getElementById("upgradeCompleted"), "', $txt['sample'], '" + totalTime);';

You'll get the first part localized, so translator may translate "Completed in" but not totalTime, because of this:

https://github.com/SimpleMachines/SMF2.1/blob/1c20efcc4ed23882220a94f9f33fc567537471ef/other/upgrade.php#L4305-L4311

We could translate "hour", "minute" and "second" but they add an aditional s at the end if the variable is higher than 1, so I don't know what would be the best approach here. Thoughts?

wintstar commented 6 years ago

What would be added already exists. Have a look at the screenshot. I just noticed it myself.

Gesamtverarbeitungszeit (german) is the same as Completed in (english)

upgrade_new_strings2

frandominguezl commented 6 years ago

So leaving like that should work? Try adding a new text string that only contains "Completed in" in German and then place it in the line I pasted above. As I don't have any language packs installed I can't properly test it, so if you can say if that works, that'd be great.

wintstar commented 6 years ago

setInnerHTML(document.getElementById("upgradeCompleted"), "', $txt['sample'], '" + totalTime);';

This can be removed completely, it already exists. See screenshot. The Completed in display already exists before Pull Request #4873. Look here: https://github.com/SimpleMachines/SMF2.1/blob/8902c394cf9228726a66db41053fed3131a262ff/other/upgrade.php#L3611

The same function.

frandominguezl commented 6 years ago

Actually both aren't the same text. One of them is "Time Elapsed" (the upper one), and the other one is "Completed in".

Also, "Completed in" is only shown if extra debug information option is enabled, afaik.

wintstar commented 6 years ago

Look at the screenshot above. The same time is displayed. It may be a different text but it has the same function.

frandominguezl commented 6 years ago

Yes, it has the same function and actually shouldn't, so we don't want to remove it.

wintstar commented 6 years ago

Yes, it has the same function and actually shouldn't, so we don't want to remove it.

Why show 2 times the same? Just because they are different texts, yet have the same function. I don't get it.

frandominguezl commented 6 years ago

The first one is shown always, the second one is shown after you select the "Extra debuf information" option at the beginning of upgrade proccess. They're not the same.

wintstar commented 6 years ago

Then it would be better to hide the above display in debug mode and display the debug time there.

NanoSector commented 6 years ago

Isn't it possible to split the time into x minutes and y seconds where x and y are different variables? Then you could simply sprintf() (or the javascript equivalent) the variables into the string, problem solved.

EDIT: Looking at the code we have diffMinutes and diffSeconds, even diffHours (seems a bit extreme though, but okay). We could use two strings, 'completed_in_with_hours' and 'completed_in' or something, with 'Completed in %d hours, %d minutes and %d seconds' and 'Completed in %d minutes and %d seconds' respectively.

jdarwood007 commented 6 years ago

I will look into this. I added this to the upgrader.

jdarwood007 commented 6 years ago

Ok, I think this should be changed as follows.

There is 2 places this occurs in, one is in debug mode after the db upgrade is done. The second one is at the very end.

The debug mode one, we can let it be a little more simple, as its mainly used for development purposes to understand how long the database upgrade part took (actually time from start till db upgrader finished actually).

That language string becomes:

$txt['upgrade_success_time_db'] = 'Successful! Completed in %1$d hours, %1$d minutes and %3$d seconds.';

That code becomes:

            if ($is_debug)
            {
                $active = time() - $upcontext['started'];
                $hours = floor($active / 3600);
                $minutes = intval(($active / 60) % 60);
                $seconds = intval($active % 60);

                echo '', sprintf($txt['upgrade_success_time_db'], $hours, $minutes, $seconds), '<br>';
            }
            else
                echo '', $txt['upgrade_success'], '<br>';

The second one needs to be a bit more friendly since it is intended for general upgrades to know how long the entire process took.

So those language strings

$txt['upgrade_completed_time_hms'] = 'Completed in %1$d hours, %2$s minutes and %3$s seconds';
$txt['upgrade_completed_time_ms'] = 'Completed in %2$s minutes and %3$s seconds';
$txt['upgrade_completed_time_s'] = 'Completed in %3$s seconds';

And that code becomes:

            if ($upcontext['current_debug_item_num'] == $upcontext['debug_items'])
            {
                $active = time() - $upcontext['started'];
                $hours = floor($active / 3600);
                $minutes = intval(($active / 60) % 60);
                $seconds = intval($active % 60);

                $totalTime = '';
                if ($hours > 0)
                    $totalTime .= $hours . ' hour' . ($hours > 1 ? 's' : '') . ' ';
                if ($minutes > 0)
                    $totalTime .= $minutes . ' minute' . ($minutes > 1 ? 's' : '') . ' ';
                if ($seconds > 0)
                    $totalTime .= $seconds . ' second' . ($seconds > 1 ? 's' : '') . ' ';
            }

            echo '
                    <p id="upgradeCompleted">';

            if (!empty($totalTime) && $hours > 0)
                echo '', sprintf($txt['upgrade_completed_time_hms'], $hours, $minutes, $seconds), '';
            elseif (!empty($totalTime) && $minutes > 0)
                echo '', sprintf($txt['upgrade_completed_time_ms'], $minutes, $seconds), '';
            elseif (!empty($totalTime) && $seconds > 0)
                echo '', sprintf($txt['upgrade_completed_time_s'], $seconds), '';

I think this gets us what we need. There isn't a elegant way without adding a 4th string to do hours without minutes but have seconds, or just minutes. I think it will be best to accept it saying 0 seconds or 0 minutes in those cases. Otherwise we will have to add more language strings to make things "correct".

We could also format the time as HH:MM:SS and say it like "Upgrade completed in 02:32:16". Looks a bit weird but simple language handling then.

Edit,

I flipped around the 2 codes, but the concept still applies. The db one is the one we actually always show, the end upgrader one is the one only for debug mode.