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

--replace doesn't urlencode values #321

Closed
gromnitsky opened this issue Aug 15, 2024 · 3 comments · Fixed by #327
Closed

--replace doesn't urlencode values #321

gromnitsky opened this issue Aug 15, 2024 · 3 comments · Fixed by #327
Labels
bug Something isn't working

Comments

@gromnitsky
Copy link

$ trurl 'example.com/?q=x#bar' --replace q='#x'
http://example.com/?q=#x#bar

I thought it would print http://example.com/?q=%23x#bar.

@emanuele6
Copy link
Collaborator

--force-replace and --trim query= have similar bugs:

$ trurl --force-replace a%=z 'https://example.com/'
trurl note: key 'a%' not in url, appending to query
https://example.com/?a%=z
$ trurl --trim query=a% 'https://example.com/?a%25'
https://example.com/?a%25
$ trurl --trim query=a%25 'https://example.com/?a%25'
https://example.com/

@bagder bagder added the bug Something isn't working label Aug 16, 2024
@jacobmealey
Copy link
Contributor

I propose we add : modifier to trim so you could do:

$ trurl --trim query=a% 'https://example.com/?a%25'
https://example.com/
$ trurl --trum query:=a%25  'https://example.com/?a%25'
https://example.com/

I'm not sure how we would handle it for replace. any ideas?

@emanuele6
Copy link
Collaborator

emanuele6 commented Aug 19, 2024

I'm not sure how we would handle it for replace. any ideas?

--replace/--force-replace can just work like --append query= that does not have the bug.

I propose we add : modifier to trim so you could do:

Not a fan, and that would not solve the underlying issue; anyway moving to a separate issue

bagder added a commit that referenced this issue Aug 26, 2024
Makes it work like --append query= already does.

- update man page
- update test cases

Fixes #321
bagder added a commit that referenced this issue Aug 27, 2024
Makes it work like --append query= already does.

- update man page
- update test cases

Fixes #321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants