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

Fix url interpretation #1259

Merged
merged 2 commits into from Dec 17, 2016
Merged

Fix url interpretation #1259

merged 2 commits into from Dec 17, 2016

Conversation

ppot
Copy link
Contributor

@ppot ppot commented Dec 17, 2016

Previous url-command.js used domainRegex
export const domainRegex = /([0-9]+[:.]*)+|([a-zA-Z0-9.]+[.][a-zA-Z0-9]+([:][0-9]+)*){1}/

    if (domainRegex.test(domain)) {
      const result = path.match(domainRegex)[0];
      return `http://${result}`;
    }

The forced http made command starting with 3 open in a <webview>

Exemple of supported url:

http://www.hyper.is
https://www.hyper.is
http://hyper.is/over/there?name=ferre
http://hyper.is/hello
http://www.q.hyper.is/hello
http://hyper.qc.ca/hello
http://localhost:1337/
http://hyper:1337/load?now=serve
http://localhost:1337/#/now
http://192.143.134/over/ok?there=hello
https://192.143.134:80/

Close : #1042 , #910, #594, #97

@rauchg rauchg merged commit b2f67c7 into vercel:master Dec 17, 2016
@rauchg
Copy link
Member

rauchg commented Dec 17, 2016

Thank you @ppot. Looks beautiful

Copy link
Contributor

@albinekb albinekb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the name isUrl id expect it to return a bool.. otherwise 👌

@ppot
Copy link
Contributor Author

ppot commented Dec 21, 2016

@albinekb isUrl is better interpreted in that case. since we test the url. But maybe matchURL would be a better word?

@ppot ppot mentioned this pull request Dec 31, 2016
2 tasks
tylerong pushed a commit to tylerong/hyper that referenced this pull request Jul 3, 2017
* Fix url interpretation

* update regex
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

Successfully merging this pull request may close these issues.

None yet

3 participants