file_uri_target() purpose is unclear

subscribe

I am *very* confused as of what file_uri_target() is actually supposed to do (transform an URI... into what?)...

So it seems that file_uri_target() is actually just designed to return the part of the URI after the '://' (while keeping it URL-encoded). As a consequence, the current implementation looks correct.

But file_uri_target() feels completely foobar right now. I guess we need something like this.

I think flickr is not a good example because this kind of field should have own formatter and does not correlate with image field at all

subscribe. and don't forget that image != imagefield, and with stream wrappers there's no technical reason that image couldn't work on remote images.

This seems like the sanest way out of the mess we've made.

This looks very good to me. It's probably not the only modification we need to make to the stream wrappers, but it's a good first step.

i was going to test out an example or two, but remembered that, unfortunately, the stream wrapper implementation is still a bit broken, as we can't file_copy from an external stream, because file_exists returns FALSE for streams (which is called against the source uri in file_unmanaged_copy). thus, we have to use a raw copy() as in http://drupalcode.org/viewvc/drupal/contributions/modules/media/media.mo....

thus, i have no real way of knowing if this approach is sound or not, as the only place where it's going to even matter is for local file systems. boo.

setting status back.

subscribe

I believe pwolanin wrote a lot of this code so I'd like him to give it a thumbs up.

Looks like this is reverting basically back to what we had earlier, so it seems fine to me.

One comment - why use substr() when elsewhere we do this:

function file_uri_scheme($uri) {
  $data = explode('://', $uri, 2);

  return count($data) == 2 ? $data[0] : FALSE;
}

We should be consistent, so CNW to make this code match more like:

function file_uri_target($uri) {
  $data = explode('://', $uri, 2);

  if (count($data) == 2) {
    return trim($data[1], '\\/');
  }
  return FALSE;
}

Rerolled, your honor.

Matching the logic.

[edit] new patch fixes the problem

The logic in question is returning FALSE on failure, as the function did before.

This looks ok to me now.

Committed to CVS HEAD. Thanks.