Skip to content
This repository was archived by the owner on Sep 18, 2021. It is now read-only.

Some cctld to be linkified #63

Merged
merged 3 commits into from
Jul 15, 2014
Merged

Some cctld to be linkified #63

merged 3 commits into from
Jul 15, 2014

Conversation

jugyo
Copy link

@jugyo jugyo commented Jul 7, 2014

No description provided.

@@ -94,15 +94,16 @@
"台灣|新加坡|香港|한국" +
")(?=[^\\p{Alnum}@]|$))";
private static final String URL_PUNYCODE = "(?:xn--[0-9a-z]+)";
private static final String SPECIAL_URL_VALID_CCTLD = "(?:(?:" + "co|tv" + ")(?=[^\\p{Alnum}@]|$))";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we avoid string concatenation here and just use the final "(?:(?:co|tv)(?=[^\\p{Alnum}@]|$))" string?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we can. I was a little worried when we will have many ccTLD in here in the future :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. You can put them in a separate constant right away then. It's up to you though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like the fact that it is split, easier to read :)

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, having many same string like ")(?=[^\\p{Alnum}@]|$))" isn't cool :P

@KL-7
Copy link
Contributor

KL-7 commented Jul 14, 2014

:shipit:

@jakl
Copy link
Contributor

jakl commented Jul 15, 2014

Make sure there are at least tickets in rb/js/objc/conformance
:shipit:

@jugyo
Copy link
Author

jugyo commented Jul 15, 2014

Yes, rb/js/objc are remaining.

jugyo pushed a commit that referenced this pull request Jul 15, 2014
@jugyo jugyo merged commit 5823901 into master Jul 15, 2014
@jugyo jugyo deleted the some_cctld_to_be_linkified branch July 15, 2014 01:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants