Skip to content
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

Implement -[NSFileManager URLsForDirectory:inDomains:] #419

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

2xsaiko
Copy link
Contributor

@2xsaiko 2xsaiko commented Jun 10, 2024

This behaves like NSSearchPathForDirectoriesInDomains (at least, superficially. Not sure if Apple's implementation has some special behavior on top that their documentation doesn't mention.)

The documentation comment is copied from NSSearchPathForDirectoriesInDomains.

@2xsaiko 2xsaiko requested a review from rfm as a code owner June 10, 2024 18:47
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.

The concept is fine, but there are two show-stopping issues of non-portable coding with the implementation:

  1. The use of generics: you need to replace the generic syntax with the macros provided in NSObjCRuntime.h
  2. The use of blocks: you need to replace any blocks code with portable code. In this case it's simple enumeration so you might best use the FOR_IN macro provided in GSFastEnumeration.h

@gcasa
Copy link
Member

gcasa commented Jun 11, 2024

@2xsaiko I don't believe a copyright assignment is needed for this change. Please write [email protected] and request a copyright assignment for GNUstep and CC me on the request at [email protected]. Also, please make the changes suggested via @rfm. The reason is that GNUstep supports more compilers than just clang and not all support the same features. Thank you, GC

@gcasa gcasa closed this Jun 11, 2024
@gcasa gcasa reopened this Jun 11, 2024
@gcasa
Copy link
Member

gcasa commented Jun 11, 2024

Sorry, I accidentally hit the "close" button. I reopened this.

@2xsaiko
Copy link
Contributor Author

2xsaiko commented Jun 11, 2024

Done!

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.

One tiny issue; the dot syntax to access a property is non-portable; older compilers don't support properties, so we must call the method to get the count of objects in the paths array.

However, I'll merge and then fix that rather than holding things up.

@rfm rfm merged commit 92247d1 into gnustep:master Jun 12, 2024
8 checks passed
@rfm
Copy link
Contributor

rfm commented Jun 12, 2024

oops ... also an excess RELEASE to be removed and some formatting tweaks

@2xsaiko
Copy link
Contributor Author

2xsaiko commented Jun 12, 2024

Hmm, I see, the array returned by NSSearchPathForDirectoriesInDomains is autoreleased. Should that be the case for the array returned from this method too then?

Sorry about that, this was originally written in ARC mode so I didn't see if it breaks anything and I'm used to COM where there's only retain and release so you generally have to release everything returned from a method call :^)

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.

3 participants