Repository navigation
Swing, SwingX and JavaFX compatibility layers: desktop Java apps as Codename One apps - #5980
shai-almog wants to merge 288 commits into
Conversation
…us bar, hyperlink JXTitledPanel, JXHeader, JXCollapsiblePane, JXTaskPane and its container, VerticalLayout and HorizontalLayout, JXStatusBar with its constraints, and JXHyperlink with the hyperlink actions, which open an address through Codename One. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ext fields JXMonthView draws one month and selects through the SwingX date selection models; JXDatePicker is a Codename One date Picker that keeps a month view in step. PromptSupport and the JX text fields map the prompt to the hint of the text field behind the component. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AbstractPainter and the layout and area bases, the matte, compound, rectangle, text, image, gloss, pinstripe, checkerboard, busy, alpha and shape painters, and JXLabel, JXButton, JXBusyLabel and JXTitledSeparator over them. Tests draw into a recording graphics. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ighlighters and rollover Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ides columns Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lifecycle and input events Node, Parent, Group, Region and Pane with one lazily created Codename One peer per node, the JavaFX layout pass pushing bounds into the peers in device pixels, capture and bubble dispatch of pointer and key input, focus, hover and pressed state, the stage hosts (form, window, dialog), Application, Platform and FxLifecycle, the styling hooks, and the first controls (Control, Labeled, ButtonBase, Button, Label). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ene graph to zero SpotBugs findings Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
HBox, VBox, BorderPane, StackPane, GridPane (with ColumnConstraints, RowConstraints and ConstraintsBase), FlowPane, TilePane and AnchorPane, with the per child constraints, grow and shrink distribution, margins, spans, percentages, wrapping and anchors, and the style hooks the styling contract promises for them. LayoutPanesTest holds each pane to child bounds worked out by hand from the documented layout rules. Region gains a package-private way to position a child inside a margin without resizing it, which BorderPane needs for its edges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ab panes TextInputControl, TextField, PasswordField, TextArea, CheckBox, ToggleButton, RadioButton, Toggle, ToggleGroup, Hyperlink, Separator, Slider, ProgressIndicator, ProgressBar, ScrollPane, SplitPane, TabPane, Tab, SelectionModel, SingleSelectionModel and IndexRange, each over the matching Codename One component where one exists. The scroll, split and tab panes are built from scene-graph regions so their content stays a real descendant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
javafx.animation: Animation, Timeline, KeyFrame, KeyValue, Interpolator, AnimationTimer, Transition and the pause, fade, translate, scale, rotate, sequential and parallel transitions, all driven by one frame clock (runtime.FrameClock) that registers with the form on screen only while something runs and has a manual mode for tests. javafx.concurrent: Worker, WorkerStateEvent, Task and Service. Codename One has no java.util.concurrent beyond the atomic classes, so a Task is a Runnable rather than a FutureTask, has no blocking get(), and a Service has no executor property. The hand-over between threads uses atomics and Platform.runLater only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds javafx.scene.shape (Shape and its subclasses, Path and its elements, SVGPath with a complete path data parser), javafx.scene.text.Text, javafx.scene.transform (2D), javafx.scene.canvas and javafx.scene.image. Everything draws through the one Renderer over FxPath, which gain an elliptical arc, an elliptical round rectangle and a clip. Node.getTransforms() is folded into the paint matrix and so into localToParent, bounds and picking. A canvas keeps a display list that is replayed on paint and is bounded by clearRect and a cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…logs ListView and TableView show their items in recycled cells over a shared row flow, with single and multiple row selection. ComboBox, ChoiceBox and DatePicker sit on the Codename One picker. ContextMenu, Menu and MenuBar open as popups in a layer over the owner's form, which every host reaches through SceneInput. Dialog, Alert, TextInputDialog and ChoiceDialog block through a modal stage. Control gains a context menu and a tooltip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…(T) of an absent item as JavaFX has it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llapsible pane's toggle action, action accelerators The click tests no longer wait the click count out; the test base resets it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…odelSupport Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… classes Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…yout A text area given its text inside a scroll pane overflowed the stack on the pane's next layout: JViewport.doLayout showed the caret, showing a rectangle validated the viewport, and the viewport -- still invalid, because it was in the middle of that very layout -- laid itself out again. Container.validateTree and JViewport.doLayout no longer start a pass that is already under way, which covers the same shape in JList.ensureIndexIsVisible, ScrollDelegate.reveal (JTable, JTree) and JComponent.scrollRectToVisible. The caret is shown after the widget received the text that moved it, not before, so an appended log is followed to its last line. JXErrorPane's workaround is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
maven/swing-compat and maven/javafx-compat are relocated into an application and translated by ParparVM with it, where a failed cast is not an exception. Both are roots of check-cast-semantics.sh now, and the PR build compiles them so that --require-all finds them. The six findings were all in javafx.beans.binding.Bindings: the create*Binding factories read the result of an erased Callable inside a try that catches Exception. The result is read as an Object and tested with instanceof. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l acts once The two scroll bars are children of the pane. A desktop port gives them room by policy and overflow and they are dragged with the mouse; a touch device gives them none and keeps Codename One's own indicator. Corner components are laid out, and the upper trailing one also beside a column header when there is no bar below it, which is where SwingX puts the column control. A MouseWheelListener still receives every wheel event: the window's root peer listens to Codename One's wheel events, which fire before anything scrolls. What was wrong is that the content scrolled as well. The Codename One event is consumed now when a listener received it, or when the pane has wheel scrolling turned off, and a wheel event passed on to the pane scrolls it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s on Without line wrap, the default as on the desktop, the area is as wide as its longest line and a scroll pane scrolls it sideways; with it, the area is as wide as the viewport. The Codename One widget always breaks at its width, so it is kept from breaking by being given the width the text needs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h links A pane with the content type text/html is drawn by the reader the labels use instead of as its text with the tags taken out, wrapped at the pane's width. The reader keeps the href of the link a run of text is in, and a click on one fires an ACTIVATED HyperlinkEvent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pup selects once An editable combo box is now a container of its ComboBoxEditor's component and an arrow button that opens a menu of the model's items; Enter in the field makes the text the selected item and fires comboBoxEdited. The modal popup of a combo box that is not editable keeps what the user highlights to itself until it closes, so a dismissed popup fires nothing, and hidePopup closes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A desktop application is compiled against a full JDK and names members the device's classes do not have. The remap now redirects them: - System properties (user.home, os.name, the separators), getenv, exit, lineSeparator; Runtime shutdown hooks, availableProcessors, maxMemory; Thread daemon/name/interrupt state, timed join and the default uncaught exception handler, which is fed from Display's EDT error handling. - String.join/matches/format(Locale)/toLowerCase(Locale), the StringBuilder and StringBuffer searches, substring and replace, and the Character tests. - The Integer/Long/Short/Byte/Double/Float/Boolean statics, and Math's exact, floor, hyperbolic and rounding functions; the transcendental ones go to MathUtil. StrictMath follows Math. - Whole classes: InvocationTargetException, Thread.UncaughtExceptionHandler, java.util.logging Logger and Level over Log, java.util.prefs Preferences over the framework's Preferences. A rule's target may now name a differently called method, which is how Long.decode and Integer.decode share one class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…record The remap step writes the class the project settings name (codename1.packageName + codename1.mainName): for a Swing application a DesktopLifecycle whose runMain() calls the recorded main(String[]), for a JavaFX one an FxLifecycle whose createApplication() constructs the recorded Application -- its main is never called, and Application.launch does nothing. The kind is inferred when the record leaves it out, a missing record falls back to the only candidate, and everything else fails the build saying what cn1-desktop.properties should hold. The importer sets the template's main class aside so the generated one can take its name, and reports third-party libraries as bundled and relocated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
compat_unrelocated searched class files for the bytes of a package name, and so reported an application that merely holds "javax/swing/JTable" in a string. It now asks UnrelocatedTypes, a small program in the build engine that reads each class as the relocation does and reports type references only. A failure of the check itself no longer reads as a clean directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A dependency jar that is part of the application (Maven's compile scope, Gradle's runtime classpath) and refers to a desktop layer is unpacked into the classes directory before the relocation, so it is relocated, redirected and compliance-checked as application code. A provided jar, and one that names no desktop layer, is left as it was. What was unpacked is recorded inside the classes: an unchanged jar is left alone on the next run, a compliance finding names the jar it is in, the check indexes the relocated copy rather than the jar, and the staged application no longer merges the jar a second time, unrelocated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ientations A wrapping list is a grid of equal cells, with the desktop's rules for how many rows and columns it has, and left and right move the selection by a column. Cell bounds, hit testing and sizes are checked against the JDK's own list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ends mid-drain A period that elapses while the queue is drained is a new event, so the coalesced count is bounded rather than exactly one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9919796706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// The `**/*.fxml` and `**/*.css` files under the desktop resources. | ||
| /// The task is skipped when there are none. | ||
| @InputFiles | ||
| @SkipWhenEmpty |
There was a problem hiding this comment.
When the final .fxml or .css file is deleted, @SkipWhenEmpty makes this task finish with NO-SOURCE without invoking DesktopResourceCompiler.run() (Gradle documentation). The compiler's cleanup for stale FxmlDocuments.java and .cn1css files therefore never runs, while both output directories remain attached to the main source set, so an incremental build can continue compiling and packaging resources that no longer exist. Allow the empty-input invocation to run so it can clear its previous outputs.
Useful? React with 👍 / 👎.
| if (millis < 0) { | ||
| signal.get(); | ||
| } else { | ||
| signal.get((int) Math.max(1, Math.min(millis, Integer.MAX_VALUE))); |
There was a problem hiding this comment.
When a thread is blocked here on an incomplete future and another thread interrupts it, AsyncResource.get() ultimately waits through Util.wait, which catches InterruptedException and resumes waiting. Consequently CompletableFuture.get(), executor Future.get(), and CountDownLatch.await() cannot honor their declared interruption contract and may leave cancellation or shutdown code blocked until an unrelated completion occurs. Use an interrupt-aware wait path and propagate the interruption instead of relying on AsyncResource.get().
Useful? React with 👍 / 👎.
| declared = wireDesktopCompat(common, DesktopProjectImporter.moduleDir(source, module)); | ||
| r = new DesktopProjectImporter(MavenLog.of(getLog())).importProject(source, module, common, mainClass, |
There was a problem hiding this comment.
When entry-point selection later fails—for example, because the source project has multiple candidates and no main-class setting—wireDesktopCompat has already written compatibility and library dependencies into common/pom.xml. The command then reports that nothing was imported, but still leaves the destination project modified. Perform the non-mutating import validation before writing the POM, or restore the original POM when importProject fails.
Useful? React with 👍 / 👎.
| /// Every match from here on, each as its own result. | ||
| public Stream<MatchResult> results() { | ||
| List<MatchResult> all = new ArrayList<MatchResult>(); | ||
| reset(); |
There was a problem hiding this comment.
Calling reset() here makes results() restart at the beginning of the entire input and also discards any configured region. For example, after find() has consumed the first match, the JDK stream contains only the remaining matches, whereas this implementation includes the first match again; matcher.region(...).results() likewise searches outside the requested region. Build the results stream from the matcher's current state instead of resetting it.
Useful? React with 👍 / 👎.
| if (formatter == DateTimeFormatter.ISO_LOCAL_TIME) { | ||
| return ISO_TIME_PATTERN; | ||
| } | ||
| return ISO_DATE_TIME_PATTERN; |
There was a problem hiding this comment.
For every unremembered formatter except ISO_LOCAL_DATE and ISO_LOCAL_TIME, this fallback assumes the local date-time pattern. Consequently common calls such as DateTimeFormatter.ISO_INSTANT.withZone(zone) and ISO_OFFSET_DATE_TIME.withZone(zone) lose their Z/offset and fractional-second formatting and instead emit a plain local timestamp. Distinguish all supported ISO constants rather than mapping them to ISO_DATE_TIME_PATTERN.
Useful? React with 👍 / 👎.
| if (BASIC.indexOf((char) b) >= 0) { | ||
| throw new IllegalArgumentException("Illegal base64 line separator character 0x" | ||
| + Integer.toString(b, 16)); |
There was a problem hiding this comment.
A custom MIME separator containing = passes this validation because = is not in BASIC, even though it is Base64's padding character and the JDK rejects it. The encoder then inserts padding-looking bytes between complete groups; this implementation's own MIME decoder treats such an = at a group boundary as an invalid ending, so getMimeEncoder(4, new byte[]{'='}) can produce output that cannot be decoded. Include the padding character in the forbidden separator set.
Useful? React with 👍 / 👎.
| for (int i = KNOWN.size() - 1; i >= 0; i--) { | ||
| Known k = KNOWN.get(i); | ||
| DateTimeFormatter f = k.formatter.get(); | ||
| if (f == null) { | ||
| KNOWN.remove(i); |
There was a problem hiding this comment.
When formatters are created or used from multiple threads, known() iterates and removes from the shared ArrayList while remember() can concurrently append to it. A removal can shrink the list after another thread has chosen its index, causing get(i) to fail, while racing appends can lose a formatter's metadata and make later withZone or format calls use the wrong fallback behavior. Protect all accesses to KNOWN with one synchronization boundary.
Useful? React with 👍 / 👎.
| // Managed versions and plugin dependencies are not the module's. | ||
| String pom = text.replaceAll("(?s)<dependencyManagement>.*?</dependencyManagement>", " ") | ||
| .replaceAll("(?s)<build>.*?</build>", " "); | ||
| Matcher dep = POM_DEPENDENCY.matcher(pom); |
There was a problem hiding this comment.
When a POM keeps a dependency disabled inside <!-- ... -->, this regex still matches its <dependency> block because comments are never removed. librariesOf() can then wire that stale coordinate into the destination POM, so importing a project that Maven itself builds without the dependency may fail artifact resolution or bundle unwanted code. Strip XML comments or read the Maven model before scanning dependency elements.
Useful? React with 👍 / 👎.
- The importer reads POMs without their XML comments, and the import goal restores the POM when the import fails after wiring it. - JdkTime keeps its formatter table copy-on-write, and an ISO_ constant given a zone keeps its own offset, Z and fraction. - Base64 rejects '=' in a MIME line separator; Matcher.results() no longer resets the matcher. - Rebaseline hello RAM on macOS arm64, Windows x64 and Linux arm64: the hello application ships the compatibility runtimes, so the benchmark translates about 9,600 classes instead of 7,350. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
CodenameOne/maven/compat-jdk/src/main/java/com/codename1/compat/jdk/File.java
Lines 326 to 327 in ec52692
Honor deleteOnExit during orderly application exit
For every remapped desktop application, File.deleteOnExit() silently does nothing, so temporary files that callers expect to be removed during normal termination remain in the application sandbox across launches. This can steadily consume storage and retain sensitive temporary data even when the app terminates through the orderly JdkSystem.exit path, which already runs shutdown hooks. Record requested paths and delete them during that exit sequence instead of returning without registering any cleanup.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (used) { | ||
| throw new IllegalStateException(USED); | ||
| } | ||
| used = true; | ||
| return source; |
There was a problem hiding this comment.
When one stage closes the shared pipeline, only that stage's used flag is set. A downstream stage therefore still passes this check and consumes the source after its close handlers and active flat-mapped stream have already been released; for example, Stream.of(1).map(x -> x).close() followed by count() returns 1 instead of throwing IllegalStateException. Track the closed state in the shared StreamState and reject take() and further operations once any stage closes.
Useful? React with 👍 / 👎.
| kotlinRelocation(t -> new com.codename1.gradle.tasks.RemapCompatAction( | ||
| kotlinDir.get(), compileClasspath, onClickNames, true, null) | ||
| .withDesktopEntryRecord(desktopEntry).withApplicationMain(desktopMain) | ||
| .shippingWhenEmpty(javaSources).execute(t), |
There was a problem hiding this comment.
For a Gradle desktop module with Kotlin sources but no Java sources, compileJava is NO-SOURCE, so this relocation action deliberately becomes the full runtime-shipping pass via shippingWhenEmpty. Unlike the Java action, however, it never calls withApplicationLibraries(main.getRuntimeClasspath()); consequently CompatRemapper.bundleLibraries() sees applicationLibraries == null and skips unpacking and relocating implementation/runtime dependency jars. A Kotlin-only app that depends on a Swing or JavaFX library can therefore ship that library with unresolved desktop references or omit its relocated classes entirely.
Useful? React with 👍 / 👎.
| if (k == null || k.zone == null) { | ||
| return ZonedDateTime.parse(text, formatter); | ||
| } | ||
| return ZonedDateTime.of(localDateTimeFrom(formatter.parse(text)), k.zone); |
There was a problem hiding this comment.
When the formatter has an override zone and the input also contains an offset, this discards the parsed offset and reinterprets the local wall time in the override zone. For example, parsing 2020-01-01T12:00:00+02:00 with ISO_OFFSET_DATE_TIME.withZone(UTC) should produce 10:00Z, but this path produces 12:00Z, shifting the represented instant by two hours. Preserve the parsed instant or offset before converting it to the override zone.
Useful? React with 👍 / 👎.
| int d = hexDigit(c); | ||
| if (d < 0 || digits >= 16) { | ||
| throw new IllegalArgumentException("Invalid UUID string: " + name); |
There was a problem hiding this comment.
This applies a 16-digit limit independently to every UUID group even though their maximum widths are 8-4-4-4-12. As a result, malformed inputs such as 100000000-0-0-0-0 are accepted and then truncated by the masks below to the all-zero UUID, while UUID.fromString is required to reject them; this also lets multiple distinct untrusted strings collapse to the same identifier. Validate each group against its own width before accumulating it.
Useful? React with 👍 / 👎.
| public static ZonedDateTime plusDays(ZonedDateTime value, long days) { | ||
| return ZonedDateTime.of(value.toLocalDateTime().plusDays(days), value.getZone()); |
There was a problem hiding this comment.
When date-based arithmetic lands in a daylight-saving overlap, reconstructing the result with ZonedDateTime.of always selects the earlier valid offset instead of retaining the original offset when possible. For example, subtracting one day from 2024-10-28T02:30+01:00[Europe/Berlin] should produce the later +01:00 occurrence on October 27, but this returns the earlier +02:00 occurrence and changes the represented instant by an hour. Resolve the new local date-time using value.getOffset() as the preferred offset; the month and year helpers have the same issue.
Useful? React with 👍 / 👎.
…ry fixes from review - Adding days, months or years to a zoned value keeps the offset it had when the result falls in an hour the clocks go back over. - Text with an offset, parsed by a formatter with an override zone, is a moment shown in that zone, for the ISO constants and for patterns. - A stream stage made from a pipeline that was since closed is refused. - A Kotlin-only desktop module on Gradle bundles its runtime libraries and compiles its FXML, as the javac pass does. - UUID.fromString documents that an overlong group is truncated, which is what the JDK does, and a test holds it to that. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b053d6629
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A jar that is no longer a dependency takes its classes with it. | ||
| for (Map.Entry<String, Entry> e : before.entrySet()) { | ||
| if (!after.containsKey(e.getKey())) { | ||
| delete(classesDir, e.getValue().files, allFiles(after)); |
There was a problem hiding this comment.
During an incremental build, if a previously bundled dependency is removed and the application now supplies a class or resource at one of that dependency's recorded paths, compilation writes the new file before this cleanup runs, but this unconditional deletion still treats the path as library-owned and removes it. The resulting artifact can omit the application's newly compiled class even though a clean build succeeds; verify that the file still contains the previously recorded library bytes before deleting it, as the runtime cleanup does.
Useful? React with 👍 / 👎.
| public List<Runnable> shutdownNow() { | ||
| shutdown.set(true); | ||
| stopped.set(true); | ||
| timer.cancel(); | ||
| return pool.shutdownNow(); |
There was a problem hiding this comment.
When shutdownNow() is called while a delayed or periodic task is still waiting in this class's Timer, that task is cancelled by timer.cancel() but is absent from the list returned by pool.shutdownNow() because it was never submitted to the underlying pool. Callers therefore cannot recover all tasks that never commenced as the ExecutorService contract promises; collect the non-running entries from live and include their Scheduled runnables in the returned list.
Useful? React with 👍 / 👎.
| String own = element(POM_PARENT.matcher(text).replaceAll(" "), "artifactId"); | ||
| if (text.length() == 0 || own == null || !own.equals(element(parent.group(1), "artifactId"))) { |
There was a problem hiding this comment.
When the relative parent file has the requested artifactId but a different groupId or version, this accepts it and inherits its dependencies even though Maven rejects that local POM and resolves the requested parent elsewhere. This can make an import wire unrelated libraries or versions into the destination; compare the candidate parent's effective groupId, artifactId, and version with all three coordinates declared in the module before adding it to the chain.
Useful? React with 👍 / 👎.
| private static final Pattern KOTLIN_EXTENDS = Pattern.compile( | ||
| "\\bclass\\s+(\\w+)[^{\\n]*?:[^{\\n]*?((?:javafx\\.application\\.)?Application)\\s*\\("); | ||
| private static final Pattern KOTLIN_STATIC_MAIN = Pattern.compile("@JvmStatic\\s+fun\\s+main\\s*\\("); | ||
| private static final Pattern KOTLIN_TOP_MAIN = Pattern.compile("(?m)^fun\\s+main\\s*\\("); |
There was a problem hiding this comment.
A valid Kotlin launcher declared as public fun main() is not matched because this expression requires fun to be the first token on the line. If it is the project's only entry point, the importer reports that there is nothing to start even though Kotlin generates the normal static facade main method; accept the optional public modifier while still ensuring the function is top-level.
Useful? React with 👍 / 👎.
| for (int i = common; i < a.length; i++) { | ||
| if ("..".equals(a[i])) { | ||
| throw new IllegalArgumentException("Unable to compute relative path from " + this + " to " + other); |
There was a problem hiding this comment.
For a source path containing an internal segment that cancels a preceding name, such as Paths.get("a/..").relativize(Paths.get("b")), this loop reaches the .. and throws, whereas the JDK normalizes that source and returns b. Similar ordinary paths such as a/../c also fail, so normalize both name sequences before finding the common prefix while continuing to reject genuinely leading, uncancellable .. segments.
Useful? React with 👍 / 👎.
| timer = TIMER.get(); | ||
| } | ||
| final Timeout task = new Timeout(action); | ||
| timer.schedule(task, Math.max(0, millis)); |
There was a problem hiding this comment.
On ParparVM, every call to Timer.schedule starts a separate Timer.T thread, and TimerTask.cancel() only sets a flag that is not checked until after the initial sleep. Consequently, creating many long orTimeout or completeOnTimeout operations whose futures complete immediately leaves one thread per operation sleeping until the original deadline, potentially exhausting native threads even though the timeout actions have been released; use a delay mechanism that wakes or removes cancelled waits instead.
Useful? React with 👍 / 👎.
| /// application has; a platform that kills the process runs none. | ||
| public final class JdkSystem { | ||
|
|
||
| private static final Map<String, String> OVERRIDES = new HashMap<String, String>(); |
There was a problem hiding this comment.
When background tasks call System.setProperty, clearProperty, or getProperty concurrently, they access this shared HashMap without synchronization. Concurrent insertions into an unsynchronized HashMap can lose entries or expose inconsistent table state, so otherwise independent property updates may disappear; serialize all accesses to OVERRIDES or replace it with a thread-safe map.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nd importer fixes from review - CompletableFuture timeouts and ScheduledPool wait on one shared thread (Delays) instead of a device Timer thread per task; a cancelled wait leaves the queue at once. - ScheduledPool.shutdownNow answers the work still waiting for its time. - A library's recorded file that changed since the last build is the application's own: it is neither deleted with a dropped jar nor overwritten by a kept one. - A local parent POM is taken only when group, artifact and version match. - A Kotlin top-level main may be declared public or suspend. - Path.relativize normalizes dotted paths as a current JDK does. - The system property table and shutdown hooks are guarded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
CodenameOne/maven/compat-jdk/src/main/java/com/codename1/compat/jdk/File.java
Lines 257 to 258 in d04a01d
Do not report unsupported permission revocation as successful
For any existing file, setWritable(false) returns true even though this implementation performs no permission change and canWrite() continues to return true. Code that uses the return value to confirm that a file was made immutable can therefore proceed under a false assumption; setReadable(false) and setReadOnly() have the same problem. Return failure for requests that cannot match the unchanged readable/writable state.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!running.compareAndSet(false, true)) { | ||
| // The run before this one has not ended; a fixed rate does | ||
| // not pile runs up. | ||
| return; |
There was a problem hiding this comment.
When a fixed-rate task takes longer than its period, the next timer firing reaches this branch while running is still true and returns after consuming the only armed firing. Because fixed-rate tasks have delayAfter == 0, once() does not arm another firing when the current run finishes, so the periodic task silently stops forever while its future remains incomplete. Rearm the next fixed-rate firing after the active run completes when an overlap is skipped.
Useful? React with 👍 / 👎.
| if (!ran.compareAndSet(false, true) || outcome.get() != RUNNING) { | ||
| return; | ||
| } | ||
| runner.set(Thread.currentThread()); |
There was a problem hiding this comment.
If cancel(true) runs after this initial outcome check but before runner.set(...), cancellation succeeds while observing a null runner and therefore interrupts nothing; this thread then publishes itself and executes all of doInBackground() despite having been cancelled before it started. This can allow cancelled work and its external side effects to continue, so recheck the outcome after publishing the runner or otherwise coordinate publication with cancellation.
Useful? React with 👍 / 👎.
| com.codename1.io.Preferences.set(storeKey(key), value); | ||
| List<String> keys = index(); | ||
| if (!keys.contains(key)) { | ||
| keys.add(key); | ||
| saveIndex(keys); |
There was a problem hiding this comment.
When two threads put different keys into the same node, both can save their values and then read the same old index before each appends its own key; the last saveIndex wins and permanently drops the other key from keys() and clear(), leaving an orphaned stored value. The read-modify-write of the per-node index needs a shared synchronization boundary across Preferences instances.
Useful? React with 👍 / 👎.
| StringBuilder all = new StringBuilder(); | ||
| for (String key : keys) { | ||
| all.append(key).append('\n'); | ||
| } |
There was a problem hiding this comment.
Java preference keys may legally contain newline characters, but writing each key verbatim with \n as the index delimiter splits such a key into multiple bogus entries. After put("a\nb", value), keys() reports a and b, while remove and clear cannot find the actual stored key, so its value persists indefinitely; escape or length-prefix keys in the index representation.
Useful? React with 👍 / 👎.
| boolean tab = LinuxNative.editTabbed(editPeer) != 0 | ||
| && LinuxWindowManager.slotForComponent(cmp) == MAIN_WINDOW_SLOT; | ||
| hideTextEditor(); | ||
| if (tab) { | ||
| keyPressed(KEY_CODE_TAB); |
There was a problem hiding this comment.
When the GTK editor reports editTabbed(...) == 2 for Shift-Tab, this collapses that value to a boolean and synthesizes an ordinary Tab event. The editor consumed the original event, so Codename One never recorded the Shift modifier and Form.desktopKeyPressed() moves focus forward instead of backward. Preserve the returned direction when forwarding the traversal request.
Useful? React with 👍 / 👎.
…rker fixes from review - The Linux port keeps the direction of the Tab that ended a native edit, so Shift-Tab moves focus backwards. - File.setWritable(false), setReadable(false) and setReadOnly() answer false: a device cannot take a permission away. - Preferences escapes line breaks in its key index and serializes index updates across instances. - SwingWorker rechecks cancellation after publishing its thread. - A fixed-rate schedule outlives an overrun (test; it already did). - The hello suite's native build gets 45 minutes: it compiles 5,400 translation units and one healthy build was stopped at 97%. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 364b9f049b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private static boolean declares(File main, String name) { | ||
| String path = name.replace('.', '/'); | ||
| return new File(main, "java/" + path + ".java").isFile() || new File(main, "kotlin/" + path + ".kt").isFile(); |
There was a problem hiding this comment.
A Kotlin filename does not determine the classes it declares, so a valid source such as com/foo/MyApp.kt containing only top-level helpers compiles to MyAppKt, but this check reports a collision with the generated com.foo.MyApp class and aborts the import. Conversely, a differently named Kotlin file can actually declare MyApp and bypass this validation. Parse the package and top-level declarations before deciding that the generated main name is occupied.
Useful? React with 👍 / 👎.
| + "(?:\\[\\s*\\]\\s*\\w+|\\.\\.\\.\\s*\\w+|\\s+\\w+\\s*\\[\\s*\\])\\s*\\)"); | ||
| private static final Pattern KOTLIN_EXTENDS = Pattern.compile( | ||
| "\\bclass\\s+(\\w+)[^{\\n]*?:[^{\\n]*?((?:javafx\\.application\\.)?Application)\\s*\\("); | ||
| private static final Pattern KOTLIN_STATIC_MAIN = Pattern.compile("@JvmStatic\\s+fun\\s+main\\s*\\("); |
There was a problem hiding this comment.
A valid Kotlin launcher written as @JvmStatic public fun main() inside an object or companion is missed because this expression requires fun immediately after the annotation. With conventional indentation it also cannot match the top-level-main expression, so a project whose only launcher spells out public is rejected as having nothing to start. Allow the valid Kotlin modifiers between @JvmStatic and fun.
Useful? React with 👍 / 👎.
| public Stream<T> sorted(Comparator<? super T> comparator) { | ||
| Comparator<? super T> order = comparator == null ? JdkFunctions.<T>natural() : comparator; | ||
| Iterator<T> up = take(); |
There was a problem hiding this comment.
When application code calls stream.sorted(null), the JDK overload taking a comparator throws NullPointerException, but this silently switches to natural ordering. That can conceal a missing comparator and either produce differently ordered results or defer the failure until elements happen not to be mutually comparable; require a non-null comparator here, while the no-argument overload can continue supplying the natural comparator explicitly.
Useful? React with 👍 / 👎.
| if (used == 0 || used + d.width <= avail) { | ||
| if (used > 0) { | ||
| used += hgap; |
There was a problem hiding this comment.
When the remaining row width exactly fits the next component but not its preceding hgap, this condition keeps the component on the current row and only adds the gap afterward. For example, with 10 pixels remaining and a 10-pixel component plus a 5-pixel gap, used exceeds avail, so the component is laid out beyond the container edge instead of wrapping as Swing's FlowLayout does. Include hgap in the fit check whenever the row is nonempty.
Useful? React with 👍 / 👎.
| String s = nm.trim(); | ||
| int radix = 10; | ||
| if (s.startsWith("0x") || s.startsWith("0X")) { |
There was a problem hiding this comment.
The JDK accepts a sign before a radix prefix, so valid calls such as Color.decode("-0x1") and Color.decode("+0xFF0000") work on desktop but fail here because the prefix is checked before the sign. Conversely, parsing through Long and then casting silently accepts and truncates out-of-range inputs such as 0x100000000, which the JDK rejects. Parse the optional sign and prefix together while enforcing the signed 32-bit range.
Useful? React with 👍 / 👎.
| if (isWhole(n) && isWhole(number)) { | ||
| long a = n.longValue(); | ||
| long b = number.longValue(); | ||
| c = a < b ? -1 : a > b ? 1 : 0; | ||
| } else { | ||
| c = Double.compare(n.doubleValue(), number.doubleValue()); |
There was a problem hiding this comment.
When a number filter receives types such as BigInteger or BigDecimal, this converts both operands to double, collapsing distinct high-precision values. For example, filtering for equality with BigInteger("9007199254740992") incorrectly includes 9007199254740993, whereas Swing compares same-class Comparable numbers exactly. Preserve the number type's comparison before falling back to cross-type numeric conversion.
Useful? React with 👍 / 👎.
| public void transform(Point2D[] ptSrc, int srcOff, Point2D[] ptDst, int dstOff, int numPts) { | ||
| for (int i = 0; i < numPts; i++) { | ||
| Point2D src = ptSrc[srcOff + i]; | ||
| ptDst[dstOff + i] = transform(src, ptDst[dstOff + i]); |
There was a problem hiding this comment.
When ptSrc and ptDst are the same array and the destination range begins inside the source range, this forward loop overwrites a later source point before reading it. For example, translating two points from offset 0 to offset 1 makes the second iteration transform the already-transformed first point instead of the original second point. The JDK permits overlapping arrays here, so copy the source range or iterate in the safe direction as the primitive overloads already do.
Useful? React with 👍 / 👎.
| public PathIterator getPathIterator(AffineTransform at) { | ||
| SegmentIterator it = new SegmentIterator(PathIterator.WIND_NON_ZERO, at, 6); | ||
| double x = getX(); | ||
| double y = getY(); | ||
| double w = getWidth(); |
There was a problem hiding this comment.
When a rectangle has a negative width or height, the JDK's path iterator is immediately empty, but this always enqueues an outline. A common case is the negative-size rectangle returned by createIntersection for disjoint inputs; rendering or flattening that result therefore produces an inverted rectangle instead of nothing. Return the untouched iterator for negative dimensions; the unconditional path construction in Ellipse2D and RoundRectangle2D has the same issue.
Useful? React with 👍 / 👎.
| public void reload() { | ||
| instances.clear(); | ||
| next = 0; | ||
| entries = null; |
There was a problem hiding this comment.
If reload() is called while an iterator obtained from this loader is still in use, the JDK requires that iterator to throw ConcurrentModificationException. Here it instead observes the cleared instance cache and reset shared next index, so it can silently restart discovery and instantiate an already-consumed provider again or mix the old iteration with the new provider set. Track a reload generation in each iterator and reject further hasNext() or next() calls after it changes.
Useful? React with 👍 / 👎.
…pty shapes and reload fixes from review - The importer decides whether the generated main class name is taken from what Kotlin sources declare, not from their file names, and accepts @JvmStatic public fun main. - Stream.sorted(null) throws, as the JDK does. - Color.decode reads a sign before the radix prefix and rejects values past a signed 32-bit int. - RowFilter compares two numbers of one class as that class does. - Rectangle2D, Ellipse2D and RoundRectangle2D of negative size have no outline. - An iterator taken before ServiceLoader.reload refuses to go on. - FlowLayout's wrap test and AffineTransform's Point2D[] overload stay as the JDK has them; the reasons are in comments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Validate encoded-image slices before native pointer arithmetic
When Image.createImage(bytes, offset, len) reaches this entry point, neither offset nor offset + length is checked against the Java array length before the pointer is passed to g_memory_input_stream_new_from_data. An invalid but ordinary slice such as an offset near the end with an oversized length therefore makes GdkPixbuf read beyond the managed array instead of producing a Java bounds exception, potentially crashing the native Linux process; validate the complete range before forming bytes + offset.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public void clearSelection() { | ||
| removeSelectionIntervalImpl(minIndex, maxIndex, false); |
There was a problem hiding this comment.
When the model is already empty, minIndex is Integer.MAX_VALUE and maxIndex is -1; passing those sentinels to removeSelectionIntervalImpl makes changeSelection iterate from -1 through Integer.MAX_VALUE, after which the loop counter overflows and the loop never terminates. Thus common calls such as JTable.setSelectionMode(...), JList.setSelectedIndices(...), or a second clearSelection() on an empty selection can freeze the event thread; return immediately when isSelectionEmpty() is true.
Useful? React with 👍 / 👎.
| public void setDelay(int delay) { | ||
| if (delay < 0) { | ||
| throw new IllegalArgumentException("Invalid delay: " + delay); | ||
| } | ||
| this.delay = delay; |
There was a problem hiding this comment.
When setDelay is called after start(), this only updates the field while the active java.util.Timer remains scheduled with the period captured by schedule(...). Swing applies the new delay to subsequent firings (after any already-scheduled firing), so code that slows down or speeds up an active animation or polling timer continues forever at its original rate here unless it explicitly restarts the timer.
Useful? React with 👍 / 👎.
|
|
||
| /// Runs `action` once `millis` have passed. | ||
| static Handle after(long millis, Runnable action) { | ||
| Handle handle = new Handle(System.currentTimeMillis() + Math.max(0, millis), action); |
There was a problem hiding this comment.
When a timeout converts to a very large millisecond value, adding it to System.currentTimeMillis() overflows to a negative deadline, which next() treats as already due. For example, new CompletableFuture<>().orTimeout(Long.MAX_VALUE, TimeUnit.MILLISECONDS) completes exceptionally almost immediately instead of remaining pending, and scheduled work with the same delay also runs immediately; clamp the deadline or calculate elapsed durations without overflowing.
Useful? React with 👍 / 👎.
| selectedFiles = new File[files.length]; | ||
| System.arraycopy(files, 0, selectedFiles, 0, files.length); | ||
| setSelectedFile(selectedFiles[0]); |
There was a problem hiding this comment.
setSelectedFiles mutates the array and delegates only to setSelectedFile, so listeners registered for SELECTED_FILES_CHANGED_PROPERTY receive no event when the multi-selection changes. File chooser accessories and controllers commonly listen to that bound property; changing from {a, b} to {c, d} can therefore leave them showing the old selection even though getSelectedFiles() has changed, so preserve the old array and fire the selected-files property change.
Useful? React with 👍 / 👎.
| @Override | ||
| public V setValue(V value) { | ||
| V old = entry.setValue(value); | ||
| lastValue = value; |
There was a problem hiding this comment.
An entry returned by the iterator remains usable after next() advances, but its setValue writes this iterator-wide lastValue. For example, after e1 = it.next(); e2 = it.next(); e1.setValue(v); it.remove(), the backing iterator removes e2 while the emitted change reports e2's key with v as the removed value, so bindings receive a corrupt MapChange; keep each wrapper's value separate from the metadata used for removal of the current entry.
Useful? React with 👍 / 👎.
…d-files event and map removal fixes from review - javax.swing.Timer.setDelay reschedules a running timer: the firing already due keeps its time and the new period applies after it. - Delays clamps a delay too long to add to the clock to the end of time instead of wrapping into the past and firing at once. - JFileChooser.setSelectedFiles fires SELECTED_FILES_CHANGED_PROPERTY, and the chooser's own selections go through it. - ObservableMapWrapper's entry iterator reads the removed entry's value at removal, so setValue on an earlier entry no longer changes it. - Linux createImage(byte[], int, int) rejects a range outside the array in Java and in the native decoder. - DefaultListSelectionModel.clearSelection: comment on why an empty selection never loops (the -1 end returns at once, as in the JDK). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5eea8e1241
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!running.compareAndSet(false, true)) { | ||
| // The run before this one has not ended; a fixed rate does | ||
| // not pile runs up. Skipping it does not end the schedule: | ||
| // the next firing was armed above, before this check, so an | ||
| // overrun drops only the firings that fell inside it. | ||
| return; |
There was a problem hiding this comment.
When a fixed-rate task runs longer than its period, this branch discards every firing that occurs while the previous invocation is active. The new re-arming at lines 95–99 keeps the schedule alive, but it still differs from scheduleAtFixedRate, whose overdue executions start late and sequentially rather than being omitted; for example, a 250 ms task with a 100 ms period runs again around 250 ms on the JDK but waits until the 300 ms firing here, permanently losing occurrences and reducing polling or update rates.
Useful? React with 👍 / 👎.
| public static ScheduledExecutorService newScheduledThreadPool(int corePoolSize) { | ||
| if (corePoolSize < 0) { | ||
| throw new IllegalArgumentException(); | ||
| } | ||
| return new ScheduledPool(new Pool(Math.max(1, corePoolSize))); |
There was a problem hiding this comment.
Executors.newScheduledThreadPool(0) is required to reject the non-positive core size with IllegalArgumentException, but this check accepts zero and silently creates a one-worker pool via Math.max(1, corePoolSize). Invalid configuration therefore appears successful and scheduled work runs with behavior different from the desktop JDK; the ThreadFactory overload inherits the same behavior.
Useful? React with 👍 / 👎.
| V value; | ||
| try { | ||
| value = work.call(); |
There was a problem hiding this comment.
When a firing has been handed to a busy underlying pool but its wrapper has not begun, cancel(...) successfully cancels only outcome; once the worker becomes available, this call still invokes the user's callable unconditionally. A scheduled future can therefore report isCancelled() == true while its supposedly cancelled task subsequently starts and performs external side effects; check cancellation before invoking work or retain a cancellable handle to the submitted pool task.
Useful? React with 👍 / 👎.
| public static double sinh(double x) { | ||
| return (MathUtil.exp(x) - MathUtil.exp(-x)) / 2; |
There was a problem hiding this comment.
For small magnitudes such as 1e-20, both exponential terms round to 1.0, so this subtraction returns exactly zero even though Math.sinh(1e-20) is approximately 1e-20. Because desktop Math.sinh calls are remapped here, numerical code using small signals silently loses them; use a stable small-argument formulation rather than subtracting two nearly equal exponentials.
Useful? React with 👍 / 👎.
| if (newObservable == null) { | ||
| throw new NullPointerException("Cannot bind to null"); | ||
| } | ||
| if (!newObservable.equals(observable)) { |
There was a problem hiding this comment.
When a property is already bound and bind is called with a distinct ObservableValue whose equals method considers it equal to the old source, this condition skips the rebind and leaves the property listening to the old object. JavaFX binding source changes are identity-based because ObservableValue defines no equality semantics, so custom value wrappers that compare by content can otherwise keep delivering updates from the wrong source; the primitive and string property bases repeat this check.
Useful? React with 👍 / 👎.
| model = newModel; | ||
| if (newModel != null) { | ||
| newModel.addChangeListener(bridge); | ||
| } |
There was a problem hiding this comment.
After setModel replaces the bounded-range model, this method updates the listener wiring but never fires the bound model property change that Swing emits. Code such as property-change-driven controllers therefore continues to observe or configure the old model even though getModel() returns the replacement; preserve the old value and call firePropertyChange("model", old, newModel) after assignment.
Useful? React with 👍 / 👎.
| static Interpolator spline(double x1, double y1, double x2, double y2) { | ||
| if (!(x1 >= 0 && x1 <= 1 && y1 >= 0 && y1 <= 1 && x2 >= 0 && x2 <= 1 && y2 >= 0 && y2 <= 1)) { | ||
| throw new IllegalArgumentException("Control point coordinates must all be in range [0,1]"); | ||
| } |
There was a problem hiding this comment.
JavaFX constrains only the two x coordinates of Interpolator.SPLINE to [0,1]; y coordinates may lie outside that range to create overshoot or anticipation curves. This validation rejects valid interpolators such as SPLINE(0.2, 1.5, 0.8, -0.5), so ported animations using those curves fail during construction instead of running.
Useful? React with 👍 / 👎.
…mall-argument sinh/tanh from review - ScheduledPool: a fixed rate's firings that come during an overrun are owed and run late, one after another, as the JDK's scheduleAtFixedRate does, instead of being dropped. - ScheduledPool: work cancelled while it waited for a worker no longer starts once it gets one. - JdkNumbers.sinh/tanh: a Taylor series below |x| = 1, so a small argument is no longer cancelled to zero. - Code comments on four findings that contradict the JDK/OpenJFX: newScheduledThreadPool(0) is accepted, PropertyBase.bind compares by equals, JProgressBar.setModel fires no "model" event, and Interpolator.SPLINE checks the y coordinates too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca2f3a31b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| List<LocalDate> dates = new ArrayList<LocalDate>(); | ||
| for (long day = from; day < to; day++) { | ||
| dates.add(LocalDate.ofEpochDay(day)); |
There was a problem hiding this comment.
LocalDate.datesUntil is a lazy stream, but this loop materializes every intervening date before returning. A call such as start.datesUntil(LocalDate.MAX).limit(1) should produce one value promptly; the remapped implementation instead attempts to allocate billions of LocalDate objects and can hang or exhaust memory before limit runs. Generate dates from the epoch-day iterator as the stream is consumed.
Useful? React with 👍 / 👎.
| List<MatchResult> all = new ArrayList<MatchResult>(); | ||
| while (find()) { | ||
| all.add(toMatchResult()); |
There was a problem hiding this comment.
Matcher.results() must expose subsequent find() operations lazily, but this immediately exhausts the matcher and stores every result. Consequently, matcher.results().limit(1) still scans and allocates snapshots for the entire input, and code that creates the stream before performing another matcher operation observes different matcher state than on the JDK. Back the stream with an iterator that calls find() only when consumed.
Useful? React with 👍 / 👎.
| public static LocalDate plusWeeks(LocalDate date, long weeks) { | ||
| return date.plusDays(weeks * 7L); |
There was a problem hiding this comment.
For sufficiently large week counts, multiplying by seven wraps before plusDays sees the value, whereas LocalDate.plusWeeks uses exact multiplication and throws on overflow. For example, plusWeeks(LocalDate.of(2024, 1, 1), 7905747460161236407L) scales to 1 after overflow here and incorrectly returns January 2 instead of failing; use an exact multiply before delegating.
Useful? React with 👍 / 👎.
| for (Future<T> future : futures) { | ||
| try { | ||
| future.get(); |
There was a problem hiding this comment.
If the caller is interrupted while one of these future.get() calls waits, the method propagates InterruptedException without cancelling the remaining futures, even though ExecutorService.invokeAll requires unfinished tasks to be cancelled in that case. The same leak occurs if submission fails partway through the input collection, so tasks can continue performing side effects after invokeAll has exited abnormally; cancel every accumulated future from a finally block unless the full wait completes.
Useful? React with 👍 / 👎.
| if (v < from) { | ||
| shifted.add(Integer.valueOf(v)); | ||
| } else if (v >= from + removed) { | ||
| shifted.add(Integer.valueOf(v - removed + added)); | ||
| } |
There was a problem hiding this comment.
For a replacement change, every selected index inside the removed range is dropped even when an added row occupies the same position. Thus items.set(i, replacement) on a ListView or TableView with row i selected unexpectedly clears the selection instead of retaining index i and updating selectedItem to the replacement; preserve still-valid positions within one-for-one replacement ranges.
Useful? React with 👍 / 👎.
| if (!from.renameTo(to)) { | ||
| if (!from.isDirectory()) { | ||
| throw new IOException("Could not move " + source + " to " + target); | ||
| } |
There was a problem hiding this comment.
When renameTo fails for a regular file—for example, when moving between device storage volumes—this throws immediately, while the directory path below falls back to copy-and-delete and the method's contract promises the same fallback wherever an in-place rename is unavailable. Ordinary Files.move calls therefore fail solely because the source is a file rather than a directory; transfer the file to the target and delete the source when the non-atomic rename cannot be performed.
Useful? React with 👍 / 👎.
…vokeAll cancellation and in-place row replacement from review - LocalDate.datesUntil and Matcher.results are lazy streams, as in the JDK: limit(1) makes one date / finds one match. - plusWeeks/minusWeeks (LocalDate, LocalDateTime, ZonedDateTime) throw on a week count whose days overflow instead of wrapping. - invokeAll cancels the unfinished tasks when it exits early; a Future's get no longer starts waiting on an interrupted thread. - RowSelection keeps a row selected when items.set replaces it in place; setAll still clears the selection, as in JavaFX. - Files.move: comment on why a regular file needs no copy fallback (File.renameTo already copies and deletes outside its directory). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ain class preflight The desktop remap writes the application's main class into the classes directory with its own SourceFile marker, so no source file backs it and master's preflight reported it as a stale class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Compared 181 screenshots: 181 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
@codex review |
1 participant
What this adds
An existing Swing or JavaFX project builds as a Codename One application for every target, the way an Android module does with the Android compatibility layer. The sources are imported with
cn1:import-desktop-projectintocommon/src/main/desktopand built unmodified.maven/swing-compat:java.awt, Java2D,javax.swing,java.beans,javax.imageio, a SwingX subset and the FlatLaf entry points, written over Codename One components. Authored already relocated, because the JDK owns those package names; applications compile against the JDK's Swing.maven/javafx-compat: thejavafx.*subset (scene graph, controls, charts, layout panes, properties and bindings, animation, effects, Canvas), authored under the real names.maven/fxml-compiler: FXML and JavaFX CSS compiled at build time; nothing on a device parses XML or reflects.maven/compat-jdk: the JDK classes a device lacks (streams,java.util.function,java.nio.file, regex,CompletableFuture,java.timegaps and more), shared with the Android layer.maven/build-engine: one composed relocation pass over every layer, generated entry points and registries, records and pattern switches desugared, and the compliance check extended so that unsupported API is a build error reported under its original name and source line.scripts/desktop-compat-samples), screenshot tests inhellocodenameone, an integration test, a real-port interaction harness (maven/integration-tests/desktop-compat-realport) and a benchmark harness (scripts/desktop-compat-benchmarks).docs/developer-guide/Desktop-Interop.asciidoc, with measured size, memory and start-up tables.Verified locally
Not verified / known gaps
desktop-compat.ymladditions,desktop-compat-benchmarks.yml) have never run on GitHub. Windows is unmeasured; macOS runtime numbers needrun-macos-bench.sh.DesktopCompat*tests have to be seeded from the first CI run.🤖 Generated with Claude Code