<AWT Dev> [9] RFR 8138838: docs cleanup for java.desktop
Alexander Stepanov
alexander.v.stepanov at oracle.com
Tue Oct 27 18:02:00 UTC 2015
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 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