field_read_instance doc says it will not return deleted instance, but that is a parameter option

Good catch! I think both of these lines should be removed:

Generally, you should use the field_info_instance() instead.
This function will not return deleted instances. Use field_read_instances() instead for this purpose.

Probably the phrase "directly from the database" is a bit misleading too, since the function it calls is doing a lot of stuff (invoking hooks etc.).

Good project for a novice doc contributor...

Removed the two lines, and tried to clarify the description. Needs a review for wording.

grumble grumble, line endings.

I don't think we want to lose the line that says to generally use field_info_instance(). It could have better grammar, but I don't think that point is incorrect.

So if you could put that line back in (even better, put it back in and fix the grammar so it makes sense), the rest of the patch is lovely.

How is this?

 * Generally, you should use the field_info_instance() to allow
 * other modules the opportunity to append additional info, 
 * formatters, and widgets, as well as to take advantag of caching.

Would help if "advantage" was spelled right.

There seems to be something wrong with your patch (probably Unix vs. windows/mac line endings)?

That aside, I would also omit "the" in:
+ * Generally, you should use the field_info_instance() to allow
and maybe it should also have a phrase "instead of this function" in there?

Removed "the", ensured proper line endings.
Browsing the api docs shows that most "use x instead" follow this format, so I left out "instead of this function", but if you think it would be more clear I will add it in.

I think most of the "use x instead" notes do use the word "instead", don't they? A quick grep found several hundred uses of the word "instead". I don't think that paragraph is clear without "instead" being in there at all. I'm fine with leaving out "of this function".

The placement for most of them is at the end of the sentance. Putting "instead" in as-is seems to make it a bit wordy.

Current

+ * Generally, you should use field_info_instance() to allow
+ * other modules the opportunity to append additional info, 
+ * formatters, and widgets, as well as to take advantage of caching.

"instead"

+ * Generally, you should use field_info_instance() instead to
+ * allow other modules the opportunity to append additional info, 
+ * formatters, and widgets, as well as to take advantage of caching.

"instead of..."

+ * Generally, you should use field_info_instance() instead of 
+ * this function to allow other modules the opportunity to append 
+ * additional info, formatters, and widgets, as well as to take
+ * advantage of caching.

Alternate?

+ * Generally, you should use field_info_instance() instead, as 
+ * it provides caching and allows other modules the opportunity
+ * to append formatters, widgets, and other information.

That last one reads fine to me, but I will wait for more feedback before rolling a patch and bogging down the poor bot.

Either of the last two is fine with me. Could also say:
Generally you should instead use ...

Pick your favorite. Thanks!

"instead use" seems a bit awkward to me, going with the last one as-is.

Patch is fine, if you can fix line endings or whatever is causing the testbot to fail.

There should be settings in your editor to use Linux/Unix line endings for PHP files (.module, .inc, etc.). Hopefully.

I have eclipse set up to http://drupal.org/node/75242 down to the letter, and have reviewed the settings several times now. It seems the CVS plugin just does not want to play nice when making patches.

Attached patch is same as #13, ran through a linefeed converter.

That's odd. I use Eclipse for Drupal patching on Windows frequently, and I don't have any problems with line endings. How are you creating the patches in Eclipse? I always save directly to the file system, then upload the file I have created. If you save to clipboard (which I think is the default), then the editor you are using to create the file could be screwing up the line endings.

This patch does have one problem I can see: there is a space at the end of this line:
+ * other modules the opportunity to append additional info,

Actually, the wording has reverted to the bad grammar from the patch in #6.

Thats...odd. I must have grabbed the wrong patch during conversion.

This patch actually contains the alternate text. Checked for spaces, and notepad++ confirms linefeeds are in unix format. Godspeed.

I second the RTBC motion.

Committed to CVS HEAD. Thanks.

Thanks for the fix, and sorry for chiming only after the event.

I'm not sure what this part refers to :
"and allows other modules the opportunity to append additional formatters, widgets, and other information"
To me, this implies an alter() hook on field_info_instance() - which doesn't exist.

From what I can trace out, the following hooks end up appending data to the output. It's late, and the function makes use of many other functions, so I probably didn't get all of them on this pass.

field_read_instance=>field_info_collate_fields=>hook_extra_fields
field_read_instance=>field_info_collate_fields=>_field_info_prepare_field=>field_info_field_settings=>_field_info_collate_types->hook_field_info, hook_field_info_alter (collate_types calls many hooks, but this seems to be the only hook whose data is used by field_settings)
field_read_instance->field_info_collate=>_field_info_prepare_field=>field_storage_details_alter

yched - so do you think that the note about using field_info_instance() is wrong, misleading, or should be reworded in some way? From #24 my inclination would be that it's correct, but you are more of an expert on the Field system than I am...