feat: provide a method for the user to bypass the first run dialog #344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…download In the case that Ou Dedetai is started with an existing user directory, we know that at some point in the past the user launched Logos for the first time and logged in. Since we know they're already logged in, they should be past the first run dialog. However in the case the dialog failed for whatever reason, the key may still be set in the database. Provide a means for the user to easily bypass this first run dialog in the case that it failed. While there is some risk that in the future Logos will introduce some logic into this first run dialog that's strictly required for the app to function, if that's the case the worst this can do is a break a first time install - that's already likely broken. The user needs to reinstall anyways, we're no worse than we were. Tested: - Accepting and refusing using a crashed first time resource download on GUI and TUI and CLI - Opening Program settings after recovering, observed settings are restored from <data/> to actual contents. It should be noted that on the TUI and GUI if the user refuses the first prompt, they'll need to restart the app in order to change their mind. However on the other hand if the user actually wants to ignore it, this behavior is less intrusive. It should be noted that on the TUI, the user is prompted as if they were on the CLI - which is entirely fine, the user is already used to the terminal Still to test: - Exit the first run dialog very early, and see if skipping it causes any issues.
thw26
requested changes
Feb 25, 2025
| persistent_config.faithlife_product | ||
| ) | ||
| except Exception: | ||
| logging.exception("Failed to check to see if installation is broken.") |
There was a problem hiding this comment.
We might want this to say something like, "Unable to detect an issue." We didn't fail our check in this case; we did check, but we didn't find anything.
ou_dedetai/utils.py
Outdated
| @@ -653,6 +653,14 @@ def parse_bool(string: str) -> bool: | |||
| return string.lower() in ['true', '1', 'y', 'yes'] | |||
|
|
|||
|
|
|||
| def execute_sql(path: str | Path, sql_statements: list[str]) -> list[Any]: | |||
There was a problem hiding this comment.
As above; I think we should put SQL operations into a db.py module.
ou_dedetai/repair.py
Outdated
| time.sleep(5) | ||
| return | ||
| local_user_prefrences_path = Path(logos_appdata_dir) / "Documents" / logos_user_id / "LocalUserPreferences" / "PreferencesManager.db" #noqa: E501 | ||
| ou_dedetai.utils.execute_sql( |
There was a problem hiding this comment.
See comments on adding a db.py module.
We have a couple other bits of code in the works, sitting on the backburner, which also interact with the Logos sqlite DBs. It would be good to put these in their own module so that we don't have to retrieve them at a later time.
| f"Recovery attempt of {app.conf.faithlife_product} complete. " | ||
| f"{app.conf.faithlife_product} should now launch directly, " | ||
| "bypassing first time resource download dialog." | ||
| ) |
There was a problem hiding this comment.
We might want to include our instructions here telling the user to download their library.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BLOCKER: This leaves non-functional installs for now since the arial font is not installed. Factbook crashes after using this. Consider fixing the arial issue before merging this.
In the case that Ou Dedetai is started with an existing user directory, we know that at some point in the past the user launched Logos for the first time and logged in. Since we know they're already logged in, they should be past the first run dialog. However in the case the dialog failed for whatever reason, the key may still be set in the database.
Provide a means for the user to easily bypass this first run dialog in the case that it failed.
While there is some risk that in the future Logos will introduce some logic into this first run dialog that's strictly required for the app to function, if that's the case the worst this can do is a break a first time install - that's already likely broken. The user needs to reinstall anyways, we're no worse than we were.
Tested:
Unhandled exception: page fault on read access to 0x000000000000007c in 64-bit code (0x006ffff63389f1). Logos was trying to index at the time. Removed the Data/Documents directories, and slowly let Logos settle, after re-downloading all of my books it still crashes the same way. Installed "arial" font and resolved the issueIt should be noted that on the TUI and GUI if the user refuses the first prompt, they'll need to restart the app in order to change their mind. However on the other hand if the user actually wants to ignore it, this behavior is less intrusive.
It should be noted that on the TUI, the user is prompted as if they were on the CLI - which is entirely fine, the user is already used to the terminal
Still to test: