adjust API to changes in SwiftPM #433
Conversation
motivation: stay with the times changes: * use observability system instead of diganostics engine * some APIs in SwiftPM are now throwing, adjust to this change
tomerd
force-pushed
the
fix/adjust-to-swiftpm-3818
branch
from
4e75720 to
493eab1
Compare
|
@swift-ci please test |
| @@ -366,7 +369,7 @@ extension SwiftPMWorkspace { | |||
| args += ["-c"] | |||
| args += td.sources.map { $0.pathString } | |||
| args += ["-I", buildPath.pathString] | |||
| args += td.compileArguments() | |||
| args += try! td.compileArguments() | |||
There was a problem hiding this comment.
Why do we know this cannot throw here? Same question for basicArguments below.
There was a problem hiding this comment.
this is in response to swiftlang/swift-package-manager#3818 which unwinds a case where previously SwiftPM had a fatalError when this failed. I started going down the path of changing the LSP codebase to throwing everywhere but that became a bit much for me to do without understanding the codebase (delegate signatures, etc) so I decided to use try! (which is the same as before in practice) and let the team unwind this more safely. wdyt?
There was a problem hiding this comment.
to be clear I am happy to bring these changes back in, but they end up effecting things like the BuildSystem protocol (registerForChangeNotifications needs to handle errors or marked as throwing) which is a bit beyond my understanding of LSP. basically:
public func registerForChangeNotifications(for uri: DocumentURI, language: Language) {
guard let delegate = self.delegate else { return }
// TODO: Support for change detection (via file watching)
let settings = try self.settings(for: uri, language) // <----- we need to deal with this
DispatchQueue.global().async {
delegate.fileBuildSettingsChanged([uri: FileBuildSettingsChange(settings)])
}
}
There was a problem hiding this comment.
I see - the interesting question for us I guess is what kinds of errors there are here and whether we can prevent them or if we need to handle them. For example, I see that one error is related to validating the target -- that seems like something we would want to just handle up front. But if there are other kinds of errors here maybe we need to just log and continue.
There was a problem hiding this comment.
I think errors from here are rare, but fatalError seems wrong since it will cause a crash. logging is fine I guess. would you like me to do that?
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
tomerd
force-pushed
the
fix/adjust-to-swiftpm-3818
branch
from
7f9e81f to
3fdc56c
Compare
|
@swift-ci please test |
|
@swift-ci please test |
motivation: stay with the times
changes: