mattcasters commented on code in PR #8896:
URL: https://github.com/apache/hop/pull/8896#discussion_r4262800606
##########
plugins/misc/lint/src/main/java/org/apache/hop/lint/LinterConfigPlugin.java:
##########
@@ -221,6 +221,9 @@ public Map<String, Object> saveToHopConfig() {
void applyEnabledState() {
if (!isLinterEnabled()) {
LintResultsManager.getInstance().clearResults();
+ // Without this, switching the linter back on skips every file that has
not changed since it
+ // was last checked, and the project comes back without findings.
+ BackgroundLintService.getInstance().getTracker().clear();
Review Comment:
**[bug]** Clearing the tracker here is not atomic with `publish()`, and
disabling the linter does not bump `projectGeneration`. `publish`
(BackgroundLintService.java line 196) calls `tracker.markChecked` whenever the
generation still matches, then `updateResultsForFile` returns without storing
because the linter is already off. The project pass keeps walking its remaining
files, so every file it finishes after this line is marked checked with no
findings. Turning the linter back on skips those files, which is the gap this
clear is meant to close. `lintGraphInternal` and `lintFileInternal` re-mark the
same way, because they never go through `publish`.
**Suggestion:** After the flag is already false, take the
`LintResultsManager` lock `publish` uses, increment `projectGeneration`, and
clear the tracker in that same section. `publish` should return before
`markChecked` when the generation mismatches or the linter is off.
`switchingTheLinterOffForgetsWhichFilesWereChecked` only marks a file and then
disables; it does not finish a pass after the clear.
##########
plugins/misc/lint/src/main/java/org/apache/hop/lint/BackgroundLintService.java:
##########
@@ -141,11 +149,62 @@ public void scheduleGraphLint(HopGuiAbstractGraph graph,
boolean force) {
});
}
+ /**
+ * Called when a project is enabled. When it is another project than before,
the findings and the
+ * record of what was checked belong to the project that was left: drop
both, and stop the passes
+ * still running for it from publishing.
+ *
+ * <p>The tracker has to go as well. It skips files that have not changed
since they were last
+ * checked, so switching back to a project would otherwise leave it without
findings.
+ */
+ public void switchProject(String projectPath) {
Review Comment:
**[bug]** `switchProject` bumps `projectGeneration` and clears the results,
but only `lintProjectAsync` consults that generation. `lintGraphInternal`
(`markChecked` at line 294) and `lintFileInternal` (line 397) still mark the
file and call `updateResultsForFile` directly. Closing a tab does not cancel a
worker already on the pool: an edit goes through `scheduleGraphLint`, and
opening or saving goes through `scheduleFileLint`. That worker can finish after
`HopGuiProjectAfterEnabled`, which runs only after `closeAllFiles` and
`openLastFiles`, and put the previous project's file back into the list this
method just cleared. Show Lint Results then lists both projects again.
`ExplorerLintGuiPlugin.runLintIntoProblemsTab` / `runLintWithPopup` and the
verify extensions write the manager the same way.
**Suggestion:** Capture `currentProjectGeneration()` when those passes start
and publish through `publish()`, so a stale pass neither stores findings nor
calls `markChecked`. A file lint started by `openLastFiles` before this method
runs carries the old generation and is dropped; `lintProjectAsync` lints the
new project because the tracker was cleared. Gate the Explorer and verify
writers the same way, and add a test that a file lint submitted before
`switchProject` does not reappear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
