jdk icon indicating copy to clipboard operation
jdk copied to clipboard

Published 20 hours ago • verified publisher icon openjdk

8285306: Fix typos in java.desktop

Open magicus opened this issue 3 years ago • 10 comments
trafficstars

I ran codespell on the src/java.desktop directory, and accepted those changes where it indeed discovered real typos.

I ignored typos in public methods and variables. Maybe they can be fixed later on without much fanfare, if they are in internal classes. Typos in exposed APIs are likely here to stay.

I will update copyright years using a script before pushing (otherwise like every second change would be a copyright update, making reviewing much harder).

The long term goal here is to make tooling support for running codespell. The trouble with automating this is of course all false positives. But before even trying to solve that issue, all true positives must be fixed. Hence this PR.

Progress

  • [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • [x] Change must not contain extraneous whitespace
  • [x] Commit message must refer to an issue

Issue

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/8328/head:pull/8328
$ git checkout pull/8328

Update a local copy of the PR:
$ git checkout pull/8328
$ git pull https://git.openjdk.org/jdk pull/8328/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 8328

View PR using the GUI difftool:
$ git pr show -t 8328

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/8328.diff

magicus avatar Apr 21 '22 08:04 magicus

:wave: Welcome back ihse! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Apr 21 '22 08:04 bridgekeeper[bot]

@magicus The following labels will be automatically applied to this pull request:

  • client
  • i18n

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

openjdk[bot] avatar Apr 21 '22 08:04 openjdk[bot]

Webrevs

mlbridge[bot] avatar Apr 21 '22 08:04 mlbridge[bot] 

 static class AquaHierarchyButtonListener implements HierarchyListener {

  •    // Everytime a hierarchy is change we need to check if the button if moved on or from

  •    // Every time a hierarchy is change we need to check if the button if moved on or from

change -> changed if moved -> is moved

src/java.desktop/macosx/classes/sun/lwawt/macosx/CEmbeddedFrame.java

  •        // it won't be invoced if focuse is moved to a html element

  •        // it won't be invoced if focus is moved to a html element

invoced -> invoked "a html" -> "an html"

src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_MidiOut.c

  • // $$fb 2002-04-04: It is responsibility of the application developer to

"is the"

src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_MidiOut.c

  • // $$fb 2002-04-04: It is responsibility of the application developer to

same as above

Regarding changes in gif + freetype diff --git a/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c b/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c

diff --git a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c

Please exclude ALL 3rd party libraries from this PR.

prrace avatar Apr 27 '22 03:04 prrace

@aivanov-jdk While I approve of finding and fixing spelling mistakes, grammatical or semantic errors, the mistakes you posted suggestions for was not at all associated with the mechanical changes made by codespell that this PR included, and I assume you noticed them just because you read the text around the typo my PR fixed.

In hindsight, it would have been better if you've made these changes yourself, in a separate PR.

But since you had spent the effort of looking for these problems, and suggesting fixes, I have now accepted all of them. Due to how the Github UI works, this was quite tedious, so please refrain from submitting more than a handful suggestions next time you review a PR.

magicus avatar May 13 '22 16:05 magicus

Thank you, @magicus, I should've noted the changes and created a branch on top of yours.

That's right. Even looking through all the changes in this PR was tedious, more than hundred files doesn't work well.

aivanov-jdk avatar May 13 '22 18:05 aivanov-jdk

Please exclude ALL 3rd party libraries from this PR.

Are these the only files to be excluded?

src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c

src/java.desktop/share/native/libfreetype/src/autofit/afcjk.c
src/java.desktop/share/native/libfreetype/src/autofit/aflatin.c
src/java.desktop/share/native/libfreetype/src/base/ftsynth.c
src/java.desktop/share/native/libfreetype/src/raster/ftraster.c

aivanov-jdk avatar May 13 '22 19:05 aivanov-jdk

@magicus This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Jun 11 '22 01:06 bridgekeeper[bot]

Thank you bot. I'm on it!

magicus avatar Jun 11 '22 09:06 magicus

@magicus This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Aug 15 '22 09:08 bridgekeeper[bot]

@magicus this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout typos-in-java.desktop
git fetch https://git.openjdk.org/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

openjdk[bot] avatar Sep 06 '22 12:09 openjdk[bot]

I have now removed the changes to libfreetype, and also libjpeg. These were the only 3rd party source I could locate.

I apologize for this PR. It was too huge. If it is of any comfort, I've learnt my lesson and won't repeat it. And once it started getting pushed down by more prioritized work, I got a bit scared about going back and look through all this for additional (unspecified) 3rd party sources, so it got left behind, hanging around.

@prrace I hope this is acceptable to merge now.

magicus avatar Sep 06 '22 12:09 magicus

/contributor add turbanoff

magicus avatar Sep 06 '22 14:09 magicus

@magicus turbanoff was not found in the census.

Syntax: /contributor (add|remove) [@user | openjdk-user | Full Name <email@address>]. For example:

  • /contributor add @openjdk-bot
  • /contributor add duke
  • /contributor add J. Duke <[email protected]>

User names can only be used for users in the census associated with this repository. For other contributors you need to supply the full name and email address.

openjdk[bot] avatar Sep 06 '22 14:09 openjdk[bot]

/contributor add @turbanoff

magicus avatar Sep 06 '22 14:09 magicus

@magicus Contributor Andrey Turbanov <[email protected]> successfully added.

openjdk[bot] avatar Sep 06 '22 14:09 openjdk[bot]

Is src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c a third-party code?

I don't think so. I find no comments in the source code, or any kind of indication on this being so. If you say it is, I can revert the changes in that file.

magicus avatar Sep 13 '22 21:09 magicus

I see now that Phil cryptically said:

Regarding changes in gif + freetype diff --git a/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c b/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c

diff --git a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c

Please exclude ALL 3rd party libraries from this PR.

That might be interpreted as stating that gifdecoder.c is 3rd party source code (although that was by no means clear to me the first time I read it). I'll revert the changes in that file, and also src/java.desktop/share/native/libfontmanager/freetypeScaler.c.

magicus avatar Sep 13 '22 21:09 magicus

I see now that Phil cryptically said:

Regarding changes in gif + freetype diff --git a/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c b/src/java.desktop/share/native/libawt/awt/image/gif/gifdecoder.c

diff --git a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c

Please exclude ALL 3rd party libraries from this PR.

That might be interpreted as stating that gifdecoder.c is 3rd party source code (although that was by no means clear to me the first time I read it). I'll revert the changes in that file, and also src/java.desktop/share/native/libfontmanager/freetypeScaler.c.

I don't know why I mentioned those two files like that but those particular two are JDK code so are fair game.

I did write "Please exclude ALL 3rd party libraries from this PR." and later : "We need to revert the native code changes to "libfreetype". and those points are correct.

You did fix the latter, but there are still some 3rd party files in there that are edited : glext.h, wsutils.h, multiVis.c

prrace avatar Sep 13 '22 22:09 prrace

@prrace I have now reverted the changes to glext.h, wsutils.h and multiVis.c. Is this finally ready for merging?

(Going forward, I think we absolutely need to have some way to document in the code tree that certain files is 3rd party, like the UPSTREAM notation I previously suggested, or some variant thereof. This "tribal knowledge" about what is 3rd party is not beneficial to anyone, and only wastes both your and my time...)

magicus avatar Sep 21 '22 11:09 magicus

Everything looks fine, and you can freely ignore my suggestions. They are just thoughts. And yeah we should probably earmark third party files to avoid "tribal knowledge".

SWinxy avatar Sep 22 '22 01:09 SWinxy

/integrate

magicus avatar Sep 22 '22 17:09 magicus

@magicus This pull request has not yet been marked as ready for integration.

openjdk[bot] avatar Sep 22 '22 17:09 openjdk[bot]

@prrace Do you think you can approve this now, so we can finally close it? (I promise I won't open huge PRs like this in the future; lesson well learnt.)

magicus avatar Oct 12 '22 11:10 magicus

@magicus This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8285306: Fix typos in java.desktop

Co-authored-by: Andrey Turbanov <[email protected]>
Reviewed-by: aturbanov, prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 89 new commits pushed to the master branch:

  • 5dbd49511518819acbbff9968cdf426af759cf2c: 8295457: Make the signatures of write barrier methods consistent
  • 7b2e83b3955c034208325ea5477afd3c5e1da41a: 8295469: S390X: Optimized builds are broken
  • 63867c4b52a331f8d77f2c32dc8053c0f990dfc6: 8295433: EpsilonHeap doesn't need to override post_initialize()
  • e7375f9c527fd86dc1414a308a440903fb9f22da: 8295468: RISC-V: Minimal builds are broken
  • bd41428a5602e59034e61bf34eed686d4c7b052a: 8293590: Some syntax checks performed by URL.openConnection() could be performed earlier, at URL construction
  • 78fed9d3074ecfde5dfcd31f433ba104ad059d21: 7175397: The divider color is not changed to green when dragging for Nimbus LaF.
  • 8c40b7dc8cd7b6a6d0c9349b991e0e01b69349c3: 8292177: InitialSecurityProperty JFR event
  • e7a964b4dbbdd21eba87dc94eb3680e9553f5039: 8295268: Optimized builds are broken due to incorrect assert_is_rfp shortcuts
  • 0b7d811c98cb45a822b7ef56e5ee99d1b5483e78: 8294730: Add @throws and @implNote clauses to BigInteger::isProblablePrime and BigInteger::nextProblablePrime
  • 4434cbb719db37880b48474ba342f300ed4828a8: 8295264: Fix PaX check on RISC-V
  • ... and 79 more: https://git.openjdk.org/jdk/compare/bdb4ed0fb136e9e5391cfa520048de6b7f83067d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk[bot] avatar Oct 18 '22 16:10 openjdk[bot]

/integrate

magicus avatar Oct 18 '22 17:10 magicus

Going to push as commit 2a799e5c82395919b807561da4a062e0fe6da31d. Since your change was applied there have been 90 commits pushed to the master branch:

  • d1f794587cbac221649114b71f2fce5e1f8b7e49: 6924219: (fc spec) FileChannel.write(ByteBuffer, position) behavior when file opened for append not specified
  • 5dbd49511518819acbbff9968cdf426af759cf2c: 8295457: Make the signatures of write barrier methods consistent
  • 7b2e83b3955c034208325ea5477afd3c5e1da41a: 8295469: S390X: Optimized builds are broken
  • 63867c4b52a331f8d77f2c32dc8053c0f990dfc6: 8295433: EpsilonHeap doesn't need to override post_initialize()
  • e7375f9c527fd86dc1414a308a440903fb9f22da: 8295468: RISC-V: Minimal builds are broken
  • bd41428a5602e59034e61bf34eed686d4c7b052a: 8293590: Some syntax checks performed by URL.openConnection() could be performed earlier, at URL construction
  • 78fed9d3074ecfde5dfcd31f433ba104ad059d21: 7175397: The divider color is not changed to green when dragging for Nimbus LaF.
  • 8c40b7dc8cd7b6a6d0c9349b991e0e01b69349c3: 8292177: InitialSecurityProperty JFR event
  • e7a964b4dbbdd21eba87dc94eb3680e9553f5039: 8295268: Optimized builds are broken due to incorrect assert_is_rfp shortcuts
  • 0b7d811c98cb45a822b7ef56e5ee99d1b5483e78: 8294730: Add @throws and @implNote clauses to BigInteger::isProblablePrime and BigInteger::nextProblablePrime
  • ... and 80 more: https://git.openjdk.org/jdk/compare/bdb4ed0fb136e9e5391cfa520048de6b7f83067d...master

Your commit was automatically rebased without conflicts.

openjdk[bot] avatar Oct 18 '22 17:10 openjdk[bot]

@magicus Pushed as commit 2a799e5c82395919b807561da4a062e0fe6da31d.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

openjdk[bot] avatar Oct 18 '22 17:10 openjdk[bot]