@tonypartridge can you look at #17661 (review) please

@brianteeman already done?

The sprintf stuff

Oh good point.. Maybe add some quotes then

done @brianteeman

I have tested this item ✅ successfully on a78203a

Which was what I originally did but changed to your one mentioned
@brianteeman before the pull ? will update shortly.

--
Tony Partridge

On 24 August 2017 at 08:28:16, Brian Teeman (notifications@github.com)
wrote:

@brianteeman commented on this pull request.

In administrator/language/en-GB/en-GB.com_redirect.ini
#17661 (comment):

@@ -6,7 +6,9 @@
COM_REDIRECT="Redirects"
COM_REDIRECT_ADVANCED_OPTIONS="Advanced"
COM_REDIRECT_BATCH_OPTIONS="Bulk process to add new URLs"
-COM_REDIRECT_BATCH_TIP="Enter expired URL (mandatory) with a new URL (optional) separated with | (eg old-url|new-url). Each line one entry!"
+COM_REDIRECT_BATCH_TIP="Enter expired URL (mandatory) with a new URL (optional) separated with %1$s (eg old-url%2$snew-url). Each line one entry!"

almost but not quite. As both arguments are the same then you can simply
use %s

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17661 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABVglggX5i47Vq-b01JI35XbZy3Ngntdks5sbSYQgaJpZM4O-No4
.

$1 and $2 was for the other one that you couldnt do where there were two different strings ;)

I have tested this item ✅ successfully on 5380c7c

@mbabker can I get your thoughts on this? AppVeyor isn't happy we are using %1$s twice, yet it's perfectly valid case to do this.

I would like to see this merged for J3.8.

The AppVeyor failures had nothing to do with this PR, just the usual platform instability there.

I haven't been going out of my way to scan for PRs to merge, mainly just grabbing RTC stuff or things people explicitly ping/nudge me on. On a glance PR looks fine.

Ahh no worries :-). Looks like this skipped an RTC label, it’s had two
tests and only changes are basic language changes.

--
Tony Partridge

On 29 August 2017 at 23:43:21, Michael Babker (notifications@github.com)
wrote:

The AppVeyor failures had nothing to do with this PR, just the usual
platform instability there.

I haven't been going out of my way to scan for PRs to merge, mainly just
grabbing RTC stuff or things people explicitly ping/nudge me on. On a
glance PR looks fine.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17661 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABVglgxvHV52LclKRZNEy_Ih4QTv4lTEks5sdJQIgaJpZM4O-No4
.

RTC as stated above.

That is incorrect, using %s requires multiple replacements to be defined.
It follows a structured order. See comments that's what @Quy also tested
and it failed with a warning.

--
Tony Partridge

On 30 August 2017 at 07:36:18, infograf768 (notifications@github.com) wrote:

@infograf768 commented on this pull request.

In administrator/language/en-GB/en-GB.com_redirect.ini
#17661 (comment):

@@ -6,7 +6,9 @@
COM_REDIRECT="Redirects"
COM_REDIRECT_ADVANCED_OPTIONS="Advanced"
COM_REDIRECT_BATCH_OPTIONS="Bulk process to add new URLs"
-COM_REDIRECT_BATCH_TIP="Enter expired URL (mandatory) with a new URL (optional) separated with | (eg old-url|new-url). Each line one entry!"
+COM_REDIRECT_BATCH_TIP="Enter expired URL (mandatory) with a new URL (optional) separated with %1$s (eg old-url%1$snew-url). Each line one entry!"

as it is the same variable, we do not need to use %1$s. Simpler to use %s
, i.e.
COM_REDIRECT_BATCH_TIP="Enter expired URL (mandatory) with a new URL
(optional) separated with %s (eg old-url%snew-url). Each line one entry!"

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17661 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABVgljZvhDkciF4qQhqF6UBoecpBeNBBks5sdQLigaJpZM4O-No4
.

I just tested, and it is totally fine. No warning at all.

It could very well be a behavior that is different across PHP versions. If it works now, which seems is the case, there is no need to change back to a variation that did not work.