-
Notifications
You must be signed in to change notification settings - Fork 233
Add SourceKitObject
that represents sourcekitd_object_t
in Swift
#490
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
Conversation
So much nicer! 💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome. Everything I've ever dreamed of. 😍
@@ -196,7 +196,7 @@ public enum Request { | |||
/// A `cursorinfo` request for an offset in the given file, using the `arguments` given. | |||
case cursorInfo(file: String, offset: Int64, arguments: [String]) | |||
/// A custom request by passing in the sourcekitd_object_t directly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should update this comment.
|
||
import Foundation | ||
#if SWIFT_PACKAGE | ||
import SourceKit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: I generally avoid indenting compiler directive bodies. See https://github.com/jpsim/SourceKitten/blob/0.19.1/Source/sourcekitten/main.swift#L9-L15
} | ||
} | ||
|
||
extension UID: Hashable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on using the auto-synthesized Hashable
implementation for Swift 4.1+?
extension UID: Hashable {
#if !swift(>=4.1)
public var hashValue: Int {
return uid.hashValue
}
public static func == (lhs: UID, rhs: UID) -> Bool {
return lhs.uid == rhs.uid
}
#endif
}
I don't feel strongly, but it seems like it might be easier to find code that can be removed once we require Swift 4.1+ to compile SourceKitten.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not doing this. Can't auto-synthesize Hashable conformance outside the source module.
|
||
class SourceKitObjectTests: XCTestCase { | ||
|
||
func testExample() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testDescription
?
20a5707
to
5720e1a
Compare
Thanks for reviews!
Could you please review again? |
@jpsim Should I open another PR against this branch for changes that added after previous reviews? |
No need for a separate PR, I'll review this shortly. Looks mostly good, I think I just have a few pieces of small but breaking feedback |
Any news on this? Would be nice to have in a release :) |
Sorry the delay is entirely my fault. I'll review & merge this now. |
to sourcekitdObject to avoid confusion between 'SourceKitObject's and sourcekitd "objects".
5720e1a
to
b182a23
Compare
I renamed |
Tired of waiting for CI. Will fix if I get notified of failures when tests eventually run on master. |
Some CI jobs for this PR failed since the branch has been removed before starting jobs. |
Yes that’s expected. |
This adds
SourceKitObject
,SourceKitObjectConvertible
andUID
that salvaged from #274.Fixes #489
This is a different solution than #488.
By using this changes, the sample code in #487 (comment) can be written as following:
/cc: @ileitch