Fix silent corruption, data loss and crashes in the editor UI

.github/probe/JideResolutionProbe.java

@@ -0,0 +1,33 @@
+/**
+ * Runs the look-and-feel resolution that jide-oss performs inside every JIDE component, against
+ * a built jar, and exits non-zero if it fails.
+ *
+ * LookAndFeelFactory decides which style to install by asking
+ * "lnf instanceof com.sun.java.swing.plaf.windows.WindowsLookAndFeel". An instanceof must
+ * resolve its class before it can answer false, so an absent or inaccessible class is a link
+ * error thrown out of a constructor rather than a quiet no. The application reaches this
+ * whenever someone types into a combo box in a sheet: EditorComboBox installs a
+ * ComboBoxSearchable, its search popup is a JidePopup, and JidePopup.updateUI() - which runs
+ * from the JComponent constructor - calls installJideExtension().
+ *
+ * The look and feel is set to FlatLaf first because that is the one the application installs,
+ * and because it is the case that matters: JIDE recognises Metal and Aqua and answers from an
+ * earlier branch, never reaching the instanceof. A probe left on the default look and feel
+ * passes whether or not the class is present, which is no check at all.
+ */
+public class JideResolutionProbe
+{
+    public static void main(String[] args)
+    {
+        System.setProperty("java.awt.headless", "true");
+        try {
+            javax.swing.UIManager.setLookAndFeel("com.formdev.flatlaf.FlatDarculaLaf");
+            com.jidesoft.plaf.LookAndFeelFactory.installJideExtension();
+        }
+        catch (Throwable t) {
+            System.out.println("FAIL: " + t.getClass().getName() + ": " + t.getMessage());
+            System.exit(1);
+        }
+        System.out.println("OK: WindowsLookAndFeel resolved and the JIDE extension installed");
+    }
+}

.github/probe/manifest_value.py

@@ -0,0 +1,29 @@
+"""Reads one main-section attribute from a jar manifest on stdin.
+
+A manifest is wrapped to 72 bytes and continued with a leading single space, and the break can
+fall in the middle of a token: the Add-Exports value this repository ships spans three lines and
+currently splits java.desktop/sun.awt.shell across two of them. Which tokens get split moves
+whenever the value changes, so reading the raw file is unreliable in both directions -- a package
+that is present can be unfindable, and splitting one yields a garbage token rather than an
+obvious error. Unfolding first is the only way to read the value as the JVM does.
+
+Only the main section is considered: per-entry sections follow the first blank line and may
+repeat attribute names.
+"""
+import sys
+
+name = sys.argv[1]
+text = sys.stdin.buffer.read().decode("utf-8", "replace").replace("\r\n", "\n")
+main_section = text.split("\n\n", 1)[0]
+
+unfolded = []
+for line in main_section.split("\n"):
+    if line.startswith(" ") and unfolded:
+        unfolded[-1] += line[1:]
+    else:
+        unfolded.append(line)
+
+for line in unfolded:
+    if line.startswith(name + ":"):
+        print(line.split(":", 1)[1].strip())
+        break

.github/workflows/build.yml

@@ -0,0 +1,200 @@
+name: Build
+
+on:
+  push:
+    branches: ['**']
+  pull_request:
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Set up JDK
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          # This module compiles at source/target 20.
+          java-version: '21'
+          cache: maven
+
+      # None of the three sibling libraries are on Maven Central at the versions this module
+      # depends on, so each is built from source. A branch of the same name wins when one
+      # exists, because a change spanning several of these repositories is developed on
+      # matching branches; otherwise main. Order matters -- Nds4j underpins the other two.
+      - name: Build upstream libraries
+        run: |
+          set -euo pipefail
+          ref="${{ github.head_ref || github.ref_name }}"
+          for repo in Nds4j Nds4j-ToolUI PokEditor-Core; do
+            target="$ref"
+            if ! git ls-remote --exit-code --heads "https://github.com/turtleisaac/$repo.git" "$ref" >/dev/null 2>&1; then
+              target=main
+            fi
+            echo "::group::$repo @ $target"
+            git clone --quiet --depth 1 --branch "$target" \
+              "https://github.com/turtleisaac/$repo.git" ".upstream/$repo"
+            mvn -B -ntp -q install -DskipTests -f ".upstream/$repo/pom.xml"
+            echo "::endgroup::"
+          done
+
+      # Attributed up front, so an unresolvable dependency is reported as itself rather than
+      # surfacing later as an unexplained compilation failure.
+      - name: Verify dependencies resolve
+        run: |
+          set +e
+          output=$(mvn -B -ntp dependency:resolve 2>&1)
+          status=$?
+          set -e
+          if [ "$status" -ne 0 ]; then
+            echo "$output" | tail -40
+            if echo "$output" | grep -q "VariableTracker"; then
+              echo "::error title=Unpublished dependency blocks this build::io.github.turtleisaac:VariableTracker:1.0-SNAPSHOT is not published to any repository this build can reach, and pom.xml declares no <repositories> entry for it. PokEditor therefore cannot be built from a clean checkout -- not by CI, and not by a new contributor. Fix by publishing VariableTracker, adding a <repositories> entry that serves it, or vendoring it into this repository."
+            fi
+            exit "$status"
+          fi
+
+      - name: Build and test
+        # Everything except the dead-code specifications below. This job must stay green: a
+        # failure here is a regression, and it is only readable as one because the known
+        # failures are not mixed in with it.
+        run: mvn -B -ntp verify -Djava.awt.headless=true -DexcludedGroups=dead-code
+
+      # Tests asserting properties that framework/ and gui_old/ classes do not hold. Those
+      # classes have no callers anywhere in src/main, so the failures are not defects anyone
+      # can hit - they are the specification for whoever revives or deletes the code.
+      #
+      # The count is asserted rather than reported. A bare "expected to fail" job is decorative:
+      # nothing notices when the number goes up, so a genuine regression could be silenced by
+      # tagging it, and nothing notices when it goes down either, so a fix never prompts anyone
+      # to remove the tag. Pinning the number makes both show up as a build failure.
+      - name: Known dead-code failures
+        run: |
+          set +e
+          output=$(mvn -B -ntp test -Djava.awt.headless=true -Dgroups=dead-code 2>&1)
+          set -e
+          summary=$(echo "$output" | grep -E '^\[(ERROR|INFO|WARNING)\] Tests run: [0-9]+, Failures' | tail -1)
+          echo "$summary"
+          run=$(echo "$summary" | sed -E 's/.*Tests run: ([0-9]+).*/\1/')
+          failures=$(echo "$summary" | sed -E 's/.*Failures: ([0-9]+).*/\1/')
+          errors=$(echo "$summary" | sed -E 's/.*Errors: ([0-9]+).*/\1/')
+          red=$(( failures + errors ))
+          if [ "$run" != "$EXPECTED_TAGGED" ]; then
+            echo "::error title=Tagged test count changed::$run tests carry the dead-code tag, expected $EXPECTED_TAGGED. If you added a tag, say why in the commit and update EXPECTED_TAGGED; a regression must not be tagged away."
+            exit 1
+          fi
+          if [ "$red" -lt "$EXPECTED_RED" ]; then
+            echo "::notice title=A known failure now passes::$red of $run tagged tests fail, down from $EXPECTED_RED. Something was fixed -- remove its @Tag(DEAD_CODE) and lower EXPECTED_RED."
+            exit 1
+          fi
+          if [ "$red" -gt "$EXPECTED_RED" ]; then
+            echo "::error title=More tagged tests failing::$red of $run tagged tests fail, up from $EXPECTED_RED."
+            exit 1
+          fi
+          echo "$red of $run tagged tests fail, as expected."
+        env:
+          # Keep in step with the @Tag(DEAD_CODE) annotations in src/test.
+          EXPECTED_TAGGED: '17'
+          EXPECTED_RED: '17'
+
+      # The jar handed to testers and attached to releases. It is built here because it was
+      # previously assembled by hand outside the repository, which is how it came to ship
+      # without the two look-and-feel fixes below.
+      - name: Build the runnable jar
+        run: mvn -B -ntp -Pdist package -DskipTests
+
+      # Building it is not the same as checking it. Shade succeeds whether or not the manifest
+      # carries Add-Exports and whether or not WinLaF's classes made it in, so a build-only step
+      # would go green on exactly the artifact that crashes for a user. Both halves are executed
+      # here, and the second is executed against a deliberate negative so the check cannot
+      # quietly become decorative.
+      - name: Verify the runnable jar
+        run: |
+          set -euo pipefail
+          jar=target/PokEditor-3.2.0-dist.jar
+          javac -cp "$jar" -d target/probe .github/probe/JideResolutionProbe.java
+
+          # Continuation lines in a manifest begin with a single space and the break can fall
+          # mid-token: this value spans three lines and currently splits sun.awt.shell across
+          # two. Which tokens split moves with the value, so the raw file has to be unfolded --
+          # otherwise a present package can read as absent, and a split one becomes a garbage
+          # --add-exports flag rather than an error.
+          exports=$(unzip -p "$jar" META-INF/MANIFEST.MF | python3 .github/probe/manifest_value.py Add-Exports)
+          main=$(unzip -p "$jar" META-INF/MANIFEST.MF | python3 .github/probe/manifest_value.py Main-Class)
+          echo "Main-Class:  $main"
+          echo "Add-Exports: $exports"
+
+          if [ "$main" != "io.github.turtleisaac.pokeditor.Main" ]; then
+            echo "::error title=The jar is not runnable::Main-Class is '$main'. Launching it with java -jar will fail."
+            exit 1
+          fi
+
+          # 1. This runner's own conditions. No JDK outside Windows ships
+          #    com.sun.java.swing.plaf.windows, so the system-scoped WinLaF.jar is the only thing
+          #    satisfying the resolution -- and the shade plugin drops system scope unless the
+          #    pom unpacks it. Nothing in src/main names that class, so this step is what stands
+          #    between someone deleting WinLaF.jar as dead weight and a user hitting
+          #    NoClassDefFoundError on their first keystroke in a sheet.
+          if ! java -cp "$jar:target/probe" JideResolutionProbe; then
+            echo "::error title=WinLaF classes are missing from the jar::The look-and-feel class jide-oss resolves is absent, so building any JIDE component dies with NoClassDefFoundError. The dist profile unpacks WinLaF.jar because <scope>system</scope> is neither compile nor runtime and the shade plugin resolves neither."
+            exit 1
+          fi
+
+          # 2. Windows. There the JDK does ship that package, inside java.desktop, which does not
+          #    export it -- so resolution fails with IllegalAccessError no matter what is on the
+          #    classpath, and only the manifest entry helps. The condition is reproduced by
+          #    patching the class into java.desktop so it is present but encapsulated, exactly as
+          #    it is there, and by giving jide-oss an os.version it recognises: its table stops at
+          #    6.2, so on Windows 10 and 11 its own isWindowsVistaAbove() is false and the crash
+          #    arrives by a different branch.
+          #
+          #    The exports the jar itself declares are what get applied, so trimming the manifest
+          #    fails this step. On the command line the JVM warns about the packages java.desktop
+          #    does not have on Linux; in the manifest form the shipped jar uses, it is silent.
+          mkdir -p target/winpatch
+          unzip -qo "$jar" 'com/sun/java/swing/plaf/windows/*' -d target/winpatch
+          flags=""
+          for token in $exports; do flags="$flags --add-exports $token=ALL-UNNAMED"; done
+
+          if ! java -Dos.name="Windows 7" -Dos.version="6.1" \
+                    --patch-module java.desktop=target/winpatch $flags \
+                    -cp "$jar:target/probe" JideResolutionProbe; then
+            echo "::error title=The jar would crash on Windows::The Add-Exports entry in its manifest does not cover com.sun.java.swing.plaf.windows, so typing into a combo box in a sheet throws IllegalAccessError. See the dist profile in pom.xml."
+            exit 1
+          fi
+
+          # The negative. If this passes, the check above proves nothing -- either the simulation
+          # stopped reproducing the Windows condition or the probe stopped reaching the code that
+          # fails, and in both cases the step needs fixing rather than trusting.
+          echo "Negative control (an IllegalAccessError here is the expected result):"
+          if java -Dos.name="Windows 7" -Dos.version="6.1" \
+                  --patch-module java.desktop=target/winpatch \
+                  -cp "$jar:target/probe" JideResolutionProbe >/dev/null 2>&1; then
+            echo "::error title=This step no longer proves anything::The probe passed without any --add-exports, so it is not reaching the resolution that fails on Windows. Fix the check; do not delete it."
+            exit 1
+          fi
+          echo "Verified: resolves on this runner, resolves under the Windows condition, and fails without the manifest entry."
+
+      - name: Upload the runnable jar
+        uses: actions/upload-artifact@v4
+        with:
+          name: PokEditor-jar
+          path: target/PokEditor-3.2.0-dist.jar
+          if-no-files-found: error
+
+      - name: Upload build output
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: build-output
+          path: |
+            **/target/surefire-reports/**
+            **/target/*.log
+          if-no-files-found: ignore

.gitignore

@@ -17,3 +17,6 @@
 /Found/
 /Gen 5 Editing Information/
 /GoogleSheetsAPI/out/
+
+# Maven build output
+/target/

README.md

@@ -26,6 +26,97 @@ Builds are available in the [Releases page here on GitHub](https://github.com/tu
 
 Unlike prior versions of PokEditor, v3 is intended to fully be used within the tool. No more exporting sheets or editing them elsewhere. Additionally, there is very little effort required by the user to get it set up this time around. No more annoying sheets setup process, it should just automatically load everything into the sheets the instant you open a project.
 
+# Building from source
+
+Requires a JDK (the module compiles at source/target 20; CI builds on 21) and Maven.
+
+### 1. Install the sibling libraries first, in this order
+
+None of the three are on Maven Central at the versions this pom pins, so each has to be built
+and installed locally before PokEditor will resolve. Order matters: Nds4j underpins the other
+two.
+
+```
+git clone https://github.com/turtleisaac/Nds4j.git          && mvn -f Nds4j/pom.xml          install -DskipTests
+git clone https://github.com/turtleisaac/Nds4j-ToolUI.git   && mvn -f Nds4j-ToolUI/pom.xml   install -DskipTests
+git clone https://github.com/turtleisaac/PokEditor-Core.git && mvn -f PokEditor-Core/pom.xml install -DskipTests
+```
+
+When a change spans several of these repositories it is developed on branches of the same name
+in each, so check out matching branches before installing. CI does this automatically, falling
+back to `main` where no matching branch exists.
+
+> **A clean checkout does not build yet.** `pom.xml` also declares
+> `io.github.turtleisaac:VariableTracker:1.0-SNAPSHOT`, which is published nowhere the build can
+> reach and has no `<repositories>` entry. The field script editor is disabled, but
+> `ScriptDocument` still imports the library, so it is still needed to compile. See
+> `TECH_DEBT.md`.
+
+### 2. Build the runnable jar
+
+```
+mvn clean -Pdist package
+```
+
+This produces **`target/PokEditor-3.2.0-dist.jar`**, which is the artifact to hand to a tester or
+attach to a release — every dependency in one archive, launched with `java -jar`. Add
+`-DskipTests` to skip the suite.
+
+The plain `mvn package` deliberately does **not** produce it: `target/PokEditor-3.2.0.jar` holds
+this module's classes without its dependencies and is not runnable on its own. `dist` is a
+separate profile because it costs ~17MB of output that nothing in CI consumes. (Under `-Pdist`
+that plain jar also picks up the unpacked `WinLaF` classes as a side effect of the step below.
+Nothing distributes it, so this is harmless — but it is why a `-Pdist` build should start from
+`clean`.)
+
+### Do not hand-assemble the jar
+
+The `dist` profile does two things that are easy to miss, and getting either wrong produces a
+jar that starts fine and then dies the first time someone types into a combo box in a sheet:
+
+- **It writes an `Add-Exports` manifest entry.** jide-oss predates the module system and does
+  `instanceof com.sun.java.swing.plaf.windows.WindowsLookAndFeel` on a path every JIDE component
+  reaches. On Windows that package lives in `java.desktop`, which does not export it, so the
+  check fails with `IllegalAccessError` before it can answer false.
+- **It unpacks `WinLaF.jar`.** That dependency is `<scope>system</scope>`, and the shade plugin
+  resolves only compile and runtime scope, so it is dropped silently. On Linux and macOS no JDK
+  ships that package at all and this copy is the only thing satisfying it, so without it the
+  same code path dies with `NoClassDefFoundError` instead.
+
+Neither substitutes for the other — on Windows, parent-first delegation finds `java.desktop`'s
+copy and shadows `WinLaF.jar` entirely. `TECH_DEBT.md` has the full account.
+
+### CI builds this too
+
+Every push builds the same jar and uploads it as a run artifact named **PokEditor-jar**, so a
+tester can download it from the Actions run rather than waiting for a release. CI also verifies
+it: it runs the look-and-feel resolution described above against the built jar, once under this
+runner's own conditions and once under a simulated Windows one, and then checks that the second
+fails when the manifest entry is withheld — because a check that passes either way would go green
+on exactly the artifact that crashes.
+
+### 3. Run it
+
+```
+java -jar target/PokEditor-3.2.0-dist.jar
+```
+
+### Running the tests
+
+```
+mvn verify -Djava.awt.headless=true -DexcludedGroups=dead-code
+```
+
+This is what CI's `Build` step runs, and it must stay green. The `dead-code` tag marks tests for
+classes with no callers anywhere in `src/main` — they are the specification for whoever revives
+or deletes that code, not defects anyone can hit. They are expected to fail, so omitting
+`-DexcludedGroups` will report failures on a healthy tree.
+
+Those tests run in CI's separate `Known dead-code failures` step, which pins both the tagged
+count and the failure count. Pinning them means a regression cannot be hidden by tagging it, and
+a fix prompts someone to remove the tag — so if you add or fix a tagged test, update
+`EXPECTED_TAGGED` / `EXPECTED_RED` in `.github/workflows/build.yml` and say why in the commit.
+
 # List of Spreadsheet-Based Editors
 
 * Personal Data Editor