file_get_file_references() @return doc totally wrong

Someone must have copied and pasted that @return from another function -- good catch carlos8f! This is probably a good project for a novice patch contributor to fix.

It might also be useful to add a bit to the documentation: maybe explaining what a "file reference" is?

Subscribing.

I'd actually like to improve the docs in this area a bit more. There is not enough explanation of when to use file_usage_add() and file_usage_delete(). This will probably come up a fair bit in D7 as it's a new WTF. In the image_example #992778: Image example: Can't change style after image is loaded. recently came up.

Could we take a look and try to grok the actual file reference thing and get it handled in the handbook and in core code?

Sounds like this is not so much of a novice issue any more. :)

Well, i see the situation is still there, also in the dev version, it's funny (actually sad -no ofense) to see that this issue was practically resolved yet it was not aplied to D7.
"It might also be useful to add a bit to the documentation: maybe explaining what a "file reference" is?"
..please, please...;-)

neoglez: There is not a patch that has been accepted yet. We don't apply nearly/practically OK patches to Drupal, only patches that are completely OK. If you'd like to help by making a fully acceptable patch, that would be lovely!

Has to go into D8 first.

Updated return value to be array rather than integer and updated comments.

Hi Kathy, good start!

Your new patch does not quite follow our documentation standards:
http://drupal.org/node/1354
There needs a blank line between the summary and the new explanation paragraph.

A couple of other things I would fix:
- Since there are several parameters to this function, " If passing FIELD_LOAD_CURRENT," might be clearer as "If $age is FIELD_LOAD_CURRENT..." -- similar with the other value?

Hmmm... There are some issues with the first and second lines...
- I don't think that "for example" needs to be in the 2nd sentence of the explanation.
- I am not sure entities "declare" file references... if they do, this needs some explanation?
- I guess the whole explanation is a bit confusing to me. These things being called "references" are not references in the programming sense, they are entities or revisions that have a field with the file attached, right?. Maybe we should either explain that? How about just changing the first line to say

Returns a list of entities or revisions that reference a file.

Then the whole explanation could be left out, I think?

Updated to add blank lines and param definition updates per comment #14

Removed misc whitespaces that shouldn't have been there.

Looks good, thanks! The docs seem very clear to me, and are formatted correctly.

One grammar nitpick: The word "which"... it either needs to have a comma before it, or it should be "that" instead. I think it should be "that" in the two places in this patch.

Changed 'which' to 'that' (per #17) and re-rolled to use D8 core dir (11/1/11)

Sorry, must have missed this the last time. The line wrapping here isn't quite right -- some of the 2nd line should be moved up to the first line, so that it wraps as close to 80 characters as possible:

+ * Returns a list of entities or revisions that reference a file.
+ * A file should not be deleted if still referenced.
+ *

I'm also wondering if this would be clearer with a data type:

  * @param $field_type
- *   (optional) The name of a field type. If given, limits the reference check
- *   to fields of the given type.
+ *   (optional) A field type that limits the reference check to fields of the
+ *   given type.  Default is type 'file'.

@param string $field_type
The reason is that "types" in Drupal are sometimes strings, sometimes objects, sometimes arrays, and you took out the "name of a field type" that was in the original (not sure why)... actually I like the wording in the original better, myself.

Update per comments in #19

Nitpick: After a . at the end of the sentence, we need to violate everything we learned in 8th grade typing class and put only 1 space rather than two. :)

Other than that, looks great, thanks!

Updates per #21

Updates per #23

Wow, this patch changed a lot since the last time I reviewed it (#20). I am not really in favor of the current patch, with all the $ and () inside @code. We normally explain return structure by using words/lists.

Also, I'm not convinced that it is correct -- looking at the code for this function, it looks like it probably returns an array of objects, not an array of arrays as suggested in #5 and put into this patch. Feel free to tell me I'm wrong... but normally doesn't a query execute() return an array of objects?

OK I'm probably wrong on this. Here's the one function that calls file_get_file_references:
http://api.drupal.org/api/drupal/core--modules--file--file.module/functi...

Anyway, we do normally use wording like this to describe returned array structure:

An associative array of xyzs, in which the keys are foo IDs, and the values are associative arrays representing bars. The bar arrays have the following elements:
- abc: The blah.
etc.

I started working on this and came up with simply "Nested array of entities keyed by field name, entity type and entity ID.". That is imprecise since it does not consider all the variations.

Having this as wrong as it is, is silly. We should at least change it from "An integer" to "An array" without delay. Bumping priority a to get some attention.

Sorry, you can't just bump a documentation issue up to major to "get attention". Incorrect docs are just not "major". See http://drupal.org/node/45111

To get this fixed, the most appropriate response would be to make a new patch. :)

Also, the patch above has already been reviewed and marked "needs work". We need a new patch.

You are right. I was just feeling frustrated that something so obviously wrong and easy to fix has become derailed by such a big discussion. Would a patch that simply changes "Integer" to "Array" be accepted as an interim solution while we work out better documentation for this function?

Not really. We need to just figure out good documentation for the function and get that into a patch. We don't really tend to accept "interim fix" patches in Drupal Core.

Also, please leave the status at "needs work" until there is another patch to review. Those of us following the issue will still respond to comments, but "needs review" means "there is a patch to review". (There is a link to a page explaining issue status under the Status field, if you are confused.)

That is frustrating. But thank you anyway. :)

Looking at old issues... this was fixed in 8 but is still a problem in 7.

This will be solved as part of #1805690: file_get_file_references() is rather bogus.