Skip to content

Fix __dirname access in server.cli.ts#232411

Merged
Tyriar merged 1 commit intomainfrom
tyriar/230584
Oct 28, 2024
Merged

Fix __dirname access in server.cli.ts#232411
Tyriar merged 1 commit intomainfrom
tyriar/230584

Conversation

@Tyriar
Copy link
Copy Markdown
Contributor

@Tyriar Tyriar commented Oct 28, 2024

Fixes #230584

@bpasero I'm not sure how to verify the server CLI, I just copied over the same fix as cli.ts

@Tyriar Tyriar added this to the November 2024 milestone Oct 28, 2024
@Tyriar Tyriar requested a review from bpasero October 28, 2024 12:48
@Tyriar Tyriar self-assigned this Oct 28, 2024
@bpasero
Copy link
Copy Markdown
Member

bpasero commented Oct 28, 2024

@Tyriar wasn't this fixed already with the change in #231423?

@Tyriar
Copy link
Copy Markdown
Contributor Author

Tyriar commented Oct 28, 2024

Nope, it failed verification

Copy link
Copy Markdown
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

I think aligning makes good sense, but if this still fails, we need to fix as candidate, and I yet have to understand why __dirname would not be defined 🤔

@bpasero
Copy link
Copy Markdown
Member

bpasero commented Oct 28, 2024

I will work on a recovery fix to also address the other issue 👍

@Tyriar Tyriar merged commit 2029af4 into main Oct 28, 2024
@Tyriar Tyriar deleted the tyriar/230584 branch October 28, 2024 18:12
Copy link
Copy Markdown
Contributor

@jamesharris-garmin jamesharris-garmin left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this correctly, I was a bit in over my head on running/testing this locally.

import { DeferredPromise } from '../../base/common/async.js';
import { FileAccess } from '../../base/common/network.js';

const __dirname = dirname(url.fileURLToPath(import.meta.url));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can probably drop this line if it isn't used/useful at all, but that is a different patch.

@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WSL terminal CLI is broken

3 participants