Skip to content

Conversation

wafuwafu13
Copy link
Contributor

@wafuwafu13 wafuwafu13 commented May 6, 2023

recreate #16064

isFile, isDirectory, isSymlink are defined in Deno.FileInfo, but isBlockDevice, isCharacterDevice, isFIFO, isSocket are not defined.
These are defined in Node.js fs.Stats, and it is good to be defined in Deno.FileInfo to improve compatibility.

@CLAassistant
Copy link

CLAassistant commented May 6, 2023

CLA assistant check
All committers have signed the CLA.

@aapoalas
Copy link
Contributor

aapoalas commented May 6, 2023

Please turn the PR into a draft until it is ready for review / test running.

@wafuwafu13 wafuwafu13 marked this pull request as draft May 6, 2023 22:17
@wafuwafu13 wafuwafu13 marked this pull request as ready for review May 11, 2023 20:36
@wafuwafu13
Copy link
Contributor Author

wafuwafu13 commented May 11, 2023

---- integration::lsp::lsp_completions_auto_import stdout ----
deno_exe path /home/runner/work/deno/deno/target/release/deno
thread 'integration::lsp::lsp_completions_auto_import' panicked at 'assertion failed: `(left == right)`

when add isBlockDevice
https://github.com/denoland/deno/actions/runs/4901413563/jobs/8752670724

<                "exportMapKey": String("foo|6846|file:///a/b"),
>                "exportMapKey": String("foo|6845|file:///a/b"),

when add isBlockDevice, isCharDevice, isFifo, isSocket
https://github.com/denoland/deno/actions/runs/4952421541/jobs/8858694905?pr=19008

<                "exportMapKey": String("foo|6849|file:///a/b"),
>                "exportMapKey": String("foo|6845|file:///a/b"),

I have yet to figure out if this is a fatal change...

@wafuwafu13 wafuwafu13 marked this pull request as draft May 11, 2023 21:37
@wafuwafu13 wafuwafu13 marked this pull request as ready for review May 11, 2023 21:55
@wafuwafu13 wafuwafu13 changed the title feat(ext/fs): add isBlockDevice to FileInfo feat(ext/fs): add isBlockDevice, isCharDevice, isFifo, isSocket to FileInfo May 11, 2023
@bartlomieju
Copy link
Member

---- integration::lsp::lsp_completions_auto_import stdout ----
deno_exe path /home/runner/work/deno/deno/target/release/deno
thread 'integration::lsp::lsp_completions_auto_import' panicked at 'assertion failed: `(left == right)`

when add isBlockDevice https://github.com/denoland/deno/actions/runs/4901413563/jobs/8752670724

<                "exportMapKey": String("foo|6846|file:///a/b"),
>                "exportMapKey": String("foo|6845|file:///a/b"),

when add isBlockDevice, isCharDevice, isFifo, isSocket https://github.com/denoland/deno/actions/runs/4952421541/jobs/8858694905?pr=19008

<                "exportMapKey": String("foo|6849|file:///a/b"),
>                "exportMapKey": String("foo|6845|file:///a/b"),

I have yet to figure out if this is a fatal change...

It's not, it's expected. TypeScript provides completions that can change when new APIs appear. I've rebased the PR.

@bartlomieju bartlomieju added this to the 1.34 milestone May 17, 2023
Copy link
Contributor

@levex levex left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@levex levex left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 114ec3c into denoland:main May 24, 2023
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.

5 participants