Skip to content

Conversation

hmelder
Copy link
Member

@hmelder hmelder commented May 1, 2025

No description provided.

@hmelder hmelder requested a review from rfm as a code owner May 1, 2025 05:57
@hmelder
Copy link
Member Author

hmelder commented May 27, 2025

@rfm this is also supported in recent MacOS versions

Copy link
Contributor

@rfm rfm left a comment

Choose a reason for hiding this comment

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

Am I missing something? My impression is that all one needs to do to achieve the objective is:
First call +propertyListFromData:mutabilityOption:format:errorDescription: to try to parse the file.
Then, if it failed to parse, use the current code.
If it parsed but produced the wrong information, log an error because the file is wrong.
If it parsed and produced the right information, use it.
If that's sufficient, it's a smaller/simpler change and limited to just NSBundle.m

@hmelder
Copy link
Member Author

hmelder commented Jun 3, 2025

If it parsed but produced the wrong information, log an error because the file is wrong.
If it parsed and produced the right information, use it.

How can I verify this using +propertyListFromData:mutabilityOption:format:errorDescription:?
Since errorDescription is just a string, it doesn't provide details about the error. This means I can't reliably determine the type of error that occurred. The only option in case of an error is to continue execution, which can be unpredictable. For instance, if parsing fails due to a syntax error in the plist, the method might continue and log confusing or misleading warnings.

@rfm
Copy link
Contributor

rfm commented Jun 3, 2025

I'm not sure the order in which you get error information is much of a problem, but while looking at this I realised that there already existed a private function for getting strings table content (and NSBundle wasn't using it for some reason), so I tweaked that to call +propertyListFromData:mutabilityOption:format:errorDescription: after failing with the standard strings file parsing and simplified (removed quite a bit of code) NSBundle by using it. I also added a tiny testcase for it. Does it meet your needs?

@hmelder
Copy link
Member Author

hmelder commented Jun 10, 2025

But id GSPropertyListFromStringsFormat(NSData *data) GS_ATTRIB_PRIVATE; does not really solve the problem:

Since errorDescription is just a string, it doesn't provide details about the error. This means I can't reliably determine the type of error that occurred. The only option in case of an error is to continue execution, which can be unpredictable

Something like +propertyListFromData:mutabilityOption:format:error: and then continue if the blob was not recognized as a property list.

@hmelder
Copy link
Member Author

hmelder commented Jun 22, 2025

@rfm

@rfm
Copy link
Contributor

rfm commented Sep 15, 2025

it doesn't provide details about the error. This means I can't reliably determine the type of error that occurred.

It seems to me that, at this point (we have determined that the file exists and have loaded it in) there is only one possible error: the content of the file cannot be parsed as a property list (ie it's not valid format in some way).

Certainly there's no obvious correct set of additional error information we can/should provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants