Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Nir Lisker
On Tue, 22 Nov 2022 21:30:09 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentation > > modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java > line 418: >

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Nir Lisker
On Tue, 29 Nov 2022 17:11:51 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentation > > modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/WindowStage.java > line 422: >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Nir Lisker
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Nir Lisker
On Tue, 29 Nov 2022 18:37:23 GMT, Kevin Rushforth wrote: >> interesting - there is technically no change, as `TableColumnBase` >> implements `EventTarget`. >> @hjohn does javadoc produce a different result? > > I checked, and yes it does produce a different result: > > Current: > > > public c

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread Kevin Rushforth
On Mon, 28 Nov 2022 17:04:17 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be aut

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v9]

2022-11-29 Thread Andy Goryachev
> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in > [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). > > We propose to address all these issues by replacing the old column resize > algorithm with a different one, which not only honors all the constraints

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread Nir Lisker
On Mon, 28 Nov 2022 17:04:17 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be aut

Re: RFR: 8287822: [macos] Remove support of duplicated formats from macOS [v3]

2022-11-29 Thread Kevin Rushforth
On Thu, 13 Oct 2022 04:25:19 GMT, Alexander Matveev wrote: >> - Added support for JAR and JRT protocol to AVFoundation platform. >> - Removed H.264/MP3 and AAC support from GStreamer platform, which was >> primary used to playback these formats for JAR and JRT protocols. >> - Added ability to

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v8]

2022-11-29 Thread Andy Goryachev
> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in > [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). > > We propose to address all these issues by replacing the old column resize > algorithm with a different one, which not only honors all the constraints

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 21:58:38 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 21:47:05 GMT, John Hendrikx wrote: >> I see that this change has been reverted. That's good. >> >> FWIW, I disagree with the original premise of removing it. The subclass >> shouldn't have to know or care that the superclass method is empty or not. >> >> I looked through th

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v9]

2022-11-29 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be >

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v7]

