Sorry, just a reminder. On 10/27/2015 9:02 PM, Alexander Stepanov wrote:
Hello Alexandr,> PrinterInfo.java: Should the <CODE> tag here also be changed to {@code } ? Here is again some mess about full / "minimal" fix. I still hope that the full version could be reviewed (replacing all these <code>s).> Could you separate the Swing part of the fixDone, the swing-related changes were reverted. Please find new patch / webrev.zip:http://cr.openjdk.java.net/~avstepan/8138838/noSwing/jdk.patch http://cr.openjdk.java.net/~avstepan/8138838/noSwing/webrev.zipTo ensure the automated changes have been done correctly, the following specdiff report was generated:http://cr.openjdk.java.net/~avstepan/8138838/noSwing/specdiff.tar.gz(only expected changes - 28 misprints were fixed); so hopefully there is no need to review patch line-by-line scrupulously.Thanks, Alexander On 10/27/2015 4:21 PM, Alexander Scherbatiy wrote:PrinterInfo.java: Should the <CODE> tag here also be changed to {@code } ?Could you separate the Swing part of the fix and send it to the swing-dev alias to the review?Thanks, Alexandr. On 10/16/2015 6:40 PM, Alexander Stepanov wrote:> Of cause I am not capable to review megabytes of changes in patch file.Yes, I see. I've overlooked it briefly, but yes, it was quite boring. > Probably it would be dangerous to do wrapping by a script.to control the correctness of the initial fix, the respective specdiff report was generated:http://cr.openjdk.java.net/~avstepan/8138838/specdiff.tar.gzI also think that the manual replacement is an endless task which should never be ended (and, correspondingly, will never be started).So could you please summarize:1. should I push the "minimum" part only? (after the mentioned fixes, of course)2. do these "<code> -> {@code }" replacements make sense at all (disregarding their largeness)? This is, probably, the main question, an I still didn't receive any clear answer (excepting, maybe, Martin's feedback) :)3. if the replacement is still desired (which is very doubtful) then should be the "overall" fix be split into some observable parts?Thanks, Alexander On 10/16/2015 5:37 PM, Semyon Sadetsky wrote:On 10/16/2015 1:42 PM, Alexander Stepanov wrote:I looked at the webrev only. Of cause I am not capable to review megabytes of changes in patch file. Probably it would be dangerous to do wrapping by a script. I would prefer the simplest automatic replacement as it can be to process such huge amount of code.// cutting off core-libs-dev Hello Semyon,> Since you are doing cosmetic changes, could please wrap the amended lines to 80 characters per line? > MenuComponent.java : @param d - the <code>Dimension</code>... - Should it also be replaced with brackets?> PrinterInfo.java - also <CODE> is used. Could you please specify which version are you reviewing? Minimal http://cr.openjdk.java.net/~avstepan/8138838/webrev.min.00/index.html or full http://cr.openjdk.java.net/~avstepan/8138838/jdk.patch ?If the minimal only, then of course I'll split the lines touched to make them not longer than 80 characters and will replace the "<code>" tags.Okay, then maybe it is worth to split those changes in two different requests: one is for manual corrections and another one for automatic?Otherwise the patches http://cr.openjdk.java.net/~avstepan/8138838/webrev.min.00/jdk.patch and http://cr.openjdk.java.net/~avstepan/8138838/jdk.patchwill be merged (the 2nd is mostly a subset of the 1st), and there wouldn't be any "<code>" in MenuComponent.java and PrinterInfo.java. In such a case (if you don't object) I'll do the line splitting in the "min" part only, not in all of these ~1000 files affected.One method doesn't have public modifier and another one does. It looks messy.> MultiResolutionImage.java interface has a mix of verbose/implicit method modifiers> It would be nice to reduce it to the uniform style. Sorry, I didn't catch this. Could you please explain?--SemyonThanks, Alexander On 10/15/2015 9:09 PM, Semyon Sadetsky wrote:Hi Alexander,Since you are doing cosmetic changes, could please wrap the amended lines to 80 characters per line?Also some notes:MultiResolutionImage.java interface has a mix of verbose/implicit method modifiers. It would be nice to reduce it to the uniform style. MenuComponent.java : @param d - the <code>Dimension</code>... - Should it also be replaced with brackets?PrinterInfo.java - also <CODE> is used. --Semyon On 10/14/2015 7:49 PM, Alexander Stepanov wrote:Sorry, just a reminder. If the activity is untimely, then could you please review the following minimum part of fix?http://cr.openjdk.java.net/~avstepan/8138838/webrev.min.00/index.html(some misprints + midget JDK-8138893 fixed). Thanks, Alexander On 10/5/2015 2:12 PM, Alexander Stepanov wrote:Hello, Could you please review the fix for https://bugs.openjdk.java.net/browse/JDK-8138838 Patch + webrev zipped + specdiff report: http://cr.openjdk.java.net/~avstepan/8138838Just some cosmetic changes for docs (<code>...</code> -> {@code ...} replacement) + some misprints fixed.Not sure if these changes are desired at all for now. Thanks, Alexander(Just in case, adding the prehistory and sending a copy to core-libs-dev).On 10/1/2015 2:31 PM, Alexander Stepanov wrote:Hello Martin, Stuart, Thank you for the notes,Yes, the initial utility is quite ugly, I just tried to prepare it as quickly as possible hoping that it covers the majority of "trivial" replace cases. Yes, it does not process multi-line <code> inclusions.> s = s.replace( "<CODE>", tag1); > s = s.replace( "<Code>", tag1); > s = s.replace("</CODE>", tag2); > s = s.replace("</Code>", tag2);- replaced with "s = ln.replaceAll("(?i)<code>", "<code>").replaceAll("(?i)</code>", "</code>");"Unfortunately my Perl/lisp knowledge are zero :)> Should you publish your specdiff? I guess not - it would be empty! For now it contains a single fixed misprint diff, but there are a few another misprints at the moment which I'd like to include in the patch as well.So if you don't have objections, I'll delay for a several days and then publish a final RFR (probably containing changes in some other repos like jaxws, corba or jaxp) which would be more formal (containing bug # and the final specdiff report).Thanks again, Alexander On 10/1/2015 9:54 AM, Martin Buchholz wrote:Hi s'marks, You probably don't need to absolutify paths. And you can easily handle multiple args. (just for fun!)Checks for javadoc comment; handles popular html entities; handles multiple lines; handles both tt and code:#!/bin/bash find "$@" -name '*.java' | \ xargs -r perl -p0777i -e \'do {} while s~^ *\*.*\K<(tt|code)>((?:[^<>{}\&\@]|&(?:lt|gt|amp);)*)</\1>~$_=$2; s/</</g; s/>/>/g; s/&/&/g; "{\@code $_}"~mgie'On Wed, Sep 30, 2015 at 6:16 PM, Stuart Marks <stuart.ma...@oracle.com <mailto:stuart.ma...@oracle.com>> wrote:Hi Alexander, Martin,The challenge of Perl file slurping and Emacs one-liners was toomuch to bear.This is Java, so one-liners are hardly possible. Still, there are a bunch of improvements that can be made to the Java version. (OK,and I'm showing off a bit.) Take a look at this:http://cr.openjdk.java.net/~smarks/misc/SimpleTagEditorSmarks1.java <http://cr.openjdk.java.net/%7Esmarks/misc/SimpleTagEditorSmarks1.java>I haven't studied the output exhaustively, but it seems to do a reasonably good job for the common cases. I ran it over java.lang and I noticed a few cases where there is markup embedded within<code></code> text, which should be looked at more closely.I don't particularly care if you use my version, but there are some techniques that I'd strongly recommend that you considerusing in any such tool. In particular: - Pattern.DOTALL to do multi-line matches - Pattern.CASE_INSENSITIVE- try-with-resources to ensure that files are closed properly - NIO instead of old java.io <http://java.io> APIs, particularlyFiles.walk() and streams - use Scanner to deal with input file buffering - Scanner's stream support (I recently added this to JDK 9) Enjoy, s'marks On 9/29/15 2:23 PM, Martin Buchholz wrote: Hi Alexander,your change looks good. It's OK to have manual correctionsfor automatedmega-changes like this, as long as they all revert changes.Random comments:Should you publish your specdiff? I guess not - it would beempty! while((s = br.readLine()) != null) {by matching only one line at a time, you lose the ability to make replacements that span lines. Perlers like to "slurp" in theentire file as a single string. s = s.replace( "<CODE>", tag1); s = s.replace( "<Code>", tag1); s = s.replace("</CODE>", tag2); s = s.replace("</Code>", tag2); Why not use case-insensitive regex? Here's an emacs-lisp one-liner I've been known to use: (defun tt-code () (interactive) (query-replace-regexp "<\\(tt\\|code\\)>\\([^&<>\\\\]+\\)</\\1>" "{@code \\2}"))With more work, one can automate transformation of embeddedthings like <But of course, it's not even possible to transform ALL uses of<code> to{@code, if there was imaginative use of nested html tags.On Tue, Sep 29, 2015 at 3:21 AM, Alexander Stepanov < alexander.v.stepa...@oracle.com <mailto:alexander.v.stepa...@oracle.com>> wrote:Updated: a few manual corrections were made (as @linkplaintags displays nested {@code } literally):http://cr.openjdk.java.net/~avstepan/tmp/codeTags/jdk.patch <http://cr.openjdk.java.net/%7Eavstepan/tmp/codeTags/jdk.patch> -checked with specdiff (which of course does not coverdocumentation for internal packages), no unexpected diffs detected. Regards, Alexander On 9/27/2015 4:52 PM, Alexander Stepanov wrote: Hello Martin,Here is some simple app. to replace <code></code> tagswith a new-style{@code } one (which is definitely not so elegant asthe Perl one-liners):http://cr.openjdk.java.net/~avstepan/tmp/codeTags/SimpleTagEditor.java <http://cr.openjdk.java.net/%7Eavstepan/tmp/codeTags/SimpleTagEditor.java>Corresponding patch for jdk and replacement log (~62kof the tag changes): http://cr.openjdk.java.net/~avstepan/tmp/codeTags/jdk.patch <http://cr.openjdk.java.net/%7Eavstepan/tmp/codeTags/jdk.patch> http://cr.openjdk.java.net/~avstepan/tmp/codeTags/replace.log<http://cr.openjdk.java.net/%7Eavstepan/tmp/codeTags/replace.log> (sorry, I have to check the correctness of the patchwith specdiff yet, so this is rather demo at the moment).Don't know if these changes (cosmetic by nature) aredesired for now ornot. Moreover, probably some part of them should go toanother repos (e.g., awt, swing -> "client" instead of "dev"). Regards, Alexander
