Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
> This function should be used when the file to be deleted does not have an entry recorded in the files table.
Shouldn't that be {files_managed} table? Though I confess I'm not entirely sure what's going on with file tables now...
Comment | File | Size | Author |
---|---|---|---|
#13 | 1029068-13.patch | 5.98 KB | jhodgdon |
#6 | 1029068-6.patch | 7.93 KB | jhodgdon |
#5 | 1029068-5.patch | 7.92 KB | jhodgdon |
#2 | 1029068.patch | 1.7 KB | jhodgdon |
Comments
Comment #1
jhodgdonActually, this function is called from file_delete(), which is the function that uses the File API. So I think that line should be struck from the documentation. The other file_unmanaged_* functions do not have this line, and I think it's misleading. They should all be documented the same way.
This is probably a good project for a novice -- just make sure all the file_unmanaged_* functions use consistent wording at the top to explain that they're not doing any database stuff.
Comment #2
jhodgdonI decided I liked the wording that was in file_unmanaged_copy:
Copies a file to a new location without invoking the file API.
So this patch makes all the other file_unmanaged_* functions have similar wording, and removes misleading and repetitive lines trying to say the same thing.
Comment #3
joachim CreditAttribution: joachim commentedThat all looks nice and succinct.
+1 from me.
I think this bit could do to survive somewhere -- is there a page that gives an overview of the FileAPI? Something like:
"Functions for manipulating files such as file_save_data() have unmanaged counterparts such as file_unmanaged_save_data(). These are identical except that they do not invoke the File API -- that is, the {file_managed} table is not affected and none of the file_* hooks will be called."
Powered by Dreditor.
Comment #4
jhodgdonDo you think we should put this text on each of the functions? file_unmanaged_save_data was the only one with this paragraph. The rest just have links between the file_unmanaged_* and file_* equivalents, and the unmanaged ones say "without invoking the file API"... Maybe rather than just @see we could do:
See file_unmanaged_whatever() for the equivalent function that does not manage the file using the File API.
See file_whatever() for the equivalent function that does manage the file using the File API.
Thoughts?
Also, I guess all the file* functions are part of:
http://api.drupal.org/api/drupal/includes--file.inc/group/file/7
and we should probably put a note there at the least as to what "unmanaged" and "managed" files are.
Comment #5
jhodgdonHere's a new patch, which attempts to make things clearer, and further standardizes the file operations functions doc headers.
Comment #6
jhodgdonMissed one spot. Try this one.
Comment #7
joachim CreditAttribution: joachim commentedI've not gone over that patch with a fine toothcomb, but in general it looks good.
Comment #8
jhodgdonHere's hoping you have time to get out your fine-toothed comb and review this sometime soon then. :)
Comment #9
joachim CreditAttribution: joachim commentedToothcomb time found :)
Very good clean-up and standardization of the file API and function docs. RTBC.
Comment #10
webchick"+ * The File interface contains functions for handling "managed" and "unmanaged"
+ * files."
This could do with a bit more explanation, because my very next question reading this is "What's the difference between a managed and unmanaged file?" This is eventually explained, but not until I've read the remaining paragraph of information.
So I'd suggest adding a second sentence after that that lays out the difference summarized in a sentence (basically, stored in the database with full hook support e.g. for file fields on entities, vs. simply stored on the file system, e.g. the site logo), then go into more details in the following sentences.
It's also not clear to me why you're saying "See foo_function() for the other equivalent" rather than @see. We don't do that elsewhere. For example, we don't say "See foo_form_validate() for this form's validate function" we say "@see form_foo_validate()."
I think it would be better and more consistent to simply explain in the Common file handling functions defgroup that there are both managed and unmanaged file and that each function links to its opposite equivalent. Thoughts?
Comment #11
joachim CreditAttribution: joachim commented> It's also not clear to me why you're saying "See foo_function() for the other equivalent" rather than @see. We don't do that elsewhere. For example, we don't say "See foo_form_validate() for this form's validate function" we say "@see form_foo_validate()."
A simple @see would work if the functions were called file_unmanaged_dostuff and file_managed_dostuff. But when you're at file_unmanaged_dostuff, seeing a 'see file_dostuff' I just think 'Huh? What? What does that dostuff do that this dostuff doesn't? Argh am I using the wrong one!'
Comment #12
jhodgdonYes, I put in the text to explain why you would want to refer to the other functions. Just saying @see here seemed like not enough explanation of the difference. But I can take that back out if you feel strongly enough that it's not necessary.
I'll rearrange the explanation of managed/unmanaged if I can figure out a better wording.
Comment #13
jhodgdonOK, here's a new patch. I'm seeing webchick's point on the @see, so I both reworded the explanation about managed vs unmanaged, and make the See ... into just @see lines.
Comment #14
jhodgdond8 at this point
Comment #15
jhodgdon#13: 1029068-13.patch queued for re-testing.
Comment #16
joachim CreditAttribution: joachim commentedLooks good to me.
Comment #17
jhodgdon#13: 1029068-13.patch queued for re-testing.
Comment #18
catchDocs only, so tagging for backport.
Comment #19
webchickGreat work! Looks good to me. However, no longer applies.
Comment #20
jhodgdon#13: 1029068-13.patch queued for re-testing.
Comment #22
jhodgdonThis documentation was long since fixed up in some other issue.