Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix missing __dirname in --locate-shell-integration-path#231423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
Tyriar merged 1 commit intomicrosoft:mainfromjamesharris-garmin:fix-server-cli
Oct 15, 2024

Conversation

@jamesharris-garmin
Copy link
Contributor

This patchfixes#230584 by applying the recommended fix from:

https://masteringjs.io/tutorials/node/__dirname-is-not-defined

prasanthcakewalk reacted with thumbs up emoji
Copy link
Member

@TyriarTyriar left a comment

Choose a reason for hiding this comment

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

lgtm@bpasero could you verify this is the right way to do this now?

Copy link
Member

@bpaserobpasero left a comment

Choose a reason for hiding this comment

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

Yes, it is:

const__dirname=path.dirname(fileURLToPath(import.meta.url));

@bpasero
Copy link
Member

But I would not call out "ESM" specifically, this is just the way of doing it now 🤔

@TyriarTyriar merged commit58cab99 intomicrosoft:mainOct 15, 2024
@bpasero
Copy link
Member

bpasero commentedOct 15, 2024•
edited
Loading

Follow up#231427

This is really a good catch and was missed during the ESM migration. We are bitten by the fact that the node.js types declare a global__dirname :-/

Update: I think we can have a ESLint rule to disallow...

Tyriar reacted with thumbs up emoji

@jamesharris-garminjamesharris-garmin deleted the fix-server-cli branchOctober 15, 2024 19:53
@jamesharris-garmin
Copy link
ContributorAuthor

Thanks everyone for the quick turn around on this.

@prasanthcakewalk
Copy link

May not be worth changing anything, but I think the import here:

importpathfrom'path';

is not necessary, sincedirname has already been imported here:
import{dirname,extname,resolve,join}from'../../base/common/path.js';

I think the import line could be removed and
const __dirname = path.dirname(url.fileURLToPath(import.meta.url));
could just be changed to
const __dirname = dirname(url.fileURLToPath(import.meta.url));
I could be wrong though; I'm not a typescript developer.

@bpasero
Copy link
Member

Yes thanks, will address that.

prasanthcakewalk and jamesharris-garmin reacted with thumbs up emoji

NikolaRHristov pushed a commit to CodeEditorLand/Editor that referenced this pull requestOct 16, 2024
…r-cliFix missing __dirname in --locate-shell-integration-path
@vs-code-engineeringvs-code-engineeringbot locked and limited conversation to collaboratorsNov 29, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@bpaserobpaserobpasero approved these changes

@TyriarTyriarTyriar approved these changes

Assignees

@TyriarTyriar

Labels

None yet

Projects

None yet

Milestone

October 2024

Development

Successfully merging this pull request may close these issues.

WSL terminal CLI is broken

4 participants

@jamesharris-garmin@bpasero@prasanthcakewalk@Tyriar
[8]ページ先頭
©2009-2025 Movatter.jp