Fix Error Cannot prevent display sleep!, #3842 #3924
Conversation
The commit in the pull request will: - Change SleepPreventer.preventSleep to only display an alert once per IINA invocation - Change Utility.showAlert to support a suppression button - Add a button to the alert to allow the user to permanently suppress the alert - Change the alert to include the error code returned by macOS Additional text was added to the alert's message requiring localization. This fixes how IINA reports a macOS power management failure. The root cause of the failure is in macOS and must be fixed by Apple.
| static private var preventedSleep = false | ||
|
|
||
| /// Ask macOS to not dim or sleep the display. |
There was a problem hiding this comment.
A brief description is fine but I'd strongly recommend just linking to the issues themselves rather than copying their contents here, because they'll remain up-to-date and are a better place to put extended discussion
There was a problem hiding this comment.
That came from my bias for keeping information with the code after encountering broken links on work projects. I've changed that portion of the comment to just refer to the two issues.
There was a problem hiding this comment.
We are not likely to migrate to JIRA anytime soon :P
iina/SleepPreventer.swift
Outdated
| @@ -28,11 +64,21 @@ class SleepPreventer: NSObject { | |||
| &assertionID) | |||
| if success == kIOReturnSuccess { | |||
There was a problem hiding this comment.
It might be worth converting this to a guard
iina/Base.lproj/Localizable.strings
Outdated
| @@ -143,7 +143,7 @@ | |||
|
|
|||
| "alert.fatal_error" = "Fatal error: %@ \nThe application will exit now."; | |||
| "alert.mpv_error" = "Internal error %@ (%@) when setting option %@."; | |||
| "alert.sleep" = "Cannot prevent display sleep!"; | |||
| "alert.sleep" = "Cannot prevent display sleep!\nmacOS error 0x%0X8"; | |||
There was a problem hiding this comment.
I think I'd prefer a "Cannot prevent display sleep! (error code)" format, as this better matches how macOS displays errors
There was a problem hiding this comment.
I first considered doing it like that and changed to this to put emphasis on this being a macOS problem. With it now not adding any words to the string should I be updating more of the lanuages, or just base and EN? The alert now looks like:
There was a problem hiding this comment.
lgtm. I prefer to not pass judgement on where the error is coming from, in general we should just present it as it is.
The commit in the pull request will: - Change SleepPreventer.preventSleep to only display an alert once per IINA invocation - Change Utility.showAlert to support a suppression button - Add a button to the alert to allow the user to permanently suppress the alert - Change the alert to include the error code returned by macOS Additional text was added to the alert's message requiring localization. This fixes how IINA reports a macOS power management failure. The root cause of the failure is in macOS and must be fixed by Apple.
The commit in the pull request will: - Change SleepPreventer.preventSleep to only display an alert once per IINA invocation - Change Utility.showAlert to support a suppression button - Add a button to the alert to allow the user to permanently suppress the alert - Change the alert to include the error code returned by macOS Additional text was added to the alert's message requiring localization. This fixes how IINA reports a macOS power management failure. The root cause of the failure is in macOS and must be fixed by Apple.
- Change SleepPreventer.preventSleep to only display an alert once per IINA invocation - Change Utility.showAlert to support a suppression button - Add a button to the alert to allow the user to permanently suppress the alert - Change the alert to include the error code returned by macOS - Add a button in Preferences/Utilities to restore suppressed alerts Additional text was added requiring localization. This fixes how IINA reports a macOS power management failure. The root cause of the failure is in macOS and must be fixed by Apple.
|
LGTM |
The commit in the pull request will:
IINA invocation
the alert
Additional text was added to the alert's message requiring localization.
This fixes how IINA reports a macOS power management failure.
The root cause of the failure is in macOS and must be fixed by Apple.
Description:
This commit includes new text that needs localization.
See the issue for screenshots of the new alert.