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

Resolve various deprecation warnings. #326

Merged
merged 2 commits into from
Feb 13, 2021
Merged

Conversation

jverkoey
Copy link
Contributor

@jverkoey jverkoey commented Jan 13, 2021

  • fileName -> fileURL
  • NSFullScreenWindowMask -> NSWindowStyleMaskFullScreen
  • NSFileHandlingPanelOKButton -> NSModalResponseOK
  • NSPNGFileType -> NSBitmapImageFileTypePNG

TODO

  • Revert the nil comparison changes.

Pre-work for #324

@LIJI32
Copy link
Owner

LIJI32 commented Jan 15, 2021

Two of these warnings are conscious design and style choices:

  • The use of filenames over URL: While long deprecated, the NSString based APIs are cleaner than the NSURL based alternatives, as NSURL doesn't provide any benefits while requiring lengthy conversions between NSURL and NSString.
  • Implicit conversion between pointers and bools is a style choice; since when did that even become a warning?

As for the rest – good catches, I never remember which of these symbols are the non-deprecated ones.

@jverkoey
Copy link
Contributor Author

Implicit conversion between pointers and bools is a style choice; since when did that even become a warning?

Ah this is a style habit for me from the Google Objective-C style guide; I don't feel strongly about it here though because we're not interacting with BOOL, so am happy to revert that change. I'll add a TODO to the PR description.

@jverkoey
Copy link
Contributor Author

jverkoey commented Jan 15, 2021

The use of filenames over URL: While long deprecated, the NSString based APIs are cleaner than the NSURL based alternatives, as NSURL doesn't provide any benefits while requiring lengthy conversions between NSURL and NSString.

Agreed the string-based APIs had historically provided better facilities for working with paths. macOS 10.6 (~2016, roughly around the time you started this repo) introduced new APIs on NSURL for working with paths, so in many cases the APIs are comparable now.

[[self.fileName stringByDeletingPathExtension] stringByAppendingPathExtension:@"ram"]

becomes

[[self.fileURL URLByDeletingPathExtension] URLByAppendingPathExtension:@"ram"].path

The need to invoke path to convert to a string can be made less obtrusive by using property notation, and I've also made the UTF8String invocations follow a similar pattern (from [self.fileName UTF8String] to self.fileURL.path.UTF8String which is intended to improve readability by removing a layer of [ ] brackets.

I'm leaning toward resolving the deprecation warnings for this, but if you feel strongly about keeping the deprecated APIs in place I will defer to your call and can revert the fileName changes if desired.

@jverkoey
Copy link
Contributor Author

Changes to nil checks have been reverted.

@jverkoey
Copy link
Contributor Author

Another proposal for the fileName -> fileURL change is that I'd be happy to make a helper method for changing the file's extension if clarity / verboseness is the primary concern.

@LIJI32 LIJI32 merged commit 3316954 into LIJI32:master Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants