Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(intellij): Use node interpreter to run commands #416

Merged
merged 17 commits into from
Sep 30, 2023

Conversation

victor-teles
Copy link
Contributor

@victor-teles victor-teles commented Sep 25, 2023

Summary

Fixes #404 - explanation

For windows the plugin need to use node interpreter to be able to run biome lsp-proxy and biome format

Fix for biome format for #402 - explanation

The biome format command was running without cwd, now it's set to run on projects root

Test Plan

  • UI Testing - Validate list of quickfixes

@victor-teles victor-teles changed the title Fix/intellij binary resolution fix(intellij): use node interpreter to run commands Sep 25, 2023
@victor-teles victor-teles changed the title fix(intellij): use node interpreter to run commands fix(intellij): Use node interpreter to run commands Sep 25, 2023
@victor-teles victor-teles marked this pull request as ready for review September 26, 2023 04:08
@victor-teles victor-teles requested a review from a team September 26, 2023 12:38
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

Thank you @victor-teles ! I would be nice to have a section in the CONTRIBUTING.md that explains a bit the UI testing and how a maintainer can run them locally

@victor-teles
Copy link
Contributor Author

Thank you @victor-teles ! I would be nice to have a section in the CONTRIBUTING.md that explains a bit the UI testing and how a maintainer can run them locally

@ematipico
Thank you for the review

I'll update CONTRIBUTING.md and add the new README.md

@github-actions github-actions bot added the A-Changelog Area: changelog label Sep 28, 2023
Copy link
Contributor

@nissy-dev nissy-dev left a comment

Choose a reason for hiding this comment

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

Thank you for quick fix! I think you can merge PR

I commented on some of the points I would like to see addressed in another follow up PR.


https://biomejs.dev plugin for JetBrains IDEs.
[Biome](https://biomejs.dev/) is a powerful tool designed to enhance your development experience. This plugin integrates seamlessly with many [JetBrains IDE's](#Supported IDEs) to provide some capabilities:
Copy link
Contributor

@nissy-dev nissy-dev Sep 30, 2023

Choose a reason for hiding this comment

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

This is great! The updated readme is really helpful for users!

I think this should be reflected to META-INF/plugin.xml by follow up PR. (We've passed on using org.jetbrains.changelog.markdownToHTML in the past, but maybe it's time to use)

@@ -190,6 +190,16 @@ Build and run the plugin requires:
./gradlew buildPlugin
```

### UI Testing intellij plugin

Before start testing the plugin you will need to start IDE by invoking the `./gradlew runIdeForUiTests &` and wait IDE startup
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to mention pnpm install is needed

@nissy-dev
Copy link
Contributor

By the way, the UI test did not work on my end with the following error.
I don't think this needs to be fixed right now, as fixing the bug is the first thing to do.

test result

16:13 🐧 ❯ ./gradlew test
Calculating task graph as configuration cache cannot be reused because the file system entry 'build/classes/kotlin/main/classpath.index' has been created.

> Configure project :
Kotlin DSL property assignment is an incubating feature.

> Task :test
CompileCommand: exclude com/intellij/openapi/vfs/impl/FilePartNodeRoot.trieDescend bool exclude = true

BasicProjectNpmTest > openQuickFixes(RemoteRobot) FAILED
    org.opentest4j.AssertionFailedError at BasicProjectNpmTest.kt:63

1 test completed, 1 failed

> Task :test FAILED

0 problems were found storing the configuration cache.

See the complete report at file:///Users/d002024/code/github.com/biomejs/biome/editors/intellij/build/reports/configuration-cache/517ysd6f9umxr929v0i5rmazs/dwau8wqkvt5e8qfxn26lff4zf/configuration-cache-report.html

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: file:///Users/d002024/code/github.com/biomejs/biome/editors/intellij/build/reports/tests/test/index.html

* Try:
> Run with --scan to get full insights.

BUILD FAILED in 19s
12 actionable tasks: 5 executed, 7 up-to-date
Configuration cache entry stored.

IDE error

2023-09-30 16:13:24,274 [  54437]   WARN - #c.i.o.v.i.l.LocalFileSystemBase - /dev/fd/20: Bad file descriptor
java.nio.file.FileSystemException: /dev/fd/20: Bad file descriptor
        at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:100)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
        at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
        at java.base/sun.nio.fs.UnixFileAttributeViews$Basic.readAttributes(UnixFileAttributeViews.java:55)
        at java.base/sun.nio.fs.UnixFileSystemProvider.readAttributes(UnixFileSystemProvider.java:148)
        at java.base/java.nio.file.Files.readAttributes(Files.java:1851)
        at com.intellij.openapi.util.io.NioFiles.readAttributes(NioFiles.java:177)
        at com.intellij.util.io.PlatformNioHelper.visitDirectory(PlatformNioHelper.java:54)
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.listWithAttributes(LocalFileSystemImpl.java:307)
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.lambda$new$0(LocalFileSystemImpl.java:49)
        at com.intellij.openapi.vfs.DiskQueryRelay.accessDiskWithCheckCanceled(DiskQueryRelay.java:45)
        at com.intellij.openapi.vfs.impl.local.LocalFileSystemImpl.listWithCaching(LocalFileSystemImpl.java:274)
        at com.intellij.openapi.vfs.newvfs.RefreshWorker.partialDirRefresh(RefreshWorker.java:321)
        at com.intellij.openapi.vfs.newvfs.RefreshWorker.processQueue(RefreshWorker.java:185)
        at com.intellij.openapi.vfs.newvfs.RefreshWorker.lambda$parallelScan$0(RefreshWorker.java:121)
        at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1768)

@victor-teles
Copy link
Contributor Author

@nissy-dev Thank you very much for the review, I will address your points in another PR
I'll let you know when I open the PR

@victor-teles victor-teles merged commit a75a9b7 into biomejs:main Sep 30, 2023
4 checks passed
@victor-teles victor-teles deleted the fix/intellij-binary-resolution branch September 30, 2023 18:27
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Error 193 in webstorm plugin: %1 is not a valid Win32 application
3 participants