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

Support search engine escaping of spaces (+) #215

Merged
merged 6 commits into from Oct 28, 2020

Conversation

@turboMaCk
Copy link
Member

@turboMaCk turboMaCk commented Oct 27, 2020

resolves #200

Interpret + in seaerch query string as space.

There is a limitation imposed by elm URL parser we can't access get the raw urls string even when using Url.Parser.Query.custom -- the string argument is already decoded. this means we can't tell if the + comes from url or was decoded from percent string (%2B). All + are replaced with space.

Showcase

updated-search

previous PR: #214

@turboMaCk turboMaCk changed the title Turbo ma ck/percent encode query Percent encode query Oct 27, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Oct 27, 2020

@github-actions
Copy link

@github-actions github-actions bot commented Oct 27, 2020

@turboMaCk
Copy link
Member Author

@turboMaCk turboMaCk commented Oct 27, 2020

@garbas Feel free to check last commit 95de1b7 but also be aware of the limitations

@turboMaCk turboMaCk requested a review from garbas Oct 27, 2020
@turboMaCk turboMaCk changed the title Percent encode query Support search engine escaping of spaces (+) Oct 27, 2020
src/Route.elm Outdated Show resolved Hide resolved
@turboMaCk turboMaCk force-pushed the turboMaCk/percent-encode-query branch from 95de1b7 to 0ef840e Oct 27, 2020
@github-actions
Copy link

@github-actions github-actions bot commented Oct 27, 2020

@garbas
Copy link
Collaborator

@garbas garbas commented Oct 27, 2020

This now works when we load the app with the url that has + in the query parameter of the query string.
But if we input + into the input box it gets replaced by space.

@turboMaCk
Copy link
Member Author

@turboMaCk turboMaCk commented Oct 28, 2020

yes exactly. Anyway I'm afraid there is no way around the 2nd problem because even with custom function from elm url parser the strings one work with are already decoded which makes it impossible to tell the which plus is plus and which one is space:/

@garbas
Copy link
Collaborator

@garbas garbas commented Oct 28, 2020

I guess we will have to resort to ports and implement our own wrapper around Url.Url where we would take values from window.location via ports into the account. That should allow us to distinguish between "space/+" and "+"

@turboMaCk
Copy link
Member Author

@turboMaCk turboMaCk commented Oct 28, 2020

I would first like to check where does the escaping happens. If it's in Browser.Navigation or within Url.Parser. If it's the later there would also be option to "patch this on just a parser level" by accessing the raw string. The only kernel function of Url parser seems to be binding to decode/encodeURIComponent. https://github.com/elm/url/blob/master/src/Elm/Kernel/Url. If we'll be able to get undecoded data from navigation object in elm we might consider either:

  1. replacing url Parser with implementation that gives us access to the raw string
  2. using navigation object as a workaround.

I'll let you know once I know more.

@github-actions
Copy link

@github-actions github-actions bot commented Oct 28, 2020

@turboMaCk
Copy link
Member Author

@turboMaCk turboMaCk commented Oct 28, 2020

@garbas See the last commit - it implements pure elm solution. In theory it would be possible to just wrap whole elm/url Parser and embed it into larger custom Parser structure to build new nice API in which it would be possible to define this function as part of Parser type. Anyway that would be a lot of code. Current approach is an attempt for pragmatic compromise - not a complete hack but not a full blown custom url parser library either. Let me know what you think.

garbas
garbas approved these changes Oct 28, 2020
Copy link
Collaborator

@garbas garbas left a comment

Works great. And I like the solution.

@garbas garbas merged commit e412085 into master Oct 28, 2020
1 check passed
@garbas garbas deleted the turboMaCk/percent-encode-query branch Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants