[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-awt-dev
Subject: Re: <AWT Dev> [9] RFR 8138838: docs cleanup for java.desktop
From: Alexander Stepanov <alexander.v.stepanov () oracle ! com>
Date: 2015-11-06 11:47:10
Message-ID: 563C933E.3080204 () oracle ! com
[Download RAW message or body]
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 fix
> Done, 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.zip
>
> To 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.gz
>>> I 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:
>>>>> // 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.
>>>> 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.
>>>>>
>>>>> Otherwise the patches
>>>>> http://cr.openjdk.java.net/~avstepan/8138838/webrev.min.00/jdk.patch
>>>>> and
>>>>> http://cr.openjdk.java.net/~avstepan/8138838/jdk.patch
>>>>> will 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.
>>>> Okay, then maybe it is worth to split those changes in two
>>>> different requests: one is for manual corrections and another one
>>>> for automatic?
>>>>>
>>>>> > 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?
>>>> One method doesn't have public modifier and another one does. It
>>>> looks messy.
>>>>
>>>> --Semyon
>>>>> Thanks,
>>>>> 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/8138838
>>>>>>>>
>>>>>>>> Just 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.marks@oracle.com <mailto:stuart.marks@oracle.com>>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Alexander, Martin,
>>>>>>>>>>
>>>>>>>>>> The challenge of Perl file slurping and Emacs one-liners
>>>>>>>>>> was too
>>>>>>>>>> much 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
>>>>>>>>>> consider
>>>>>>>>>> using 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,
>>>>>>>>>> particularly
>>>>>>>>>> Files.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
>>>>>>>>>> corrections
>>>>>>>>>> for automated
>>>>>>>>>> mega-changes like this, as long as they all revert
>>>>>>>>>> changes.
>>>>>>>>>>
>>>>>>>>>> Random comments:
>>>>>>>>>>
>>>>>>>>>> Should you publish your specdiff? I guess not - it
>>>>>>>>>> would be
>>>>>>>>>> empty!
>>>>>>>>>>
>>>>>>>>>> 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 the
>>>>>>>>>> entire 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
>>>>>>>>>> embedded
>>>>>>>>>> things 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.stepanov@oracle.com
>>>>>>>>>> <mailto:alexander.v.stepanov@oracle.com>> wrote:
>>>>>>>>>>
>>>>>>>>>> Updated: a few manual corrections were made (as
>>>>>>>>>> @linkplain
>>>>>>>>>> tags 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
>>>>>>>>>> cover
>>>>>>>>>> documentation 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> tags
>>>>>>>>>> with a new-style
>>>>>>>>>> {@code } one (which is definitely not so
>>>>>>>>>> elegant as
>>>>>>>>>> the 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 (~62k
>>>>>>>>>> of 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 patch
>>>>>>>>>> with specdiff yet,
>>>>>>>>>> so this is rather demo at the moment).
>>>>>>>>>>
>>>>>>>>>> Don't know if these changes (cosmetic by
>>>>>>>>>> nature) are
>>>>>>>>>> desired for now or
>>>>>>>>>> not. Moreover, probably some part of them
>>>>>>>>>> should go to
>>>>>>>>>> another repos (e.g.,
>>>>>>>>>> awt, swing -> "client" instead of "dev").
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Alexander
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic