In the default.settings.php there is the PHP settings part with the ini_set() functions.
At the beginning of this part - in the remark - there is an URL: http://www.php.net/manual/en/ini.php#ini.list
I think this page was moved, and the correct one is: http://www.php.net/manual/en/ini.list.php
Or just: http://www.php.net/manual/en/ini.php
Comment | File | Size | Author |
---|---|---|---|
#47 | php_manual-stop_forcing_language-692366-d7-47.patch | 17.15 KB | mariacha1 |
#47 | interdiff2.txt | 9.12 KB | mariacha1 |
#41 | php_manual-stop_forcing_language-692366-d7-41.patch | 26.54 KB | hosef |
#36 | php_manual-stop_forcing_language-692366-d7-35.patch | 26.53 KB | Albert Volkman |
#36 | interdiff.txt | 11.87 KB | Albert Volkman |
Comments
Comment #1
bradweikel CreditAttribution: bradweikel commentedComment #2
bradweikel CreditAttribution: bradweikel commentedComment #3
jhodgdonThis 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).
Comment #4
willmoy CreditAttribution: willmoy commentedGood 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.
Comment #5
jhodgdonHmmm... settings.php is in English anyway... but I have no objection to changing the patch to the language-neutral link.
Comment #6
willmoy CreditAttribution: willmoy commentedI 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.
Comment #7
Dries CreditAttribution: Dries commentedCommitted the patch in #2, so setting back to 'needs work' to address the language issue.
Comment #8
willmoy CreditAttribution: willmoy commentedHere you go, fixes the existing ones too.
Comment #9
jhodgdonI 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)
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)
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)
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)
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)
This should be @see I think.
f)
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.
Comment #11
jhodgdonI'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.
Comment #12
underq CreditAttribution: underq commentedI have try to do a patch
Comment #13
jhodgdonThis 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.
Comment #14
underq CreditAttribution: underq commentedI 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
Comment #15
jhodgdonTHANK 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:
Also, let's take this line out of the patch:
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.
Comment #16
underq CreditAttribution: underq commentedUpdate #14 with fixing #15 (I hope ;))
If you would have a separate issue I prefer let you create this ;)
Comment #17
underq CreditAttribution: underq commentedComment #18
jhodgdonMuch better, thanks!
Remaining issue with the patch:
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
Comment #19
kid_icarus CreditAttribution: kid_icarus commentedThis is my first patch, so I hope I got it right. Fixing #18.
Comment #20
kid_icarus CreditAttribution: kid_icarus commentedComment #21
jhodgdonThanks, 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...
Comment #22
Dries CreditAttribution: Dries commentedThis no longer applies and will need a re-roll. Patch looks good though.
Comment #23
underq CreditAttribution: underq commentedThis is a re-roll.
Comment #24
underq CreditAttribution: underq commentedComment #25
jhodgdonLatest patch looks good, thanks!
Comment #26
jhodgdonThis 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.
Comment #27
jhodgdonChanging 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.
Comment #28
webchickThe only strings I see that would impact D7 are:
and:
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.
Comment #29
Albert Volkman CreditAttribution: Albert Volkman commentedD7 backport.
Comment #31
xjmThanks @Albert Volkman!
Not sure what the PHP error is about, but #29 includes significantly fewer lines and files changed than #23. Is this intentional?
Comment #32
xjmConfirmed the parse error locally:
Comment #33
Albert Volkman CreditAttribution: Albert Volkman commentedAh, an extra curly bracket. Fixed!
There's several less lines because of the D8 core libraries.
Comment #34
xjmThanks 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.
Comment #36
Albert Volkman CreditAttribution: Albert Volkman commentedBack for another round :)
Comment #37
jhodgdonLooks 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.
Comment #38
icecta CreditAttribution: icecta commentedHello
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 !
Comment #39
jhodgdonRE #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/.
Comment #40
Ankabout CreditAttribution: Ankabout commentedhttp://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.
Comment #41
hosef CreditAttribution: hosef commentedI 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.
Comment #42
jhodgdonThanks 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.
Comment #43
David_Rothstein CreditAttribution: David_Rothstein commentedI 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.Comment #44
webchickYeah, I think that's a good idea. We've fixed similar incorrect links elsewhere in D7.
Comment #45
jhodgdonI 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... :)
Comment #46
jhodgdonActually, 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.
Comment #47
mariacha1 CreditAttribution: mariacha1 commentedAttached 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!).
Comment #48
jhodgdonThanks! That looks good to me.
Comment #49
jhodgdonCommitted to 7.x.