8313348: Fix typo in JFormattedTextField: 'it self' #15087

aivanov-jdk · 2023-07-31T10:14:45Z

A trivial change: ‘it self’ → ‘itself’.

In addition to it, I added a comma after ‘Similarly’ where it starts a sentence.

Progress

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

Issue

JDK-8313348: Fix typo in JFormattedTextField: 'it self' (Bug - P4)

Reviewers

Harshitha Onkar (@honkar-jdk - Committer) ⚠️ Review applies to c5fa16a1
Damon Nguyen (@DamonGuy - Committer) ⚠️ Review applies to c5fa16a1
Prasanta Sadhukhan (@prsadhuk - Reviewer) ⚠️ Review applies to c5fa16a1

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 15087

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-07-31T10:15:32Z

👋 Welcome back aivanov! 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.

openjdk · 2023-07-31T10:16:31Z

@aivanov-jdk The following label will be automatically applied to this pull request:

client

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

mlbridge · 2023-07-31T10:21:06Z

Webrevs

01: Full - Incremental (fedb0da0)
00: Full (c5fa16a1)

honkar-jdk · 2023-07-31T18:48:02Z

@aivanov-jdk LGTM and since the Javadocs changes are trivial, I believe it doesn't require a CSR ?

aivanov-jdk · 2023-07-31T19:12:37Z

LGTM and since the Javadocs changes are trivial, I believe it doesn't require a CSR ?

@honkar-jdk No, it doesn't require a CSR because it doesn't change the meaning.

mrserb · 2023-08-01T03:40:23Z

src/java.desktop/share/classes/javax/swing/JFormattedTextField.java

@@ -144,15 +144,15 @@
 * Alternatively, you could invoke <code>commitEdit</code>, which would also
 * commit the value.
 * <p>
- * <code>JFormattedTextField</code> does not do the formatting it self,
+ * <code>JFormattedTextField</code> does not do the formatting itself,


Looks like we have the same typo in some other files Like SwingSet2, etc. please double-check.

We may… I didn't scan the entire code base, I was reading the code of JFormattedTextField found the typo and decided to correct it. It's part of the Java SE API.

The word ‘it self’ is present in HTML files in SwingSet2 and SwingSet3 in the excerpts from Micrographia. I'd rather not touch it, these are meant to be the exact text of the book, there are many misspellings or archaic spellings in the files.

we have similar typo in the BasicTreeUI.java as well.

we have similar typo in the BasicTreeUI.java as well.

Yes, it's there:

jdk/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTreeUI.java

Lines 4084 to 4088 in b093880

// Preferably checkForClickInExpandControl could take

// the Event to do this it self!

if(SwingUtilities.isLeftMouseButton(e)) {

checkForClickInExpandControl(pressedPath, e.getX(), e.getY());

}

Yet it's not visible externally, it's visible only to those who read the code of the BasicTreeUI.Handler.handleSelection method whereas the typo I'm addressing here is in the published API documentation for JFormattedTextField.

It wasn't my goal to amend all the instances¹ where ‘it self’ is present in the code base — it takes much more time and effort².

It was supposed to be a quick and trivial bug report and fix which addresses this particular typo in the single file, JFormattedTextField.java. A single file with ~~a single change~~ with four changed lines should've been reviewed quickly and smoothly.

For what it's worth, we have spent much more time discussing it…

¹ There are quite a few; yet they're mostly in hotspot code comments. Eventually, all could be amended.
² I've been on this journey with ‘a the’ in various combinations, it took about two months since its inception to its completion.

For what it's worth, we have spent much more time discussing it…

Right, and I do not se a reason why other cases of the exactly same typo at least in java.desktop module is not fixed. Or do you suggest to create one more JBS issue and fix it? and then another one?

Yes, that's exactly what I suggest.

Unless you're committing to invest some considerable amount of time into cleaning up all the similar typos or other mistakes in the entire code base, doing a point-and-fix type of changes is a good trade-off, in my opinion, compared to ignoring them altogether. What I mean is limiting the change to one file, which is the smallest unit, makes it a quick fix: quick to submit a bug, quick to fix and quick to review and to integrate.

That is wrong suggestion, if the one bug found in one place it is part of the fixing process to check that it does not exists in another one. Especially for typo it is easy to check. We already discussed this for one issue in scrollbars on windows, where the same bug was missed in the similar place.

That is wrong suggestion, if the one bug found in one place it is part of the fixing process to check that it does not exists in another one. Especially for typo it is easy to check.

I don't think it is wrong, especially for typos. As I said, fixing typos in the entire code base, even if limited to the java.desktop module requires considerable amount of time. Yet fixing a typo in a certain file is a trivial fix which is easy to fix and is quick to review.

We already discussed this for one issue in scrollbars on windows, where the same bug was missed in the similar place.

In that case, the problem could be more serious. As I explained in PR #14338, I missed this function was used in another component. At the same time, no test case has been found where functionality of ScrollBar is affected.

I have submitted the follow-up issues to clean it up.

Anyway, I have updated BasicTreeUI; in addition to fixing the typo there, I expanded the wildcard imports.

Do you have any more comments? @mrserb

Anyway, I have updated BasicTreeUI; in addition to fixing the typo there, I expanded the wildcard imports.

Do you have any more comments? @mrserb

@mrserb Have I addressed your concerns? Do you have any additional comments?

DamonGuy

Change looks fine and the change itself is trivial, so looks good overall. Hopefully the rest can be updated as they're encountered when those other files are touched. I'll keep a lookout myself

openjdk · 2023-08-03T03:23:43Z

@aivanov-jdk 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:

8313348: Fix typo in JFormattedTextField: 'it self'

Reviewed-by: honkar, dnguyen, psadhukhan

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 474 new commits pushed to the master branch:

35bccac: 8315841: RISC-V: Check for hardware TSO support
a04c6c1: 8315609: Open source few more swing text/html tests
dab1c21: 8314491: Linux: jexec launched via PATH fails to find java
9a83d55: 8315373: Change VirtualThread to unmount after freezing, re-mount before thawing
68f6941: 8314452: Explicitly indicate inlining success/failure in PrintInlining
b482e6d: 8315580: Remove unused java_lang_String::set_value_raw()
9b0da48: 8315410: Undocumented exceptions in java.text.StringCharacterIterator
578ded4: 8312418: Add Elements.getEnumConstantBody
dccf670: 8306632: Add a JDK Property for specifying DTD support
a62c48b: 8315891: java/foreign/TestLinker.java failed with "error occurred while instantiating class TestLinker: null"
... and 464 more: https://git.openjdk.org/jdk/compare/408987e1ca9a42db8019b1bd7e52f85607975dde...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.

aivanov-jdk · 2023-08-05T18:42:39Z

Hopefully the rest can be updated as they're encountered when those other files are touched.

That's what I usually do. If I see a problem in a file, a class, I tend to report it and fix it. Doing it quickly is preferable because I'm not always ready to spend more time into looking around to address other instances of the same problem.

I'll keep a lookout myself

Thanks! @DamonGuy Gradually, even if one file at a time, we'll get to a cleaner code base.

bridgekeeper · 2023-09-05T05:20:00Z

@aivanov-jdk 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!

aivanov-jdk · 2023-10-05T14:35:56Z

/integrate

openjdk · 2023-10-05T14:40:25Z

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

a8eacb3: 8317240: Promptly free OopMapEntry after fail to insert the entry to OopMapCache
4c5b66d: 8317522: Test logic for BODY_CF in AbstractThrowingSubscribers.java is wrong
42be238: 8316523: Relativize esp in interpreter frames (PowerPC only)
a250ae4: 8317039: Enable specifying the JDK used to run jtreg
1ed9c76: 8309204: Obsolete DoReserveCopyInSuperWord
3630af2: 8316594: C2 SuperWord: wrong result with hand unrolled loops
c6c69b5: 8314654: Metaspace: move locking out of MetaspaceArena
3105538: 8316146: Open some swing tests 4
36314a9: 8267509: Improve IllegalAccessException message to include the cause of the exception
ddacf92: 8305765: CompressedClassPointers.java is unreliable due to ASLR
... and 817 more: https://git.openjdk.org/jdk/compare/408987e1ca9a42db8019b1bd7e52f85607975dde...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-10-05T14:40:41Z

@aivanov-jdk Pushed as commit a1c9587.

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

8313348: Fix typo in JFormattedTextField: 'it self'

c5fa16a

openjdk bot added the rfr Pull request is ready for review label Jul 31, 2023

openjdk bot added the client client-libs-dev@openjdk.org label Jul 31, 2023

honkar-jdk approved these changes Jul 31, 2023

View reviewed changes

mrserb reviewed Aug 1, 2023

View reviewed changes

DamonGuy approved these changes Aug 3, 2023

View reviewed changes

prsadhuk approved these changes Aug 3, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Aug 3, 2023

aivanov-jdk added 2 commits September 5, 2023 12:45

BasicTreeUI: it self -> itself

761b74c

Update the copyright and organise imports in BasicTreeUI

fedb0da

openjdk bot added the integrated Pull request has been integrated label Oct 5, 2023

openjdk bot closed this Oct 5, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Oct 5, 2023

aivanov-jdk deleted the 8313348-formattedField-itself branch October 5, 2023 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8313348: Fix typo in JFormattedTextField: 'it self' #15087

8313348: Fix typo in JFormattedTextField: 'it self' #15087

aivanov-jdk commented Jul 31, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jul 31, 2023

openjdk bot commented Jul 31, 2023

mlbridge bot commented Jul 31, 2023 •

edited

Loading

honkar-jdk commented Jul 31, 2023 •

edited

Loading

aivanov-jdk commented Jul 31, 2023

mrserb Aug 1, 2023

aivanov-jdk Aug 1, 2023

aivanov-jdk Aug 1, 2023

mrserb Aug 1, 2023

aivanov-jdk Aug 2, 2023

aivanov-jdk Aug 5, 2023

mrserb Aug 8, 2023

aivanov-jdk Sep 7, 2023

aivanov-jdk Sep 7, 2023

aivanov-jdk Sep 28, 2023

DamonGuy left a comment

openjdk bot commented Aug 3, 2023 •

edited

Loading

aivanov-jdk commented Aug 5, 2023

bridgekeeper bot commented Sep 5, 2023

aivanov-jdk commented Oct 5, 2023

openjdk bot commented Oct 5, 2023

openjdk bot commented Oct 5, 2023

	// Preferably checkForClickInExpandControl could take
	// the Event to do this it self!
	if(SwingUtilities.isLeftMouseButton(e)) {
	checkForClickInExpandControl(pressedPath, e.getX(), e.getY());
	}

8313348: Fix typo in JFormattedTextField: 'it self' #15087

8313348: Fix typo in JFormattedTextField: 'it self' #15087

Conversation

aivanov-jdk commented Jul 31, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jul 31, 2023

openjdk bot commented Jul 31, 2023

mlbridge bot commented Jul 31, 2023 • edited Loading

Webrevs

honkar-jdk commented Jul 31, 2023 • edited Loading

aivanov-jdk commented Jul 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DamonGuy left a comment

Choose a reason for hiding this comment

openjdk bot commented Aug 3, 2023 • edited Loading

aivanov-jdk commented Aug 5, 2023

bridgekeeper bot commented Sep 5, 2023

aivanov-jdk commented Oct 5, 2023

openjdk bot commented Oct 5, 2023

openjdk bot commented Oct 5, 2023

aivanov-jdk commented Jul 31, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Jul 31, 2023 •

edited

Loading

honkar-jdk commented Jul 31, 2023 •

edited

Loading

openjdk bot commented Aug 3, 2023 •

edited

Loading