-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Further improve CJK and encoded URL support #61
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I was thinking about this while working on b5d9e28
Can you check if it fixed it? More test cases are definitely welcome so that regressions are caught. Especially for searches like:
By the way, you don't have to force-push, it makes it difficult to see what changed between reviews |
sorry, out of habit. won't do it next time. |
This reverts commit 17b2826.
see: refined-github#61 (comment) Co-authored-by: Federico <[email protected]>
didn't notice safeDecode was only used in isCustomLink
This seems to be unfortunately correct, however I'm not a fan of loading that library for such a rare occurrence. However I think we have an alternative:
I'll try this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good now
searchParams, | ||
hash, | ||
} = url; | ||
const {search, searchParams} = url; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part, together with the change in applyToLink
, should resolve the problem at the root: shortenRepoUrl()
now works directly on what the user sees, not the result of new URL
I'll release this as a breaking version
punycode.js
to parse punycode encoded path elements fromURL
constructorstacked on top of test: catch invalid cases #60shorten-links
encodes/mangles CJK/UTF-8 characters refined-github#7744 (comment)