Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use quri instead of puri for URIs. #65

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Use quri instead of puri for URIs. #65

wants to merge 5 commits into from

Conversation

tmccombs
Copy link
Contributor

Second attempt at this. Fixed several bugs from the first attempt at
using quri.

@tmccombs
Use quri instead of puri for URIs.
81cfd2f 
Second attempt at this. Fixed several bugs from the first attempt at
using quri.
@tmccombs
Copy link
Contributor Author

Hunchentoot successfully compiles with this. If you still think I should build a compatibility layer, let me know.

@hanshuebner
Copy link
Member

The issue is not so much Hunchentoot, but existing libraries and
applications that try to pass PURI:URI instances as argument or that rely
on a PURI:URI instance to be returned from HTTP-REQUEST. The former is
relatively easy to fix, but fixing the latter would mean that that by
default, a PURI:URI would be returned and only if the user specifically
requests it - maybe by the way of setting a special variable or feature or
keyword argument, a QURI:URI would be returned instead.

Zach Beane ran the Quicklisp dist creation process with the previous
version of your change (
http://report.quicklisp.org/2015-07-10/failure-report.html) and found that
to cause many errors because many libraries depended on DRAKMA pulling PURI
as a dependency. I have not investigated how these failing libraries use
PURI, but it is probably safe to say that they'd need one of the two
suggested compatibility changes above to continue working.

-Hans

2015-07-11 23:14 GMT+02:00 Thayne McCombs [email protected]:

Hunchentoot successfully compiles with this. If you still think I should
build a compatibility layer, let me know.

—
Reply to this email directly or view it on GitHub
#65 (comment).

@tmccombs
Copy link
Contributor Author

Ok, I'll add the compatibility. Do you know which libraries require puri support?

@hanshuebner
Copy link
Member

2015-07-12 16:49 GMT+02:00 Thayne McCombs [email protected]:

Ok, I'll add the compatibility. Do you know which libraries require puri
support?

I only have the error output that Xach sent. I would propose that DRAKMA
returns a PURI:URI unless the caller explicitly requests a QURI:URI.

@tmccombs
Some minor improvements.
075fe86 
Made the following changes:
 - use QURI:MAKE-URI instead of QURI.URI:MAKE-URI. The former is in the
 main namespace, and will instantiate the proper class, rather than just
 using the generic URI class.
 - Just store the uri parameter in unparsed-uri. We only ever use
 UNPARSED-URI if it is a string, and we don't mutate it so there is no
 need to copy it.
@tmccombs
Allow passing a PURI:URI as the uri to HTTP-REQUEST
92b5c8b
@tmccombs
Make returned URI backwards compatible.
f1feee7 
Add a special variable to control whether HTTP-REQUEST returns
a PURI:URI or a QURI:URI. By default returns a PURI:URI for backwards compatibility.
@tmccombs
Replace dissect-query with quri:uri-query-params.
c6a301e 
I would also replace alist-to-url-encoded-string, but quri doesn't have
a way to pass a custom url encoder. I don't know if anyone actually uses
that feature though.
@vityok vityok mentioned this pull request Feb 14, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tmccombs @hanshuebner