Stop forcing language/mirror for PHP manual links

This looks good to me -- the new URL is correct.

Should be fixed in Drupal 7 -- I'm not sure why it was changed to Drupal 8 (perhaps a slip of the finger).

Good spot, but also we shouldn't be forcing the english language PHP manual. Should use: http://www.php.net/manual/ini.list.php, which will redirect to appropriate language page.

Hmmm... settings.php is in English anyway... but I have no objection to changing the patch to the language-neutral link.

I sympathise, but sing that song at #299308: Installing Drupal by visiting index.php (rather than install.php) leads to a fatal error when PDO is not enabled.

Committed the patch in #2, so setting back to 'needs work' to address the language issue.

Here you go, fixes the existing ones too.

I took a look at this patch, and checked the URLs to see that they worked.

There are a couple of things that need fixing... These didn't come from this patch, so maybe they should be fixed separately, but they are all visible in the patch and problems that need to be fixed at some point, and mostly related to whether the URL that is being referred to is appropriate in the first place:

a)

  *
  * tmp_name does not have backslashes added see
- * http://php.net/manual/en/features.file-upload.php#42280
+ * http://php.net/manual/features.file-upload.php#42280

This comment paragraph is badly formatted. Probably should be $tmp_name, needs some punctuation, and maybe it belongs in the code rather than in the doc header anyway? Also, this URL points to a comment on the PHP doc site, which I am not sure is a good idea in the first place.

b)

  *
  * Page compression requires the PHP zlib extension
- * (http://php.net/manual/en/ref.zlib.php).
+ * (http://php.net/manual/ref.zlib.php).

This URL points to the function reference for zlib. A better URL would probably be http://us2.php.net/manual/zlib.setup.php, which explains how to install and set it up?

c)

  * @param $context
- *   Refer to http://php.net/manual/en/ref.stream.php
+ *   Refer to http://php.net/manual/ref.stream.php

That URL leads to a huge list of functions related to stream processing. Exactly how does that explain what $context means? This needs better documentation.

d)

       // Look for wildcards in the form allowed to be used in PHP functions,
       // because we are using these to construct the load function names.
-      // See http://php.net/manual/en/language.functions.php for reference.
+      // See http://php.net/manual/language.functions.php for reference.

This URL is pointing to a page that has 4 links on it to lists of language functions. Which function should I be looking at that will illumnate the form of wildcards used in PHP functions?

e)

- * @link http://php.net/manual/en/book.pdo.php
+ * @link http://php.net/manual/book.pdo.php

This should be @see I think.

f)

-        $requirements['gd']['description'] = t('The GD library for PHP is enabled, but was compiled without PNG support. Check the <a href="@url">PHP image documentation</a> for information on how to correct this.', array('@url' => 'http://www.php.net/manual/ref.image.php'));
+        $requirements['gd']['description'] = t('The GD library for PHP is enabled, but was compiled without PNG support. Check the <a href="@url">PHP image documentation</a> for information on how to correct this.', array('@url' => 'http://php.net/manual/ref.image.php'));

This URL should be pointing to the installation page, not the function refence page. Correct URL is http://us3.php.net/manual/image.setup.php

Same URL occurs again a few lines down in the patch.

I'm reviving this issue. I thought maybe this had been fixed on another issue, but there are still quite a few instances of php.net/manual/en or us.php.net links in the Drupal 8.x documentation.

Let's clean it up. Probably the best thing would be to just start over with a new patch at this point.

So the task (good for a novice I think?) is to find URLs in the documentation that contain php.net/manual/en, and take out the /en part. It's also probably a good idea to verify that the resulting URLs actually point to the right functions.

There are also a couple of URLs that have us.php.net in them, and they probably should be just php.net.

I have try to do a patch

This looks good, at least at first glance. If someone could take the time to check the URLs and make sure they work, that would be good.

One concern I had was this one:
http://www.php.net/manual/function.rawurlencode.php#86506
I was not sure if the comment referenced exists on the non-English sites, but I went to French and found it does:
http://www.php.net/manual/fr/function.rawurlencode.php#86506
So this is fine. Whew!

Anyway, it would be great if someone wanted to review this and verify the links. If not, I'll probably get to it in a day or two.

