Centralize utilities for configuration loading and path parsing#1448
Centralize utilities for configuration loading and path parsing#1448noah-thor wants to merge 1 commit intoapple:mainfrom
Conversation
|
Mainly looking for API feedback, changes will not be mergable until TOML PR is merged |
| } | ||
| } else { | ||
| guard let resolved = UserConfiguration.resolve(from: .home, relativePath: "config.toml") else { | ||
| return |
There was a problem hiding this comment.
Should we throw here?
There was a problem hiding this comment.
Yeah, good catch or should I say good throw
There was a problem hiding this comment.
Wait actually this is the case in which a user has not provided an explicit path to the file, so we just want to be ok (not throw) if they do not yet have a toml file created at all. I will add a comment
| let path = configFile.string | ||
| guard FileManager.default.fileExists(atPath: path) else { | ||
| do { | ||
| return try TOMLDecoder().decode(T.self, from: Data("".utf8)) |
There was a problem hiding this comment.
Could we make T Initable and just return an instance here?
There was a problem hiding this comment.
Is that an existing protocol? I can't seem to find it. I assume its like:
protocol Initable {
init()
}
?
| if let envPath = ProcessInfo.processInfo.environment["CONTAINER_APP_ROOT"], !envPath.isEmpty { | ||
| return FilePath(envPath) | ||
| } | ||
| let appSupportURL = FileManager.default.urls( | ||
| for: .applicationSupportDirectory, | ||
| in: .userDomainMask | ||
| ).first!.appendingPathComponent("com.apple.container") | ||
| return FilePath(appSupportURL.path(percentEncoded: false)) |
There was a problem hiding this comment.
Could we dedup this with ApplicationRoot?
There was a problem hiding this comment.
You mean completely remove the ApplicationRoot and/or just have a single definition? I think that makes sense I was just hesitant to touch that code
4 participants
Type of Change
Motivation and Context
This attempts to create a more centralized utility to find and load configuration files, as a follow up to (pr: #1425).
This would make the file apis look something like:
Testing