Broke off from #1191236: Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged counterparts.

There is a minor grammatical error in the documentation header for the file_unmanaged_save_data function. The sentence:

If no value is provided, a randomized name will be generated and the file is saved using Drupal's default files scheme, usually "public://".

should be corrected to:

If no value is provided, a randomized name will be generated and the file saved using Drupal's default files scheme, usually "public://".
CommentFileSizeAuthor
#6 file_unmanaged_save_data-grammar_error-1192474-6.patch1.37 KBpillarsdotnet
#4 file_unmanaged_save_data-grammar_error-1192474-4.patch1.31 KBpillarsdotnet
#1 file_unmanaged_save_data-grammar_error-1192474-1.patch1.08 KBpillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Needs review
StatusFileSize
new file_unmanaged_save_data-grammar_error-1192474-1.patch1.08 KB

Patch to make the correction.

pillarsdotnet’s picture

pillarsdotnet commented
Title: Fix grammar error in file_unmanaged_save_data » Fix grammar error in file_unmanaged_save_data docs.
Issue tags: +Needs backport to D7
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

How about "and the file will be saved"?

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new file_unmanaged_save_data-grammar_error-1192474-4.patch1.31 KB

Corrected as suggested, and re-wrapped comments at 80 columns.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Wrapping isn't right - text from second line should be moved up to the first line (before your patched hunk). Also, only one space after . at end of sentence.

pillarsdotnet’s picture

pillarsdotnet commented
StatusFileSize
new file_unmanaged_save_data-grammar_error-1192474-6.patch1.37 KB

Corrected.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks good, thanks!
8.x/7.x please.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

Once again, a bit confused because in the other issues like this, we removed the "if no value specified" stuff.

pillarsdotnet’s picture

pillarsdotnet commented

In the other functions, the code to deal with the "if no value specified" case is missing, so the value is in fact required by the code, and the documentation was incorrect. In this function, there is code to deal with the "if no value specified" case, so the documentation is correct, except for the grammatical error.

Note that I also submitted patches for the other functions to make the code match the docs, rather than making the docs match the code. Since the doc fixes were committed, I closed the code-fixes as duplicates.

If we want to change this function to be consistent with the others in not dealing with the "if no value specified" case, I can submit yet another patch/issue.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Right. There's nothing in the code to deal with what happens if this required function parameter is missing. We don't normally document in PHP functions what happens if someone doesn't supply a required param, unless the code is doing something. Which it isn't here -- the original doc was wrong.

8.x/7.x please...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Got it, thanks.

Committed to 8.x and 7.x.

Automatically closed -- issue fixed for 2 weeks with no activity.