<AWT Dev> [9] RFR 8138838: docs cleanup for java.desktop

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Tue Oct 27 13:21:45 UTC 2015


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 at oracle.com <mailto:stuart.marks at 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 at oracle.com
>>>>>>>> <mailto:alexander.v.stepanov at 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
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the awt-dev mailing list