[Repo Assist] fix: reject promise on phpcbf exit codes 0 and 3 to prevent hang#48
Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
Draft
Conversation
When phpcbf exits with code 0 (nothing to fix), the Promise previously neither resolved nor rejected, causing VS Code's formatter to hang indefinitely. Users saw a spinning "Formatting..." indicator that never cleared, and subsequent saves stopped working until VS Code was restarted. Exit code 3 (general script execution error) had the same problem. Fix: call reject() for both exit codes so VS Code is immediately informed that no document edits are available and can move on. Closes #39 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions bot
added a commit
that referenced
this pull request
Mar 9, 2026
…exit code 0 - Use path.join(TmpDir, ...) instead of TmpDir + "/..." for Windows path separator compatibility (addresses PR #23 from 2018) - Reject the formatter Promise on exit code 0 (no fixable errors found), preventing the VS Code formatting spinner from hanging indefinitely (see issue #39, mirrors PR #41 and PR #48) - Buffer all stdout output and display it as an error message when phpcbf exits with code 3; the previous conditional register of the stdout handler was dead code (phpcbfError was always false at check time) so users never saw the error detail - Replace two separate conditional stdout handler registrations with a single always-registered handler that buffers output and logs to the console in debug mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 9, 2026
Draft
Contributor
Author
|
🤖 This is an automated comment from Repo Assist. This PR is superseded by #49, which includes the same exit code 0/3
I'd suggest the maintainer close this PR and review #49 instead. Thank you to everyone who contributed to identifying the root cause!
|
github-actions bot
added a commit
that referenced
this pull request
Mar 10, 2026
- Replace deprecated async fs.exists() with synchronous fs.existsSync() in addRootPath(), eliminating the race condition where executablePath would be updated after cp.spawn() had already used the old value. Supersedes PR #51. - Use path.join() for temp file construction instead of string concat with a hard-coded slash, fixing the double-slash on macOS when os.tmpdir() returns a path ending in '/'. Supersedes PR #49 (partial). - Remove broken phpcbfError flag pattern. The flag was evaluated synchronously before any exit event fired, so the conditional stdout listener was never attached for exit code 3. Replace with an unconditional stdoutOutput accumulator used inside the exit handler. Supersedes PR #49 (partial). - Reject the promise for exit code 0 (nothing to fix) so VS Code's formatter API is notified immediately instead of hanging until restart. Closes #39. Supersedes PR #48, PR #49. - Show a meaningful error message for exit code 3 using buffered stdout (or a fallback string), then reject so the spinner clears. Previously the error message was silently swallowed. - Clean up temp file in the error handler (exec.on('error')) to prevent orphaned files when the executable is not found. Supersedes PR #58. - Add block scopes around case 1/2 and default to silence no-case-declarations lint errors without disabling the rule. - Remove the unreachable 3: entry from the msgs lookup in the default case, and fall back gracefully for unknown exit codes instead of calling window.showErrorMessage(undefined). Supersedes PR #61. - Use a local const for the resolved standard in getArgs() instead of writing back to this.standard, removing the accidental side effect that could clobber the instance state mid-format. Supersedes PR #60. - Guard onDidChangeConfiguration reload with event.affectsConfiguration('phpcbf') so unrelated setting changes (e.g. font, theme) do not trigger unnecessary reloads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This is an automated PR from Repo Assist, an AI assistant.
Summary
Fixes the formatter hanging indefinitely when
phpcbfreports that the file is already clean.Root cause
When
phpcbfexits with code0(no fixable errors — file already clean), the extension's internalPromiseinsideformat()neither resolved nor rejected. VS Code's formatter API waits for the Promise to settle, which it never did. The result:Exit code
3(general script execution error) had the same unresolved-Promise problem.Fix
Call
reject()for exit codes0and3so VS Code is immediately informed there are no document edits and can proceed normally. Thereject()for code0is semantically correct — a formatter that has nothing to change should signal "no edits" via rejection per the VS Code formatter API contract.This approach mirrors the proposal in the long-standing PR #41 (open since 2020).
Changes
extension.jsreject()forcase 0andcase 3in exit-code handlerTest Status
This extension requires a live VS Code instance to run the test harness and cannot be tested in CI without a display server. The change is a 2-line addition in an existing switch statement with no logic changes for the working exit codes (1 and 2).
Manual testing:
Closes #39