2022-11-29 Thread Kevin Rushforth
On Mon, 28 Nov 2022 21:07:03 GMT, Andy Goryachev wrote: >> The current CONSTRAINED_RESIZE_POLICY has a number of issues as explained in >> [JDK-8292810](https://bugs.openjdk.org/browse/JDK-8292810). >> >> We propose to address all these issues by replacing the old column resize >> algorithm wi

Re: RFR: 8293119: Additional constrained resize policies for Tree/TableView [v4]

2022-11-29 Thread Kevin Rushforth
On Wed, 23 Nov 2022 22:46:25 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/ConstrainedColumnResizeBase.java >> line 34: >> >>> 32: * @since 20 >>> 33: */ >>> 34: public abstract class ConstrainedColumnResizeBase { >> >> Do you think this class would

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread John Hendrikx
On Tue, 29 Nov 2022 21:36:11 GMT, Kevin Rushforth wrote: >> I think that we should refrain from making "structural" changes in this >> pass. Let's keep the warning on and deal with these later. > > I see that this change has been reverted. That's good. > > FWIW, I disagree with the original pre

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread Kevin Rushforth
On Wed, 23 Nov 2022 21:41:09 GMT, Nir Lisker wrote: >> The `processEndElement` in the base class was an empty method that throws >> `IOException`. By making it `abstract` the warning is avoided, but this >> means the call to `super` here is no longer relevant (it wasn't relevant >> anyway as

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 18:24:29 GMT, Andy Goryachev wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/TableColumn.java >> line 134: >> >>> 132: * @since JavaFX 2.0 >>> 133: */ >>> 134: public class TableColumn extends TableColumnBase { >> >> This class is part of the public A

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 16:55:34 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 18:12:26 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains 23 commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >> feature/fix-easy-warnings

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Kevin Rushforth
On Wed, 23 Nov 2022 16:49:50 GMT, Andy Goryachev wrote: >> You are correct that with these kind of changes you can't see if it is >> correct just from looking at the diff. Variables would need to be named more >> explicitly, or explicit casts would need to be added to repeat the type >> inform

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Kevin Rushforth
On Tue, 22 Nov 2022 21:34:44 GMT, Kevin Rushforth wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix indentation > > modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2VramPool.java line > 54: > >> 52:

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Kevin Rushforth
On Mon, 28 Nov 2022 13:39:05 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be aut

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 16:55:34 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297412: Remove easy warnings in javafx.fxml, javafx.media, javafx.swing, javafx.swt and javafx.web [v5]

2022-11-29 Thread Andy Goryachev
On Mon, 28 Nov 2022 17:04:17 GMT, John Hendrikx wrote: >> - Remove unsupported/unnecessary SuppressWarning annotations >> - Remove reduntant type specifications (use diamond operator) >> - Remove unused or duplicate imports >> - Remove unnecessary casts (type is already correct type or can be aut

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v8]

2022-11-29 Thread John Hendrikx
> Note: I ran into a `javac` compiler bug while replacing types with diamond > operators (ecj has no issues). I had two options, add a > `SuppressWarnings("unused")` or to use a lambda instead of a method reference > to make `javac` happy. I choose the later and added a comment so it can be >

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread John Hendrikx
On Tue, 29 Nov 2022 16:38:26 GMT, Andy Goryachev wrote: > oh no, my PRs are going to wreak havoc with your large PRs. let's make sure > yours get in first. That's okay, merge conflicts can be solved. > > Unsure what you mean by the changes, as I don't know what the default > > setting (or you

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:41:24 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:56:48 GMT, John Hendrikx wrote: > Unsure what you mean by the changes, as I don't know what the default setting > (or your settings) are, but I've included the settings as an attachment: the warnings being addressed by this PR. - PR: https://git.openjdk.org/

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 07:41:24 GMT, John Hendrikx wrote: >> Note: I ran into a `javac` compiler bug while replacing types with diamond >> operators (ecj has no issues). I had two options, add a >> `SuppressWarnings("unused")` or to use a lambda instead of a method >> reference to make `javac` h

Integrated: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-29 Thread Andy Goryachev
On Fri, 21 Oct 2022 19:01:54 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 174) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not re

Re: RFR: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-29 Thread Andy Goryachev
On Tue, 29 Nov 2022 11:10:39 GMT, Ajit Ghaisas wrote: >> as determined by SkinMemoryLeakTest (remove line 174) and a leak tester >> https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java >> >> caused by: >> - adding and not removing listeners >> - adding and no

Re: RFR: JDK-8297413: Remove easy warnings in javafx.graphics [v2]

2022-11-29 Thread Kevin Rushforth
On Sat, 26 Nov 2022 19:24:21 GMT, Nir Lisker wrote: >> The cast has precedence and Java will not downcast mid calculation and lose >> precision, so all examples here are correct. > > `4L * physicalWidth * physicalHeight;` does not require an explicit cast, so > I think it's the clearest. That

Re: RFR: 8295754: PaginationSkin: memory leak when changing skin

2022-11-29 Thread Kevin Rushforth
On Thu, 20 Oct 2022 20:48:07 GMT, Andy Goryachev wrote: > Fixes memory leaks as determined by SkinMemoryLeakTest (remove line 171) and > a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > Make sure to configure the current test in LeakTe

Integrated: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest

2022-11-29 Thread Lukasz Kostyra
On Thu, 17 Nov 2022 16:59:09 GMT, Lukasz Kostyra wrote: > The change moves Locale setting in the test to `@BeforeClass` and > `@AfterClass` calls. `@BeforeClass` method call stores current default VM > locale and applies Locale.US, while `@AfterClass` method restores old VM > locale after all

Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v30]

2022-11-29 Thread Johan Vos
On Tue, 22 Nov 2022 01:40:02 GMT, Thiago Milczarek Sayao wrote: >> This cleans size and positioning code, reducing special cases, code >> complexity and size. >> >> Changes: >> >> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different >> sizes. It does not assume any si

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v6]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 13:36:14 GMT, Lukasz Kostyra wrote: >> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v6]

2022-11-29 Thread Lukasz Kostyra
> The change moves Locale setting in the test to `@BeforeClass` and > `@AfterClass` calls. `@BeforeClass` method call stores current default VM > locale and applies Locale.US, while `@AfterClass` method restores old VM > locale after all tests are completed. > > I tested it both on Mac and Wind

Integrated: 8297680: JavaDoc example for PseudoClass has minor typo

2022-11-29 Thread Karthik P K
On Tue, 29 Nov 2022 10:06:57 GMT, Karthik P K wrote: > Fixed the typo in JavaDoc example code for PseudoClass This pull request has now been integrated. Changeset: d040c1f0 Author:Karthik P K Committer: Kevin Rushforth URL: https://git.openjdk.org/jfx/commit/d040c1f0153411479ca35c5

Re: RFR: 8297680: JavaDoc example for PseudoClass has minor typo

2022-11-29 Thread Karthik P K
On Tue, 29 Nov 2022 12:37:50 GMT, Kevin Rushforth wrote: >> Fixed the typo in JavaDoc example code for PseudoClass > > Looks good. @kevinrushforth please sponsor this PR. - PR: https://git.openjdk.org/jfx/pull/964

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v5]

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 11:34:02 GMT, Lukasz Kostyra wrote: >> The change moves Locale setting in the test to `@BeforeClass` and >> `@AfterClass` calls. `@BeforeClass` method call stores current default VM >> locale and applies Locale.US, while `@AfterClass` method restores old VM >> locale after

Re: RFR: 8297680: JavaDoc example for PseudoClass has minor typo

2022-11-29 Thread Kevin Rushforth
On Tue, 29 Nov 2022 10:06:57 GMT, Karthik P K wrote: > Fixed the typo in JavaDoc example code for PseudoClass Looks good. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.org/jfx/pull/964

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v5]

2022-11-29 Thread Lukasz Kostyra
> The change moves Locale setting in the test to `@BeforeClass` and > `@AfterClass` calls. `@BeforeClass` method call stores current default VM > locale and applies Locale.US, while `@AfterClass` method restores old VM > locale after all tests are completed. > > I tested it both on Mac and Wind

Re: RFR: 8295796: ScrollPaneSkin: memory leak when changing skin

2022-11-29 Thread Ajit Ghaisas
On Fri, 21 Oct 2022 19:01:54 GMT, Andy Goryachev wrote: > as determined by SkinMemoryLeakTest (remove line 174) and a leak tester > https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/apps/LeakTest.java > > caused by: > - adding and not removing listeners > - adding and not re

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v4]

2022-11-29 Thread Lukasz Kostyra
On Mon, 28 Nov 2022 16:07:34 GMT, Andy Goryachev wrote: >> Lukasz Kostyra has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Remove trailing whitespaces > > modules/javafx.base/src/test/java/test/javafx/util/converter/LocalDateTimeStringCon

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v3]

2022-11-29 Thread Lukasz Kostyra
On Mon, 28 Nov 2022 14:51:40 GMT, Kevin Rushforth wrote: >> Lukasz Kostyra has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >> JDK-8265828-locale >>

RFR: 8297680: JavaDoc example for PseudoClass has minor typo

2022-11-29 Thread Karthik P K
Fixed the typo in JavaDoc example code for PseudoClass - Commit messages: - Fix typo in JavaDoc example for PseudoClass Changes: https://git.openjdk.org/jfx/pull/964/files Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=964&range=00 Issue: https://bugs.openjdk.org/browse/JDK-829

Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v30]

2022-11-29 Thread Johan Vos
On Tue, 22 Nov 2022 01:40:02 GMT, Thiago Milczarek Sayao wrote: >> This cleans size and positioning code, reducing special cases, code >> complexity and size. >> >> Changes: >> >> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different >> sizes. It does not assume any si

Re: RFR: 8265828: [TestBug] Save and restore the default Locale in javafx.base unit test LocalDateTimeStringConverterTest [v3]

2022-11-29 Thread Lukasz Kostyra
On Mon, 28 Nov 2022 14:49:12 GMT, Kevin Rushforth wrote: >> Lukasz Kostyra has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains four commits: >> >> - Merge branch 'master' of https://git.openjdk.org/jfx into >> JDK-8265828-locale >>

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v6]

2022-11-29 Thread John Hendrikx
On Tue, 29 Nov 2022 00:25:40 GMT, Andy Goryachev wrote: > I think this looks good. My only two suggestions would be > > 1. add util.loadStubToolkit() and > 2. collapse multiple lines where we can I will do so where possible, but a lot of these changes are automated so I may miss some. As fo

Re: RFR: JDK-8297414: Remove easy warnings in javafx.controls [v7]

2022-11-29 Thread John Hendrikx
On Mon, 28 Nov 2022 23:32:57 GMT, Andy Goryachev wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fix some indents and put declarations on one line where possible > > modules/javafx.controls/src/test/java/test/java