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.

Sign up for GitHub

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

can directly specify the path of sdk #354

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

can directly specify the path of sdk #354

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lua/flutter-tools/executable.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,16 @@ function M.get(callback)
end
end

local flutter_sdk_path = nil
if config.flutter_sdk then flutter_sdk_path = flutter_sdk_path end

if config.flutter_path then
local flutter_path = fn.resolve(config.flutter_path)
_paths = { flutter_bin = flutter_path, flutter_sdk = _flutter_sdk_root(flutter_path) }
local flutter_path = config.flutter_path
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this could be a breaking change. Would config.flutter_lookup_cmd work for you? If not, perhaps config.flutter_path could be a function. For example:

    local flutter_path = type(config.flutter_path) == "function" and config.flutter_path()
      or fn.resolve(config.flutter_path)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. flutter_lookup_cmd won't work..

why here must be fn.resolve()? I think there is no need..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.. you need it to find dart and etc..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

then what about add another config to set flutter sdk manually? I think this way to find flutter sdk is a little weird

Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have two configuration options, and introducing a third one might make things more confusing. These two options seem to cover most use cases. I agree that allowing flutter_path to be a function is a bit unconventional, but it serves as an escape hatch for manually providing the path, which I think is a reasonable compromise. Here's an example of how it would look:

{
  flutter_path = function()
    return 'path/to/flutter'
  end
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But the thing that I am thinking about is that the way to find the flutter sdk is weird. Find the absolute path of flutter and then find the path of the sdk.. but , yes, it will work for flutter. But does flutter have an official way to get the path of SDK?

local flutter_path_real_try = fn.resolve(flutter_path)
_paths = {
flutter_bin = flutter_path,
flutter_sdk = flutter_path or _flutter_sdk_root(flutter_path_real_try),
}
_paths.dart_sdk = _dart_sdk_root(_paths)
_paths.dart_bin = _flutter_sdk_dart_bin(_paths.flutter_sdk)
return callback(_paths)
Expand Down
Loading