[flutter_tools] change the IntelliJ plugin detection logic. #68020
Conversation
|
Hi @jonahwilliams |
|
I'll take a look! |
jonahwilliams
left a comment
There was a problem hiding this comment.
This is great, just a few suggestions.
Could you also leave a longer comment in intellij.dart describing the newer plugin structure and some example files you need to check?
| // Collect the files with a file suffix of .jar/.zip that contains the plugin.xml file | ||
| final List<FileSystemEntity> pluginJarPaths = _fileSystem | ||
| .directory(_fileSystem.path.join(pluginsPath, packageName, 'lib')) | ||
| .listSync() |
There was a problem hiding this comment.
since you only care about Files, you can add whereType<File>() to filter the iterable down to just them. Then you can skip the stat check here and make pluginJarPaths a List of files:
_fileSystem
.directory(_fileSystem.path.join(pluginsPath, packageName, 'lib'))
.listSync()
.whereType<File>()
.where((File file) {
final String fileExt= _fileSystem.path.extension(file.path);
return fileExt == '.jar' || fileExt == '.zip';
})
.toList()
There was a problem hiding this comment.
Thank you. I fixed it like you commented.
| return null; | ||
| } | ||
| // Prefer file with the same suffix as the package name | ||
| pluginJarPaths.sort((FileSystemEntity a, FileSystemEntity b) { |
There was a problem hiding this comment.
if the names don't match at all is it worth including them in the list?
There was a problem hiding this comment.
Yes, it is.
The reason is that there are several variations of the jar file containing META-INF/plugin.xml.
Currently, we need to check at least three files: flutter-intellij.jar / flutter-intellij-X.Y.Z.jar / flutter-idea-X.Y.Z.jar.
| try { | ||
|
|
||
| final Iterator<String> itr = mainJarPathList.iterator; | ||
| while (itr.moveNext()) { |
There was a problem hiding this comment.
(Assuming you change the list to one of files). I would stick with the for-in loop instead of manipulating the iterable directly.
for (final File file in mainJarPathList) {
...
}
There was a problem hiding this comment.
Thank you. I fixed it like you commented.
Co-authored-by: Jonah Williams <jonahwilliams@google.com>
…es instead of 'String' instances of jar path.
I've added a comment in intellij.dart. |
jonahwilliams
left a comment
There was a problem hiding this comment.
LGTM!
Thank you once again for your contribution 😄
| /// This searches on the provided plugin path for a JAR archive, then | ||
| /// unzips it to parse the META-INF/plugin.xml for version information. | ||
| /// | ||
| /// In particular, the Intellij Flutter plugin has a different structure depending on the version. |
There was a problem hiding this comment.
Thank you for the details here, this is a great comment
|
The Mac tool_tests issue is a flake, landing this now |
Description
This PR suggests improving the IntelliJ plugin "jar" detection logic.
Previously:
The IntelliJ Flutter plugin was contained
flutter-intellij.jar.Currently:
It is named
flutter-intellij-X.Y.Z.jarand does not containMETA-INF/plugin.xml.META-INF/plugin.xmlis included influtter-idea-X.Y.Z.jar.So this PR changes the rules for searching the plugin's jar file.
Concretely, it looks for the jar file containing
META-INF/plugin.xmlin the plugin's package directory and reads the package version from itsMETA-INF/plugin.xml.Related Issues
[#67918] (already fixed and closed via #67931)
Tests
I added the following tests:
packages/flutter_tools/test/general.shard/intellij/intellij_test.dart
I changed the following test:
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.