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.

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

Bug in quri:uri-equal #59

Closed
aadcg opened this issue Jun 9, 2022 · 5 comments
Closed

Bug in quri:uri-equal #59

aadcg opened this issue Jun 9, 2022 · 5 comments

Comments

@aadcg
Copy link
Collaborator

aadcg commented Jun 9, 2022

Notice the following amazing facts:

(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "")) -> T
(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer")) -> T
(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "/")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "/")) -> T
(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "/")) -> NIL
(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "/")) -> NIL
(quri:uri-equal (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer" :path "")
                (quri.uri.http:make-uri-https :host "nyxt.atlas.engineer")) -> NIL
@Ambrevar
Copy link
Collaborator

Ambrevar commented Jun 9, 2022

You got bitten by the oldest open bug: #44.

See #45 for a discussion.

@aadcg
Copy link
Collaborator Author

aadcg commented Jun 9, 2022

From my cursory look, #45 is precisely the fix I had in mind (ensuring that path is initialized with the empty string). I will get acquainted with the whole discussion.

@aadcg
Copy link
Collaborator Author

aadcg commented Jun 10, 2022

Update. Added the complete set of assertions to look out for.

@Ambrevar
Copy link
Collaborator

Can we close this?

@aadcg
Copy link
Collaborator Author

aadcg commented Jun 28, 2022

As soon as PR #60 gets merged :)

Ambrevar pushed a commit that referenced this issue Jul 5, 2022
@Ambrevar Ambrevar closed this as completed Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants