Skip to content

[Repo Assist] fix: use path.join for temp file, fix stdout buffering, reject on exit code 0#49

Draft
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-temp-path-and-error-display-a67ea439b28fb2e8
Draft

[Repo Assist] fix: use path.join for temp file, fix stdout buffering, reject on exit code 0#49
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-temp-path-and-error-display-a67ea439b28fb2e8

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot commented Mar 9, 2026

🤖 This is an automated PR from Repo Assist, an AI assistant.

Summary

Three focused fixes for correctness and Windows compatibility in extension.js.

Changes

1. Fix Windows-incompatible temp file path (addresses PR #23)

Before:

let fileName = TmpDir + "/temp-" + randomPart + ".php";

After:

let fileName = path.join(TmpDir, "temp-" + randomPart + ".php");

On Windows, os.tmpdir() returns a path with backslashes (C:\Users\...\AppData\Local\Temp). Concatenating a hardcoded / produces an invalid mixed-separator path like C:\...\Temp/temp-abc.php. path.join uses the correct separator for the platform. PR #23 (open since 2018) proposed the same fix.

2. Reject formatter Promise on exit code 0 (fixes #39, see also PR #41, PR #48)

When phpcbf exits with code 0 (file is already clean — nothing to fix), the formatter's Promise previously neither resolved nor rejected. VS Code's formatter API waits indefinitely for the Promise to settle, causing the "Formatting…" spinner to run forever and blocking subsequent saves. Calling reject() immediately signals VS Code that there are no edits.

3. Fix dead stdout error handler — show real error messages on exit code 3

Bug: The original code checked if (phpcbfError) synchronously right after constructing the Promise. At that point phpcbfError was always false (the exit event hasn't fired yet), so the stdout error handler was never registered. Users got a silent failure with no indication of what went wrong.

Fix: Buffer all stdout in a single always-registered handler. When exit code 3 fires, display the buffered output as an error message (falling back to a generic message if stdout was empty), then call reject().

This also consolidates the two previous separate exec.stdout.on("data", ...) registrations (one for errors, one for debug mode) into a single handler.

Files Changed

File Change
extension.js path.join for temp file; buffer stdout; reject() on exit code 0; error display + reject() on exit code 3

Relationship to Other Open PRs

Closes #39

Test Status

This extension requires a live VS Code instance and cannot be tested headlessly. The changes are additive/corrective with no logic changes for the working exit codes (1 and 2).

Manual test steps:

  1. Open a PHP file already fully formatted → save → spinner should clear immediately (was: hangs forever)
  2. Trigger phpcbf with a broken config → VS Code should show a real error message (was: silent failure)
  3. Open a PHP file with fixable errors → save → file is correctly updated (exit codes 1/2 unchanged)

Generated by Repo Assist ·

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f

…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>
github-actions bot added a commit that referenced this pull request Mar 9, 2026
Documents the bug fixes and improvements from the open Repo Assist PRs:
- Fix formatter hang on exit code 0 (PR #49, closes #39)
- Fix phpcbf.enable=false bypass (PR #56)
- Fix temp file leak on process error (PR #58)
- Fix onWillSaveTextDocument save timeout (PR #53, closes #35)
- Fix deprecated fs.exists (PR #51, closes #36)
- Fix undefined showErrorMessage on unknown exit code (PR #61)
- Add VS Code output channel for debug/error display (PR #50)
- Refactor getArgs standard variable (PR #60)
- Fix eslintrc and no-case-declarations lint issues

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Indent when saving does not work if phpcbf has nothing to indent

0 participants