Opened 11 years ago

Closed 7 years ago

Last modified 7 years ago

#6175 closed enhancement (fixed)

[patch] [cla] dojo.Uri

Reported by: wolfram Owned by: James Burke
Priority: high Milestone: 1.7
Component: Dojox Version: 1.0
Keywords: Cc: James Burke
Blocked By: Blocking:

Description (last modified by wolfram)

I needed a class that allows me to interact with URLs easily and prevent the hand-parsing all the time. I extended dojo.Url. Would be nice to see it in dojo. The tests are written using DocTest? using #4651 (pushing another ticket :-)) You can see them running here http://wolfram.kriesing.de/dojo/util/doh/runner.html?testModule=dojo.tests.Uri

Attachments (3)

patch6175.diff (8.5 KB) - added by wolfram 11 years ago.
patch6175.2.diff (11.3 KB) - added by wolfram 11 years ago.
patch6175.3.diff (11.3 KB) - added by wolfram 11 years ago.
DocTests? are now under "tests:"

Download all attachments as: .zip

Change History (19)

Changed 11 years ago by wolfram

Attachment: patch6175.diff added

comment:1 Changed 11 years ago by wolfram

Summary: dojox.Url[patch] [cla] dojox.Url

comment:2 Changed 11 years ago by Adam Peller

Cc: James Burke added
Milestone: 1.2
Type: defectenhancement

not sure if/how this overlaps with our existing url code.

comment:3 Changed 11 years ago by James Burke

wolfram: how would you differentiate this with what we have in dojo._Url: http://trac.dojotoolkit.org/browser/dojo/trunk/_base/_loader/loader.js#L525

dojo._Url should probably have a dojo.Url mapping now, since it is not truly private, but there is a separate ticket open on that.

comment:4 Changed 11 years ago by wolfram

hi jburke, obviously not much (just details), i just didnt find it :-(, only that dojo._Url is private, that should change then imho.

/me goes and tries the grep he used to search again

i just wanted to create a class, that allows for parsing and building a url, forth and back you know ...

comment:5 Changed 11 years ago by wolfram

If I had had #6185 I probably would have suggested to make dojo._Url public :-) and added the functionality that was missing.

Changed 11 years ago by wolfram

Attachment: patch6175.2.diff added

comment:6 Changed 11 years ago by wolfram

Summary: [patch] [cla] dojox.Url[patch] [cla] dojox.Uri

Its becoming more complete. I added

>>> new dojox.Uri("/").isAbsolute()
false

>>> new dojox.Uri("/").getAbsolute("http", "localhost")
"http://localhost/"

>>> url = new dojox.Uri("http://localhost/#frag#ment") // Fragment with # in it
>>> url.toString() 
"http://localhost/#frag%23ment"

Now also handles fragment (aka anchor) So getting more complete than dojo._Url I guess....

comment:7 Changed 11 years ago by wolfram

Description: modified (diff)

Changed 11 years ago by wolfram

Attachment: patch6175.3.diff added

DocTests? are now under "tests:"

comment:8 Changed 11 years ago by wolfram

Description: modified (diff)
Summary: [patch] [cla] dojox.Uri[patch] [cla] dojo.Uri

comment:9 Changed 11 years ago by wolfram

The upload into trac doesnt work, please download the patch here http://wolfram.kriesing.de/dojo/dojo/patch6175.diff the pretty simple Uri.js class here http://wolfram.kriesing.de/dojo/dojo/Uri.js

comment:10 Changed 11 years ago by Adam Peller

see also #5548

comment:11 Changed 11 years ago by James Burke

Some comments on the patch:

  • getQueryString() function: is this just dojo.objectToQuery()? Seems better to use that, or not expose teh getQueryString() function and have people call dojo.objectToQuery(url.query). Similarly, in your patch's constructor function, it could use dojo.queryToObject().
  • It derives from dojo._Url, but it does not seem to call the dojo._Url constructor.

In general, dojo._Url (in loader.js) seems to have a port an authority field, an instance of dojo._Url has a .query string property -- if you want it parsed, use dojo.queryToObject(). If you modify a dojo._Url instance, it looks like you can feed a dojo._Url object to the dojo._Url object to create a new dojo._Url object. This would seem to accomplish what you were hoping to get with the toString() method in this patch.

That leaves getAbsolute() as the only missing functionality from dojo._Url? The isAbsolute test seems simple enough to not need a wrapper? Or maybe fold it into dojo._Url? For getAbsolute(), I wonder, would something like this work since dojo._Url can take multiple dojo._Url/string arguments?

var url = new dojo._Url(/*whatever*/);
//Make an absolute:
url = new dojo._Url((url.scheme || window.scheme), (url.host || window.location.hostname), url), url);
}

I guess the bigger issue is to demonstrate where dojo._Url is really insufficient to require a new class. I'm also wary of using dojo.declare for it, and then also managing the namespace issue with dojo._Url and this class.

Maybe you can give more feedback as to why dojo._Url is insufficient, and should we just patch dojo._Url to address those concerns? We probably need to make it "public" but that is being tracked in another ticket.

comment:12 Changed 11 years ago by Adam Peller

Owner: changed from Adam Peller to wolfram

comment:13 Changed 11 years ago by bill

Milestone: 1.21.3

bump enhancements to next milestone, as we prepare to close out 1.2

comment:14 Changed 11 years ago by bill

Milestone: 1.3future
Owner: changed from wolfram to James Burke

comment:15 Changed 7 years ago by bill

Milestone: future1.7
Resolution: fixed
Status: newclosed

Regarding public functions, we've had dojo.moduleUrl() for a while, and now in 1.7 we have require.toUrl(), example usage: require.toUrl("dijit/foo/template.html").

Not sure if that satisfies the requirement of this ticket? I'll close this ticket for now, please open a new one if you still want more functionality and have a patch for it.

comment:16 Changed 7 years ago by ben hockey

#5548 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.