[#31465] - [4.0] Refactor JFTP error messages strings
- Fixed in Code Base
- 14 Dec 2020
- Medium
- Build: 4.0-dev
- # 31465
- Diff
- Bakual:4_RefactorLibJFTP
User tests: Successful: Unsuccessful:
Similar PR to #31456, but for JFTP strings in lib_joomla.ini
Currently, the language file lib_joomla.ini contains several strings which reference a PHP method. There are several issues with that:
- The method name should not be translated, but is part of the language string.
- The method is written in a format Classname: :Functionname (with a space between the : :). This isn't how a method is properly written.
- The classname references the old JFTP class name, not the new namespaced one.
- The same message exists multiple times, just for different functions. This is redundant work for TTs and not reuseable.
Summary of Changes
I've changed all strings starting with JLIB_CLIENT_ERROR_JFTP_ to a new format starting with JLIB_CLIENT_ERROR_FTP_ where the method name is passed using __METHOD__.
Eg:
JLIB_CLIENT_ERROR_JFTP_APPEND_BAD_RESPONSE="JFTP: :append: Bad response"
became
JLIB_CLIENT_ERROR_FTP_BAD_RESPONSE="%s: Bad response."
I've changed all calling places so the current method name (__METHOD__) is passed as a variable.
The amount of error strings went from 75 strings to 26.
Testing Instructions
You can try triggering an error by providing wrong credentials or so. But I think it's hard to trigger each error string.
So this should work with code review.
Actual result BEFORE applying this Pull Request
Error message would be for example
JFTP: :append: Bad response
Expected result AFTER applying this Pull Request
Error message would be for example
Joomla\CMS\Client\FtpClient::append: Bad response.
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration Language & Strings Libraries |
| Labels |
Added:
?
?
|
||
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31465.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31465.
Actually this is too late for j4, because removing language strings is a b/c break and have to be deprecated in 3.9.
So only way I see for this is to create only new JFTP strings (not sure if you did this) and deprecate the old one.
@HLeithner
some strings have already been deleted in #31456 and replaced, as here, by new strings using different constants
Sorry @HLeithner but the b/c promise doesnt apply to language strings. And
we have already removed over 2000 from j4 without any of them being marked
as deprecated
On Wed, 25 Nov 2020 at 11:14, Harald Leithner notifications@github.com
wrote:
@gostn https://github.com/gostn don't know why you gave me a thumb down
for just mentioning our b/c promise if you don't like it then create an
issue and we can talk about it to remove it.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#31465 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJ4P4MXDUHNI7M5LNL6NNDSRTRKTANCNFSM4T7MD6DQ
.
--
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/
https://developer.joomla.org/development-strategy.html#backward_compatibility
6.1.6 Language keys
Changing or deleting a language key is considered a backwards compatibility break. Adding new ones is not. Substantially changing the meaning associated with a language key is a compatibility break. Rephrasing something for a more accurate description or proper en-GB grammar is not.
if we don't care then we should remove this section from the b/c promise or extend it that they can be removed without notice in a major release. Changing the meaning should never be done anyway.
Within a series I agree it's an amazing ssue. Between a series should be allowed as we can break b/c
the point is only that the strings have not been deprecated, not that we can add new strings. Especially creating new strings doesn't hurt and removing the old strings in 5.0 with a deprecation notice in 4 is not a big deal.
As JM and I already said there are - thousands of unused strings removed already bwithout deprecations
That sounds like "we should break the rule even if we know that we breaking it" instead of fixing it....
The thing is, you can't really deprecated language strings.
You can add a comment on the line before, but only people actually looking into the INI file will see that. It will not be visible in Crowdin nor in PhpStorm. So it's not like deprecations in code where you actually get notified about it and thus the deprecation actually makes sense.
The time to require that every string removed in j4 because it is not used has to be marked as deprecated in j3 (even though no one will see it) was before 29 Sep 2016 when the first PR to remove strings from J4 was made. That was a huge amount of work and you can be assured that I checked, checked and double checked that the PR would be accepted BEFORE even starting the work.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-12-14 18:42:24 |
| Closed_By | ⇒ | wilsonge | |
| Labels |
Added:
?
|
||
I think this is a big improvement on the old strings - much more user friendly. Thankyou!
I have tested this item✅ successfully on 6efae96
A lot to look at! I put all of the new strings in enqueueMessage statements in the site and admin templates, with dummy variables, just to check they render. They do. Can't test the actual FTP stuff.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31465.