-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Bring back the JSPs #1018
Bring back the JSPs #1018
Conversation
@@ -145,48 +145,6 @@ public String transform(Integer source) { | |||
}, | |||
false) | |||
.setMain("toNFKD", "toNFKD", UnicodeProperty.STRING, "1.1")); | |||
|
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.
I find these very useful in practice. Is there a compelling reason to remove?
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.
Well, they are very slow (getSet involves computing the preimage of a function).
But what do you use these for, as opposed to Lowercase_Mapping, Uppercase_Mapping, and Titlecase_Mapping?
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.
I just checked (on the staging JSPs which do not yet have these changes), and both of the following contain only characters whose mappings changed in 16.0 (because we are lagging on the ICU version used in the right-hand side):
[:^Lowercase_Mapping=@toLowercase@:]
[:^Uppercase_Mapping=@toUppercase@:]
[:^Titlecase_Mapping=@toTitlecase@:]
contains one interesting difference, U+0345, because ICU skips past a leading U+0345 in titlecasing. But I think for all practical purposes, you can just use the mappings.
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.
Ok, makes sense.
@@ -20,7 +20,8 @@ th { text-align: left } | |||
UtfParameters utfParameters = new UtfParameters(queryString); | |||
|
|||
String propForValues = utfParameters.getParameter("a"); | |||
UnicodeJsp.showPropsTable(out, propForValues, "properties.jsp"); | |||
// TODO(egg): Cache this page. |
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.
Ok for now. I think just a static page with an explicit list of the properties (and if enumerated, the values) without links would be very helpful for the users, and not hard to generate at build time.
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.
Yes. And we should probably grab some information from IndexUnicodeProperties as well, e.g., which files they come from.
But not today.
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.
Agreed
BTW, I tried converting
At the expense of additional memory, it sped up the queries significantly. |
Four commits, deployed in three builds of the JSPs: