API page: http://api.drupal.org/api/drupal/modules!image!image.module/function/the...
Can we please update this documentation to clearly state the type of path expected by this function? From the documentation it seems like the path is supposed to be a relative path in the form of /sites/default/files/my_image.jpg
However, this function accepts URIs now in the form of public://my_image.jpg
Comment | File | Size | Author |
---|---|---|---|
#14 | 1444650-5_d7.patch | 860 bytes | damiankloip |
#10 | 1444650-4.patch | 879 bytes | damiankloip |
#8 | 1444650-3.patch | 991 bytes | damiankloip |
#5 | 1444650-2.patch | 877 bytes | damiankloip |
#2 | 1444650-1.patch | 871 bytes | damiankloip |
Comments
Comment #1
jhodgdonIt looks like it should accept either, but probably you can't start your path with /sites (should be just sites without the leading slash). I agree that the documentation should be updated. It also applies to Drupal 8 (it's called 'uri' not 'path' in Drupal 8 but it does the same thing).
Needs a patch... possibly a good Novice project?
Comment #2
damiankloip CreditAttribution: damiankloip commentedSomething like this maybe?
Comment #3
damiankloip CreditAttribution: damiankloip commentedComment #4
jhodgdonThanks! The text in the patch looks good to me. But the addition is making the line longer than 80 characters, so it needs to be wrapped.
Comment #5
damiankloip CreditAttribution: damiankloip commentedHere we go, this has fixed that :)
Comment #6
jhodgdonNot quite! Just make the wrapped line go all the way to the left (indented the same as the rest of the @param section).
Comment #7
jhodgdonBy the way, here are the documentation standards:
http://drupal.org/node/1354
Comment #8
damiankloip CreditAttribution: damiankloip commentedhehe, ok. I think I see what you mean with the indentation. How about this??
Comment #9
jhodgdonUm, not quite. Just leave the existing lines as they were, and indent the new ones the same as the old ones. :)
Comment #10
damiankloip CreditAttribution: damiankloip commented:) Don't worry, I *WILL* get this....
I think I see the issue with the second patch (which was closer) now that I didn't notice before I posted it!
Comment #11
damiankloip CreditAttribution: damiankloip commentedComment #12
jhodgdonThere you go! :) Don't worry, I'm (hopefully) patient with new contributors. I'll get this committed to 8.x shortly. We will also need a patch for 7.x, which is the same except for the argument name being different (I think?).
Comment #13
damiankloip CreditAttribution: damiankloip commentedYay! good news. I am definitely new to the core contributions category! been doing all of my work in the contrib space. I will look at a D7 patch shortly...
Comment #14
damiankloip CreditAttribution: damiankloip commentedYep, I think the only difference is that the key is called 'path' in D7 and 'uri' in D8. Here is a re-rolled patch for D7. Will this fail until issue version tag is changed to D7?
Comment #15
webchickYep. You can name the patch with "do-not-test" in the name (e.g. 1444650-5_d7-do-not-test.patch) to make testbot skip it.
Comment #16
damiankloip CreditAttribution: damiankloip commentedAh, clever :) I did not know that.
Comment #18
damiankloip CreditAttribution: damiankloip commentedComment #19
jhodgdonCommitted to 8.x and 7.x. Thanks!