Add support for parsing registrations wrapped in #if conditions #96
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skorulis-ap
commented
Nov 11, 2023
| @@ -32,6 +36,10 @@ public struct Registration: Equatable, Codable { | |||
| self.getterConfig = getterConfig | |||
| } | |||
|
|
|||
| private enum CodingKeys: CodingKey { | |||
| case service, name, accessLevel, arguments, isForwarded, getterConfig | |||
There was a problem hiding this comment.
Ignoring the ifConfigExpression for now. If it's needed in the future we will need to add custom encoding logic
There was a problem hiding this comment.
Nit: can you add a small comment that the list is there just to exclude the ifConfigCondition property?
skorulis-ap
force-pushed
the
skorulis/ifdef
branch
3 times, most recently
from
63a7c7e to
6a2dc3b
Compare
skorulis-ap
changed the title
skorulis-ap
force-pushed
the
skorulis/ifdef
branch
from
6a2dc3b to
8149bcb
Compare
bradfol
approved these changes
Nov 17, 2023
Comment on lines
+184
to
+186
| // Allowing for #else creates a link between the registration inside the #if and those in the #else | ||
| // This greatly increases the complexity of handling #if so raise an error when #else is used | ||
| if node.poundKeyword.text.contains("#else") { |
There was a problem hiding this comment.
Yeah I think this is reasonable. Folks that really need it can still work around by making multiple #if blocks
| @@ -32,6 +36,10 @@ public struct Registration: Equatable, Codable { | |||
| self.getterConfig = getterConfig | |||
| } | |||
|
|
|||
| private enum CodingKeys: CodingKey { | |||
| case service, name, accessLevel, arguments, isForwarded, getterConfig | |||
There was a problem hiding this comment.
Nit: can you add a small comment that the list is there just to exclude the ifConfigCondition property?
Comment on lines
+321
to
+325
| XCTAssertEqual(config.registrations[1].service, "B") | ||
| XCTAssertEqual(config.registrations[1].ifConfigCondition?.description, "SOME_FLAG && !ANOTHER_FLAG") | ||
|
|
||
| XCTAssertEqual(config.registrations[2].service, "C") | ||
| XCTAssertEqual(config.registrations[2].ifConfigCondition?.description, "SOME_FLAG && !ANOTHER_FLAG") |
skorulis-ap
force-pushed
the
skorulis/ifdef
branch
from
8149bcb to
c4afc54
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This resolves an issue I found where a type I registered was only available behind a conditional build flag. References to this type were being put into the type safety and unit tests which would fail depending on the build configuration.
This change identifies
#ifstatements, stores the condition and then wraps any generated code related to the registration with the original#if.Limitations
#elseis not supported to prevent creating relationships between registrations.#ifstatements are not supported only due to the extra parsing complexity. This can be added in the future.