On Thu, 21 Apr 2022 08:35:36 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
> 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.
Nearly 500 files are too many. Smaller chunks would be easier to review.
Some of the native code files could come from upstream libraries.
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 535:
> 533:
> 534: static class AquaHierarchyButtonListener implements
> HierarchyListener {
> 535: // Every time a hierarchy is change we need to check if the
> button if moved on or from
Suggestion:
// Every time a hierarchy is changed we need to check if the button is
moved on or from
Maybe even “_the_ hierarchy”.
And probably “_the_ toolbar” on the next line.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CEmbeddedFrame.java line 126:
> 124: // see bug 8010925
> 125: // we can't put this to handleWindowFocusEvent because
> 126: // it won't be invoced if focus is moved to a html element
Suggestion:
// it won't be invoked if focus is moved to an html element
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 552:
> 550: }
> 551:
> 552: // Orders window's children based on the current focus state
Suggestion:
// Orders window children based on the current focus state
I believe possessive is not necessary here, it's not used with inanimate
objects usually.
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 688:
> 686: }
> 687:
> 688: // Hides/shows window's children during iconify/de-iconify operation
Suggestion:
// Hides/shows window children during iconify/de-iconify operation
src/java.desktop/macosx/native/libawt_lwawt/awt/CTextPipe.m line 320:
> 318: acquiring transform arrays from JNI, filling buffers, or striking
> glyphs. All resources or memory
> 319: acquired at a given stage, must be released in that stage. Any error
> that occurs (like a failed malloc)
> 320: is to be handled in the stage it occurs in, and is to return
> immediately after freeing it's resources.
Suggestion:
is to be handled in the stage it occurs in, and is to return immediately
after freeing its resources.
src/java.desktop/macosx/native/libawt_lwawt/awt/QuartzSurfaceData.m line 96:
> 94: // The colors passed have low randomness. That means we need to
> scramble the bits of the color
> 95: // to produce a good hash key. After some analysis, it looks like
> Thomas's Wang integer hashing algorithm
> 96: // seems a nice trade off between performance and effectiveness.
Suggestion:
// seems a nice trade-off between performance and effectiveness.
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 63:
> 61: jboolean metalSupported = JNI_FALSE;
> 62:
> 63: // It is guaranteed that metal supported GPU is available macOS 10.14
> onwards
It sounds as if something is missing before “macOS”…
Suggestion:
// It is guaranteed that metal supported GPU is available since macOS 10.14
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLVertexCache.m line
248:
> 246: // we can give destination subtexturing properly but we can't
> 247: // subtexture from system memory glyph we have. So in such
> 248: // cases we are creating separate tile and scan the source
Suggestion:
// cases we are creating a separate tile and scan the source
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLVertexCache.m line
250:
> 248: // cases we are creating separate tile and scan the source
> 249: // stride into destination using memcpy. In case of OpenGL we
> 250: // can update source pointers, in case of D3D we ar doing memcpy.
Suggestion:
// can update source pointers, in case of D3D we are doing memcpy.
src/java.desktop/share/classes/javax/swing/ActionPropertyChangeListener.java
line 49:
> 47: * <p>
> 48: * WARNING WARNING WARNING WARNING WARNING WARNING:<br>
> 49: * Do NOT create an anonymous inner class that extends this! If you do
Suggestion:
* Do NOT create an anonymous inner class that extends this! If you do,
A comma will separate the condition from the result.
src/java.desktop/share/classes/javax/swing/BoxLayout.java line 503:
> 501: * The relative axis values, PAGE_AXIS and LINE_AXIS are converted
> 502: * to their absolute counterpart given the target's
> ComponentOrientation
> 503: * value. The absolute axes, X_AXIS and Y_AXIS are returned
> unmodified.
Shall we put a comma after LINE_AXIS and Y_AXIS and “are”?
src/java.desktop/share/classes/javax/swing/DefaultRowSorter.java line 1038:
> 1036: if (!sorted || viewToModel.length == 0 ||
> 1037: (lastRow - firstRow) > viewToModel.length / 10) {
> 1038: // We either weren't sorted, or to much changed, sort it
> all or
Suggestion:
// We either weren't sorted, or too much changed, sort it all or
src/java.desktop/share/classes/javax/swing/GroupLayout.java line 1654:
> 1652: /**
> 1653: * Used to compute how the two values representing two springs
> 1654: * will be combined. For example, a group that laid things out
Suggestion:
* will be combined. For example, a group that laid out things
Not sure which one is correct. Usually the particle in a phrasal verb follows
the verb is the object is a noun.
src/java.desktop/share/classes/javax/swing/JCheckBox.java line 248:
> 246:
> 247: /**
> 248: * The icon for checkboxs comes from the look and feel,
Suggestion:
* The icon for checkboxes comes from the look and feel,
src/java.desktop/share/classes/javax/swing/JLayeredPane.java line 532:
> 530: if(curLayer == layer) {
> 531: layerCount++;
> 532: /// Short circuit the counting when we have them all
Suggestion:
/// Short-circuit the counting when we have them all
src/java.desktop/share/classes/javax/swing/JLayeredPane.java line 558:
> 556: if(curLayer == layer) {
> 557: results[layerCount++] = getComponent(i);
> 558: /// Short circuit the counting when we have them all
Suggestion:
/// Short-circuit the counting when we have them all
src/java.desktop/share/classes/javax/swing/JMenu.java line 201:
> 199: * Overridden to do nothing. We want JMenu to be focusable, but
> 200: * <code>JMenuItem</code> doesn't want to be, thus we override this
> 201: * do nothing. We don't invoke <code>setFocusable(true)</code> after
Suggestion:
* to do nothing. We don't invoke <code>setFocusable(true)</code> after
src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 335:
> 333:
> 334: /**
> 335: * Returns an point which has been adjusted to take into account of
> the
Suggestion:
* Returns an point which has been adjusted to take into account the
src/java.desktop/share/classes/javax/swing/JSpinner.java line 675:
> 673: // active vs those of the JFormattedTextField. As such we
> 674: // put disabled actions in the JFormattedTextField's
> actionmap.
> 675: // A binding to a disabled action is treated as a nonexistent
Suggestion:
// A binding to a disabled action is treated as a non-existent
The word is spelt with a hyphen in all the preceding cases (in this code
review).
src/java.desktop/share/classes/javax/swing/SwingWorker.java line 262:
> 260:
> 261: /**
> 262: * handler for {@code process} method.
Suggestion:
* Handler for {@code process} method.
To be consistent in starting from a capital letter, even though it's a private
field.
src/java.desktop/share/classes/javax/swing/plaf/metal/MetalRootPaneUI.java line
316:
> 314: uninstallLayout(root);
> 315: // We have to revalidate/repaint root if the style is
> JRootPane.NONE
> 316: // only. When we needs to call revalidate/repaint with other
> styles
Suggestion:
// only. When we need to call revalidate/repaint with other styles
src/java.desktop/share/classes/javax/swing/plaf/metal/MetalTheme.java line 68:
> 66: public abstract class MetalTheme {
> 67:
> 68: // Constants identifying the various Fonts that are Theme can support
Suggestion:
// Constants identifying the various Fonts that a Theme can support
src/java.desktop/share/classes/javax/swing/plaf/nimbus/AbstractRegionPainter.java
line 332:
> 330: return laf.getDerivedColor(key, hOffset, sOffset, bOffset,
> aOffset, true);
> 331: } else {
> 332: // can not give a right answer as painter should not be used
> outside
Suggestion:
// cannot give the right answer as painter should not be used
outside
The sentence suggests there's only _one_ right answer, therefore the definite
article.
src/java.desktop/share/classes/javax/swing/plaf/nimbus/SynthPainterImpl.java
line 126:
> 124: Component c = ctx.getComponent();
> 125: boolean ltr = c.getComponentOrientation().isLeftToRight();
> 126: // Don't RTL flip JSpliders as they handle it internally
Suggestion:
// Don't RTL flip JSliders as they handle it internally
src/java.desktop/share/classes/javax/swing/plaf/synth/doc-files/synthFileFormat.html
line 876:
> 874: used for all directions.</dd>
> 875: <dt><a id="imagePainter.path"><samp>path</samp></a></dt>
> 876: <dd>Path to the image. Path to the image. If
> SynthLookAndFeel.load is
Suggestion:
<dd>Path to the image. If SynthLookAndFeel.load is
I believe the duplicate sentence can be dropped.
src/java.desktop/share/classes/javax/swing/plaf/synth/doc-files/synthFileFormat.html
line 877:
> 875: <dt><a id="imagePainter.path"><samp>path</samp></a></dt>
> 876: <dd>Path to the image. Path to the image. If
> SynthLookAndFeel.load is
> 877: passed a Class this will use the Class method getResource (with with
> the
Suggestion:
passed a Class this will use the Class method getResource (with the
src/java.desktop/share/classes/javax/swing/text/WrappedPlainView.java line 661:
> 659: * This class tries to be lightweight by carrying little
> 660: * state of it's own and sharing the state of the outer class
> 661: * with it's sibblings.
Suggestion:
* state of its own and sharing the state of the outer class
* with its siblings.
src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1779:
> 1777: * <p>
> 1778: * The CSS parser uses the parseCssValue method to convert
> 1779: * a string to whatever format is appropriate a given key
Suggestion:
* a string to whatever format is appropriate for a given key
src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 1781:
> 1779: * a string to whatever format is appropriate a given key
> 1780: * (i.e. these converters are stored in a map using the
> 1781: * CSS.Attribute as a key and the CssValue as the value).
Suggestion:
* CSS.Attribute as the key and the CssValue as the value).
src/java.desktop/share/classes/javax/swing/text/html/parser/TagStack.java line
39:
> 37: * When a start tag is encountered an element is pushed onto
> 38: * the stack, when an end tag is encountered an element is popped
> 39: * of the stack.
Suggestion:
* off the stack.
src/java.desktop/share/classes/javax/swing/tree/FixedHeightLayoutCache.java
line 645:
> 643:
> 644: /**
> 645: * Ensures that all the path components in path are expanded, accept
Suggestion:
* Ensures that all the path components in path are expanded, except
src/java.desktop/share/classes/sun/awt/image/ImagingLib.java line 68:
> 66:
> 67: /**
> 68: * Returned value indicates whether the library initialization was
Suggestion:
* Returned value indicates whether the library initialization
src/java.desktop/share/classes/sun/awt/image/ImagingLib.java line 71:
> 69: * succeeded.
> 70: *
> 71: * There could be number of reasons to failure:
Suggestion:
* There could be a number of reasons for failure:
src/java.desktop/share/classes/sun/swing/SwingUtilities2.java line 1782:
> 1780: /**
> 1781: * Returns an integer from the defaults table. If {@code key} does
> 1782: * not map to a valid {@code Integer}, or can not be converted from
Suggestion:
* not map to a valid {@code Integer}, or cannot be converted from
src/java.desktop/share/classes/sun/swing/SwingUtilities2.java line 1795:
> 1793: * Returns an integer from the defaults table that is appropriate
> 1794: * for the given locale. If {@code key} does not map to a valid
> 1795: * {@code Integer}, or can not be converted from a {@code String}
Suggestion:
* {@code Integer}, or cannot be converted from a {@code String}
And two more below.
src/java.desktop/share/classes/sun/swing/text/TextComponentPrintable.java line
772:
> 770: /*
> 771: * we do not store the same value as previous.
> in our
> 772: * documents it is often for consequent
> positions to have
Looks this actually means _consecutive_ positions?
src/java.desktop/unix/classes/sun/awt/X11/XBaseMenuWindow.java line 485:
> 483: /**
> 484: * returns item which mapped coordinates contain
> 485: * specified point, null of none.
Suggestion:
* the specified point, null if none.
src/java.desktop/unix/classes/sun/awt/X11/XBaseMenuWindow.java line 944:
> 942: * This function needs to be overridden since
> 943: * XBaseMenuWindow has no corresponding component
> 944: * so events can not be processed using standard means
Suggestion:
* so events cannot be processed using standard means
src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java line 390:
> 388: /* Implement DisposerTarget. Weak references to an Object can delay
> 389: * its storage reclamation marginally.
> 390: * It won't make the native resources be release any more quickly,
> but
Suggestion:
* It won't make the native resources be released any more quickly, but
src/java.desktop/windows/native/libawt/windows/awt_Label.h line 62:
> 60: /*
> 61: * if WM_PAINT was receiving when we can not paint
> 62: * then setup m_needPaint end when can call this function
Suggestion:
* if WM_PAINT was received when we cannot paint
* then setup m_needPaint and when can paint call this function
src/java.desktop/windows/native/libawt/windows/awt_PrintDialog.cpp line 57:
> 55: (LOWORD(wParam) == IDCANCEL))
> 56: {
> 57: // If we receive on of these two notifications, the dialog
Suggestion:
// If we receive one of these two notifications, the dialog
src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 3173:
> 3171: (LOWORD(wParam) == IDCANCEL))
> 3172: {
> 3173: // If we receive on of these two notifications, the
> dialog
Suggestion:
// If we receive one of these two notifications, the dialog
src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 1085:
> 1083: // Special awt message to call Imm APIs.
> 1084: // ImmXXXX() API must be used in the main thread.
> 1085: // In other thread these APIs does not work correctly even if
Suggestion:
// In other threads these APIs do not work correctly even if
src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp line 1087:
> 1085: // In other thread these APIs does not work correctly even if
> 1086: // it returns with no error. (This restriction is not documented)
> 1087: // So we must use thse messages to call these APIs in main thread.
Suggestion:
// So we must use these messages to call these APIs in main thread.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8328
