Skip to content

Simplify duplicated workspace-lock error handling in IDEApplication#4140

Open
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:simplify-ide-workspace-lock-error
Open

Simplify duplicated workspace-lock error handling in IDEApplication#4140
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:simplify-ide-workspace-lock-error

Conversation

@vogella

@vogella vogella commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

The "already set" branch of checkInstanceLocation had three identical hideSplash calls and two identical "cannot be set" error dialogs spread across a nested if/else. This collapses them to a single splash hide and one error path, reading the workspace lock details only when the directory actually exists. Behavior is unchanged across all four cases (dev-launch lock, lock info present, no lock info, missing directory). The bundle compiles cleanly.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR simplifies the workspace-lock failure handling in IDEApplication#checkInstanceLocation by collapsing duplicated splash-hiding and error-dialog code paths while keeping the same user-visible outcomes for the existing cases (dev-launch lock reporting, lock info present/absent, and missing workspace directory).

Changes:

  • Consolidated the “workspace cannot be set” error handling into a single path and ensured hideSplash(shell) is called once.
  • Only reads workspace lock details when the workspace directory exists.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +281 to +290
// In dev (PDE) launch mode let the launcher report the locked workspace.
if (workspaceDirectory.exists() && isDevLaunchMode(applicationArguments)) {
return EXIT_WORKSPACE_LOCKED;
}

// If the directory exists and carries lock info, show it; otherwise
// report that the workspace location could not be set.
String lockInfo = workspaceDirectory.exists()
? WorkspaceLock.getWorkspaceLockDetails(instanceLoc.getURL())
: null;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, changed

@vogella vogella force-pushed the simplify-ide-workspace-lock-error branch from 39ec0e7 to 9580995 Compare June 26, 2026 08:32
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Test Results

   855 files  ±0     855 suites  ±0   46m 49s ⏱️ - 10m 4s
 8 083 tests ±0   7 840 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 151 runs  ±0  19 497 ✅ ±0  654 💤 ±0  0 ❌ ±0 

Results for commit 90f1cd3. ± Comparison against base commit 3597fcf.

♻️ This comment has been updated with latest results.

The "already set" branch of checkInstanceLocation repeated hideSplash and
the generic "cannot be set" error dialog across a nested if/else. Collapse
to a single splash hide and one error path; behavior is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants