"utf8" is a thing. "utf" is not.
On Thu, Oct 4, 2018 at 12:58 PM, Philipp Kunz <philipp.k...@paratix.ch> wrote: > Thanks to Sherman's hint, I revised the test to use the terms unicode > character and utf encoding consistently and not utf character. Affects > only the test, mostly in comments and a few identifiers. > > Philipp > > > On Wed, 2018-10-03 at 12:48 -0700, Xueming Shen wrote: >> +1 >> >> only nit is it might be preferred to use "utf8" directly in the test >> instead of current "utf" >> >> -sherman >> >> On 10/03/2018 11:55 AM, Philipp Kunz wrote: >> > In short, bug 6202130 is about removal of comments in Attributes >> > like >> > >> > > XXX Need to handle UTF8 values and break up lines longer than 72 >> > >> > The patch here contains a test that shows that utf characters are >> > handled correctly, which has been the case before. Breaking lines >> > has >> > also been working before. >> > >> > In the main code, the patch only removes a few comments and does >> > not >> > change actual code. It all has been working before and now we know. >> > >> > Philipp >> > >> > >> > https://bugs.openjdk.java.net/browse/JDK-6202130 >> > >> > >> > >> > >> > On Wed, 2018-09-26 at 07:08 +0200, Philipp Kunz wrote: >> > > Hi, >> > > >> > > There are some comments in the source code like this: >> > > >> > > > XXX Need to handle UTF8 values and break up lines longer >> > > than 72 bytes >> > > >> > > in Attributes.java on lines 299, 327, and 370. Bug 6202130 also >> > > suggests >> > > to add a test to show that the character set is fully supported >> > > before >> > > actually removing those comments. >> > > >> > > Hence, I added such a test, see ValueUtfEncoding in attached >> > > patch, and >> > > it finds the complete utf character set supported correctly and >> > > that it >> > > has been so already before. >> > > >> > > The comments in Attributes also say that lines longer than 72 >> > > bytes need >> > > to be broken. That is a different issue and has been already >> > > addressed >> > > and solved in bug 6372077 and is now tested in LineBreakLineWidth >> > > and >> > > therefore, that line break part of the comment is obsolete now as >> > > well >> > > with the exception of version headers ("Manifest-Version" or >> > > "Signature-Version") at least when taking strictly. >> > > >> > > Version headers are not subject of bug 6202130 for which my patch >> > > intends to provide a fix and if only because multi-byte >> > > characters are >> > > not allowed in version headers and wasn't subject of 6372077 >> > > either >> > > which was about too short lines rather than too long ones. If I >> > > would >> > > change only the minimum I could argue was safe, there would have >> > > to >> > > remain a comment in Attributes.java on line 327 like >> > > >> > > > XXX Need to handle version headers longer than 72 bytes >> > > >> > > but I guess that should probably better be represented as another >> > > bug >> > > rather than this style of comment if at all desired. There are >> > > some bugs >> > > concerning version headers and particularly the behavior of the >> > > manifests in their absence, among possibly others, 6910466, >> > > 4935610, >> > > 4271239, 8196371 and 4256580 which have some relation but I >> > > haven't >> > > found one that really fits the 72 byte line length limit of >> > > version >> > > headers. Whether such a bug is created or found or not, it does >> > > not >> > > directly relate to the one I'm trying to fix here, 6202130. >> > > >> > > If also the test is good I would assume that the comments can be >> > > removed now without actually changing the Manifest and Attributes >> > > implementation. >> > > >> > > Regards, >> > > Philipp >>