Bug 248842

Comment 1 David Williams 2008-09-27 13:01:33 EDT

Just to cross-reference, see also bug 172412.

Comment 2 Ed Merks 2008-09-27 13:21:26 EDT

This is a completely orthogonal, but I noticed that many projects don't take advantage of setting the severity to "enhancement"  which is fine, but it results in community comments such as "there are 3000 bugs open against the platform," when in fact there are only 200 actual problems along with 2800 things on the wish list.  I'm making numbers up out of thin air, but you get the point.  Perceptions might well be enhanced by more carefully distinguishing between "it's broken" and "I want more goodness."

Encoding and decoding is an unavoidable scourge.  I'll be interested to see how this discussion goes, since EMF has a long history of being heavily URI based...

Comment 3 Jeff McAffer 2008-09-28 10:07:08 EDT

our url and unc path handling IS broken.  This bug is to capture that idea and the fact that other people's is quite likely broken as well.

Comment 4 Ed Merks 2008-09-28 11:31:51 EDT

If it's broken, the abstract is a little vague. If might better say *fix* path and URL support, along with some specifics of what's broken. :-P I.e., what "improvements" need to be made?

EMF effectively avoids issues to do with OS paths by insisting on the use of URIs for everything.  A UNC path would become file://server/volume/directory/file if handled by URI.createFileURI.  I wonder if that's right?

Comment 5 Jeff McAffer 2008-09-28 13:04:16 EDT

thanks for the linguistic review.  We would happily accept patches as well.

Comment 6 Ed Merks 2008-09-28 13:10:20 EDT

Nits about wording aside, the question is patches for what?  

I know there are problems because I have code like this in CommonPlugin and if I remember correctly, it wasn't possible to change the buggy platform behavior...

    /**
     * Use the platform to convert to a local URI.
     */
    protected static URI resolve(URI uri)
    {
      String fragment = uri.fragment();
      URI uriWithoutFragment = uri.trimFragment();
      String uriWithoutFragmentToString = uriWithoutFragment.toString();
      
      URL url = null;
      try
      {
        url = FileLocator.resolve(new URL(uriWithoutFragmentToString));
      }
      catch (IOException exception1)
      {
        // Platform.resolve() doesn't work if the project is encoded.
        //
        try
        {
          uriWithoutFragmentToString = URI.decode(uriWithoutFragmentToString);
          url = FileLocator.resolve(new URL(uriWithoutFragmentToString));
        }
        catch (IOException exception2)
        {
          // Continue with the unresolved URI.
        }
      }
      if (url != null)
      {
        try
        {
          return fix(url, fragment);
        }
        catch (IOException exception)
        {
          // Return the original URI.
        }
      }
      
      return uri;
    }

    protected static URI fix(URL url, String fragment) throws IOException
    {
      // Only file-scheme URIs will be re-encoded. If a URI was decoded in the workaround
      // above, and Platform.resolve() didn't return a file-scheme URI, then this will return
      // an decoded URI.
      //
      URI result = 
        "file".equalsIgnoreCase(url.getProtocol()) ?
          URI.createFileURI(URI.decode(url.getFile())) :
          URI.createURI(url.toString());
      if (fragment != null)
      {
        result = result.appendFragment(fragment);
      }
      return result;
    }

Of course I could just mind my own business as well...

Comment 7 Jeff McAffer 2008-09-28 13:34:10 EDT

Here is how you could help.  Using your experience, "identify best practices and coding patterns" and then supply patches that "make them [best practices and coding patterns] available for others to use".  Then we can proceed to "use them throughout the Equinox code base".

That is, forget about what is there, if you have code that does it all correctly, please supply it.  Tom has done some work on this but may not have caught all the cases etc.  Hopefully the code would be (or could be cast) in a form that everyone can use.  

Given the right utitility code, we can look at how update the code base to use the right behaviour.  For example, the Platform class you refer to is effectively deprecated.  In identifying the replacement, we can update the behaviour to be correct.  

Comment 8 Thomas Watson 2009-05-28 16:34:04 EDT

We did a bunch of work to improve URL/URI support in p2 and other areas in bug 207103 bug 237776 and we introduced the new URIUtil for manipulating URIs.  Nothing more is planned for 3.5.

We still have known issues with some URLs in the framework.  Mainly around setting the various location URLs for configuration, workspace etc.