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

changed href computation (fix #66) #68

Closed
wants to merge 1 commit into from
Closed

changed href computation (fix #66) #68

wants to merge 1 commit into from

Conversation

schochastics
Copy link
Member

@chainsawriot can I get your opinion again?
What is your take on the failing test?

══ Failed tests ════════════════════════════════════════════════════════════════
── Failure ('test-get.R:3:5'): all get functions work ──────────────────────────
ada_get_href(url) (`actual`) not equal to "https://user_1:[email protected]:8080/api?q=1#frag" (`expected`).

`actual`:   "https://user_1:[email protected]:8080/dir/../api?q=1#frag"
`expected`: "https://user_1:[email protected]:8080/api?q=1#frag"

@chainsawriot
Copy link
Collaborator

@schochastics I think we will need to do it with the silent convert approach (sorry that I forgot the crazy case of ".../dir/"). I will send a PR later.

chainsawriot added a commit that referenced this pull request Jan 31, 2024
@chainsawriot
Use the silent conversion approach fix #66
94fa964 
See #68 and #67

This passes the basic tests.
@schochastics
Copy link
Member Author

thanks, urls keep being "interesting"

chainsawriot added a commit that referenced this pull request Jan 31, 2024
@chainsawriot @schochastics
Use the silent conversion approach fix #66 (#69)
43284da 
* added to_unicode parameter

* added test

* Use the silent conversion approach fix #66

See #68 and #67

This passes the basic tests.

* A better test suite for #66

* More tests

* Head hygiene and default parameter

* Bürokratie

---------

Co-authored-by: schochastics <[email protected]>
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
@schochastics @chainsawriot