On Thu, 22 Oct 2020 15:55:30 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> java.util.HexFormat utility: >> >> - Format and parse hexadecimal strings, with parameters for delimiter, >> prefix, suffix and upper/lowercase >> - Static factories and builder methods to create HexFormat copies with >> modified parameters. >> - Consistent naming of methods for conversion of byte arrays to formatted >> strings and back: formatHex and parseHex >> - Consistent naming of methods for conversion of primitive types: >> toHexDigits... and fromHexDigits... >> - Prefix and suffixes now apply to each formatted value, not the string as >> a whole >> - Using java.util.Appendable as a target for buffered conversions so output >> to Writers and PrintStreams >> like System.out are supported in addition to StringBuilder. (IOExceptions >> are converted to unchecked exceptions) >> - Immutable and thread safe, a "value-based" class >> >> See the [HexFormat >> javadoc](http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html) >> for details. >> >> Review comments and suggestions welcome. > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Review comment updates to class javadoc Do you want to refactor some of the classes where `HexFormat` would be a well-suited replacement for the current code, and what do you think about my other previous review comments (mostly formatting related)? The bridgekeeper bot was so kind and censored my [previous comment](https://github.com/openjdk/jdk/pull/482#issuecomment-710709003), but apparently it can't handle review comments yet ... And no, I am not going to sign the [OpenJDK Terms of Use](https://openjdk.java.net/legal/tou/terms) for a simple pull request comment, especially when the Terms of Use document is quite long and contains the following: > You hereby grant to Oracle and all Users a royalty-free, perpetual, > irrevocable, worldwide, non-exclusive and fully sub-licensable right and > license under Your intellectual property rights to reproduce, modify, adapt, > publish, translate, create derivative works from, distribute, perform, > display and use Your Submissions (in whole or part) and to incorporate or > implement them in other works in any form, media, or technology now known or > later developed. This includes, without limitation, the right to incorporate > or implement the Submission into any product or service, and to display, > market, sublicense and distribute the Submissions as incorporated or embedded > in any product or service distributed or offered by Oracle without > compensation to you. (Might be good to rework that to actually encourage contributions. I am kind of surprised that any company is willing to contribute to the OpenJDK under these terms.) Sorry for driving this pull request off-topic, I will stop all interaction here and will not bother you any further here if that is desired. Below is the original comment: ----- It might also be good to clarify in the documentation that `parseHex(...)` is case insensitive. This is currently not obvious and one might expect that it respects `isUpperCase()`. Additionally there are quite a lot places within the JDK code where this new class could be used. I have picked some where it would definitely be an improvement (but omitted test classes): <details> <summary>Classes</summary> <ul> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/java/net/HostPortrange.java#L94";><code>java.base</code> <code>java.net.HostPortrange</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/jdk/internal/module/ModuleHashes.java#L215";><code>java.base</code> <code>jdk.internal.module.ModuleHashes</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java#L544";><code>java.base</code> <code>sun.net.www.protocol.http.DigestAuthentication</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/AbstractDrbg.java#L738";><code>java.base</code> <code>sun.security.provider.AbstractDrbg</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/provider/certpath/ResponderId.java#L269";><code>java.base</code> <code>sun.security.provider.certpath.ResponderId</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/ssl/Utilities.java#L39";><code>java.base</code> <code>sun.security.ssl.Utilities</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/Debug.java#L323";><code>java.base</code> <code>sun.security.util.Debug</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/HexDumpEncoder.java";><code>java.base</code> <code>sun.security.util.HexDumpEncoder</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/ManifestEntryVerifier.java#L244";><code>java.base</code> <code>sun.security.util.ManifestEntryVerifier</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/util/SignatureFileVerifier.java#L753";><code>java.base</code> <code>sun.security.util.SignatureFileVerifier</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L742";><code>java.base</code> <code>sun.security.x509.AVA</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L811";><code>java.base</code> <code>sun.security.x509.AVA</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L889";><code>java.base</code> <code>sun.security.x509.AVA</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L967";><code>java.base</code> <code>sun.security.x509.AVA</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/AVA.java#L1111";><code>java.base</code> <code>sun.security.x509.AVA</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.base/share/classes/sun/security/x509/X509CertImpl.java#L1982";><code>java.base</code> <code>sun.security.x509.X509CertImpl</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.naming/share/classes/com/sun/jndi/ldap/LdapName.java#L837";><code>java.naming</code> <code>com.sun.jndi.ldap.LdapName</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.naming/share/classes/javax/naming/ldap/Rdn.java#L568";><code>java.naming</code> <code>javax.naming.ldap.Rdn</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.security.jgss/share/classes/sun/security/krb5/KrbApReq.java#L314";><code>java.security.jgss</code> <code>sun.security.krb5.KrbApReq</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.security.jgss/share/classes/sun/security/krb5/internal/ktab/KeyTabEntry.java#L80";><code>java.security.jgss</code> <code>sun.security.krb5.internal.ktab.KeyTabEntry</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/java.smartcardio/share/classes/sun/security/smartcardio/PCSC.java#L174";><code>java.smartcardio</code> <code>sun.security.smartcardio.PCSC</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.compiler/share/classes/com/sun/tools/javac/file/BaseFileManager.java#L355";><code>jdk.compiler</code> <code>com.sun.tools.javac.file.BaseFileManager</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.compiler/share/classes/com/sun/tools/javac/main/Main.java#L490";><code>jdk.compiler</code> <code>com.sun.tools.javac.main.Main</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java#L158";><code>jdk.crypto.cryptoki</code> <code>sun.security.pkcs11.P11Util</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/Functions.java#L168";><code>jdk.crypto.cryptoki</code> <code>sun.security.pkcs11.wrapper.Functions</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.internal.vm.compiler/share/classes/org.graalvm.compiler.java/src/org/graalvm/compiler/java/LambdaUtils.java#L129";><code>jdk.internal.vm.compiler</code> <code>org.graalvm.compiler</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jartool/share/classes/sun/tools/jar/Main.java#L2011";><code>jdk.jartool</code> <code>sun.tools.jar.Main</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/AboutDialog.java#L83";><code>jdk.jconsole</code> <code>sun.tools.jconsole.AboutDialog</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/ConnectDialog.java#L287";><code>jdk.jconsole</code> <code>sun.tools.jconsole.ConnectDialog</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/ConnectDialog.java#L595";><code>jdk.jconsole</code> <code>sun.tools.jconsole.ConnectDialog</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/HTMLPane.java#L61";><code>jdk.jconsole</code> <code>sun.tools.jconsole.HTMLPane</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jconsole/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java#L79";><code>jdk.jconsole</code> <code>sun.tools.jconsole.inspector.XArrayDataViewer</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java#L1137";><code>jdk.jdeps</code> <code>com.sun.tools.javap.AttributeWriter</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java#L149";><code>jdk.jdeps</code> <code>com.sun.tools.javap.ClassWriter</code></li> <li><a href="https://github.com/openjdk/jdk/blob/83ea863122e1d58527c8bf3cd178acf4d0c54f30/src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java#L417";><code>jdk.jlink</code> <code>jdk.tools.jmod.JmodTask</code></li> </ul> ------------- PR: https://git.openjdk.java.net/jdk/pull/482