I have check URLs one by one and all are good.

I remove two "." at the end from URLs

The comment system on php.net is commun for all language

THANK YOU for checking the URLs, and removing those two "." from line ends.

The patch still needs a small amount of work -- there is some confusing between "See ..." and "@see".

You should use "See" in all // in-code comments, and whenever there is some narrative text. You should use @see inside of a /** */ documentation blocks, to generate a See Also section. Each line of @see should only have either a function name, group/topic machine name, file name, or URL.

Places where this is violated include:

+    // @see http://php.net/manual/regexp.reference.subpatterns.php

-   * See http://php.net/manual/en/pdo.constants.php for the definition of the
+   * @see http://php.net/manual/pdo.constants.php for the definition of the
    * constants used.

-   * See http://php.net/manual/en/pdo.constants.php for the definition of the
+   * @see http://php.net/manual/pdo.constants.php for the definition of the
    * constants used.

(and several other places like these two)

Also, let's take this line out of the patch:

-      $value = t('Enabled (<a href="http://php.net/manual/en/apc.configuration.php#ini.apc.rfc1867">APC RFC1867</a>)');
+      $value = t('Enabled (<a href="http://php.net/manual/apc.configuration.php#ini.apc.rfc1867">APC RFC1867</a>)');

The reason is that this is translated text. It was done in a non-standard way (the URL should normally be separate); we should have a separate issue for fixing that.

Update #14 with fixing #15 (I hope ;))

If you would have a separate issue I prefer let you create this ;)

Much better, thanks!

Remaining issue with the patch:

-   * See @link http://www.php.net/manual/en/language.oop5.magic.php#language.oop5.magic.sleep PHP Magic Methods @endlink.
+   * See http://www.php.net/manual/language.oop5.magic.php#language.oop5.magic.sleep PHP Magic Methods @endlink.

The @link got removed. Needs to be replaced. Next section of the patch has the same problem too.

And a note for backporting: MOST but not all of this patch is OK for Drupal 7. Anything inside the first argument to t() has to be removed from the patch for Drupal 7.

Regarding the other issue, I just filed: [EDIT - had wrong issue link here, sorry!]
#1471848: User interface text containing URLs in translated part

This is my first patch, so I hope I got it right. Fixing #18.

Thanks, that's exactly the right fix! I think this is ready to commit now. I'll have to see about critical issue thresholds and conflicts and things...

This no longer applies and will need a re-roll. Patch looks good though.

This is a re-roll.

Latest patch looks good, thanks!

This is not really just documentation, so I'll leave it to Dries/catch to commit. The patch above needs to be ported to D7 and not just committed there as-is too -- we can't change the UI text in D7 so those hunks need to be taken out.

Changing to UI text component so other committers know it's not just docs and I'm not taking it on. See note on #26 about D7.

The only strings I see that would impact D7 are:

-      $description = t('Your server is capable of displaying file upload progress, but does not have the required libraries. It is recommended to install the <a href="http://pecl.php.net/package/uploadprogress">PECL uploadprogress library</a> (preferred) or to install <a href="http://us2.php.net/apc">APC</a>.');
+      $description = t('Your server is capable of displaying file upload progress, but does not have the required libraries. It is recommended to install the <a href="http://pecl.php.net/package/uploadprogress">PECL uploadprogress library</a> (preferred) or to install <a href="http://php.net/apc">APC</a>.');

and:

-      $value = t('Enabled (<a href="http://php.net/manual/en/apc.configuration.php#ini.apc.rfc1867">APC RFC1867</a>)');
+      $value = t('Enabled (<a href="http://php.net/manual/apc.configuration.php#ini.apc.rfc1867">APC RFC1867</a>)');

However, the argument could be made that these are broken strings since they do not use array('@url' => 'http://www.php.net/manual/en/book.gmp.php')), type syntax which allows us to change URL destinations without breaking strings. Therefore, I think this patch is fine, even for Drupal 7.

Committed and pushed to 8.x. This will need a backport.

D7 backport.

Thanks @Albert Volkman!

Not sure what the PHP error is about, but #29 includes significantly fewer lines and files changed than #23. Is this intentional?

Confirmed the parse error locally:

[kepler:drupal | Thu 16:18:09]$ git apply php_manual-stop_forcing_language-692366-d7-29.patch 
[kepler:drupal | Thu 16:18:13]$ php -l modules/simpletest/simpletest.install 
Parse error: parse error in modules/simpletest/simpletest.install on line 73
Errors parsing modules/simpletest/simpletest.install

Ah, an extra curly bracket. Fixed!

There's several less lines because of the D8 core libraries.

Thanks Albert Volkman!

As far as I can see the lines for the dropped hunks still exist in core in D7; they are just in different files. Grep for '/en/' with the patch in #33 applied to see what I mean. :)

Marking Needs Work because we should add in all the corrections from the D8 patch.

Back for another round :)

Looks good at first glance! Whoever reviews this patch, we need to verify:
- No us.php.net or php.net/en/ remain in D7 after the patch is applied.
- None of the changes affects translated text (first argument to t() or similar functions -- if it's the second argument, the variable substitutions, that's OK).
- All the changes still point to the correct function URLs.

Hello
I have tested this patch on fresh drupal-7.x-dev and it works fine but i don't see differences beetwen links without /en/ and others !

RE #38 - if you were in another country, you might see the non-/en/ links redirect to a php.net site in a different language instead of to us.php.net/en/.

http://www.php.net/manual/ini.php is the correct link and automatically changes to the browser language.

For me it changes to /en/ even though I'm in The Netherlands. Probably because there is no Dutch translation and my browser is English, but dawehner (IRC nick) tested in Germany and the page changed to German.

I looked through the patch and all of the links link to the correct page on php.net. There were no other links to us*.php.net/* or php.net/*/en/* in D7. Several of the line numbers were off from other things so I have rerolled the patch also.

Thanks hosef and Ankabout for the testing of this patch, and the reroll!

The patch also looks good to me. It does introduce two translated string changes in file.install (the changes in other files in t() functions are in the variables substituted into t() and not in the translated strings). As noted by webchick above in comment #28, these string changes are really broken strings (bug fix) so they should probably be fixed in spite of it breaking translations. But since this changes these strings as well as the API docs, I'll leave this to the other maintainers to make the final decision and commit it.

I think this is fine for Drupal 7 too. However, we're trying not to break strings at arbitrary times (but rather only at the beginning of a release cycle to give translators maximum time to catch up), so at this point I think we should hold those off until after Drupal 7.15 is released.

If you don't want to leave a 26 kB patch lying around for that long, though, it would of course be OK to roll a new patch that fixes all the others and commit that part now, then wait on the two remaining strings.

But also, if we're going to change those two strings anyway, shouldn't we take the opportunity to change them to use the <a href="@url"> method that @webchick mentioned? That way, if the URL ever needs to change again (unlikely, but possible) we wouldn't have to break the string again. Moving back to "needs review" for that... and maybe applies to Drupal 8 also.

Yeah, I think that's a good idea. We've fixed similar incorrect links elsewhere in D7.

I think it's a great idea too.

So if someone wants to roll a patch that has everything but the two string changes mentioned in #42, I can get that committed.

Then we can follow-up with a patch that fixes the two strings in #42 by stripping the URLs out using an @-variable, and let David commit that at a more appropriate time in the release cycle.

Setting to "needs work" to indicate these patches need to be made. One at a time might be good... :)

Actually, let's fix the 2nd part of the issue on #1471848: User interface text containing URLs in translated part, which I'd forgotten about but someone just pinged today. It addresses this exact problem.

Attached is a patch for the latest version of D7 that fixes english-specific links in the documentation only. For this patch, we're assuming that english-specific links that are visible to the user (and thus run through a translate function) will be fixed in #1471848: User interface text containing URLs in translated part, so they've been disregarded.

I'm also attaching an interdiff between this patch and #41. The interdiff will show some things that look like I'm adding back in /en/ and us.php links, but those should be resolved by #1471848: User interface text containing URLs in translated part. Basically, if you apply this patch and the latest patch from that issue, it should catch all of the problem links (I hope!).

Thanks! That looks good to me.

Committed to 7.x.