The drupal_realpath() documentation for its $uri parameter states:

If this value is omitted, Drupal's public files directory will be used [public://].

The statement is untrue and should be removed.

CommentFileSizeAuthor
#6 file_functions_docs_corrections-1991236-5.patch3.24 KBpillarsdotnet
#7 file_functions_corrections-1991236-7.patch4.49 KBpillarsdotnet
#3 file_functions_corrections-1991236-3.patch4.78 KBpillarsdotnet
#1 drupal_realpath-remove_untrue_statement-1191236-1.patch927 bytespillarsdotnet

Comments

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Needs review
StatusFileSize
new drupal_realpath-remove_untrue_statement-1191236-1.patch927 bytes

Patch to remove the untrue statement.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: The documentation for drupal_realpath() says the $uri parameter is optional, but this is not correct. » Various file functions doc say the $uri parameter is optional, but this is not correct.
Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The problem is bigger than just drupal_realpath(). I think we should fix it in the other file functions at the same time. For instance, file_copy has the same problem, which was reported here:
#1163402: file_copy doc says you can omit destination, but you can't
I'm marking that issue as a duplicate, because this one actually seems to have some action. Basically, we need to check all the functions that call drupal_realpath() and fix their doc too.

pillarsdotnet’s picture

pillarsdotnet commented
Title: Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives. » Various file functions doc say the $uri parameter is optional, but this is not correct.
StatusFileSize
new file_functions_corrections-1991236-3.patch4.78 KB

Okay, this is not just a documentation problem, so changing component.

I searched through includes/file.inc for the phrase "If this value is omitted, ... will be used, usually ...".

The docs for the file_unmanaged_copy function include this promise, and the function also contains code to implement the promised functionality. I used this example to modify other functions whose docs include the the promise but whose code lacks the implementation:

Similarly copied implementation code and documentation from file_unmanaged_save_data() to file_save_data(), correcting a slight grammar mistake along the way.

It seems to me that these changes merely implement what was originally intended, so adding the "needs tests" tag and bumping to "major".

Marking CNR for bot; if a senior core developer agrees that this is the right approach, I will also write tests.

pillarsdotnet’s picture

pillarsdotnet commented
Title: Various file functions doc say the $uri parameter is optional, but this is not correct. » Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives.
Component: documentation » file system
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: +Needs tests

Title: Various file functions doc say the $uri parameter is optional, but this is not correct. » Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives.
Status: Needs review » Needs work

The last submitted patch, file_functions_corrections-1991236-3.patch, failed testing.

pillarsdotnet’s picture

pillarsdotnet commented
Title: Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged counterparts. » Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives.
StatusFileSize
new file_functions_docs_corrections-1991236-5.patch3.24 KB

Alternative patch only changes docs to match implemented functionality.

If requested, I can break this all up into nine separate issues. EDIT: See #8

pillarsdotnet’s picture

pillarsdotnet commented
Title: Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives. » Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged counterparts.
Status: Needs work » Needs review
StatusFileSize
new file_functions_corrections-1991236-7.patch4.49 KB

Corrected patch from #3.

pillarsdotnet’s picture

pillarsdotnet commented
Status: Closed (duplicate) » Needs review

As requested by sun in IRC, broke this into nine separate issues, as follows:

Function Docs correction Code correction
file_copy() #1192440: Make docs of file_copy() match code. #1192446: Make code of file_copy() match docs.
file_move() #1192456: Make docs of file_move() match code. #1192458: Make code for file_move() match docs.
drupal_realpath() #1192466: Make docs of drupal_realpath() match code. #1192470: Make code of drupal_realpath() match docs.
file_unmanaged_save_data() #1192474: Fix grammar error in file_unmanaged_save_data docs. N/A
file_save_data() #1192482: Fix docs for file_save_data(). #1192486: Fix inconsistency in file_save_data() code.
pillarsdotnet’s picture

pillarsdotnet commented
Title: Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged alternatives. » Correct file_copy, file_move, file_save_data, and drupal_realpath to match documentation and their unmanaged counterparts.
Status: Needs review » Closed (duplicate)
valthebald’s picture

valthebald
Location Sofia
commented
Issue summary: View changes
Status: Needs review » Fixed

All subtasks are closed, marking fixed

Status: Fixed » Closed (fixed)

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