MathCAT: store YAML preferences with NVDA configuration#19373
Conversation
|
CC @RyanMcCleary. |
There was a problem hiding this comment.
Pull request overview
This pull request relocates MathCAT's YAML preferences file from a separate directory in the user's roaming AppData to the NVDA user configuration directory, aligning with NVDA project conventions.
Key Changes:
- Removed the
pathToUserPreferencesFolder()function that pointed to a separate MathCAT directory - Updated
pathToUserPreferences()to use NVDA'sgetUserDefaultConfigPath()and renamed the file tomathcat.yaml - Removed directory creation logic since NVDA handles config directory initialization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b277a77 to
33949bb
Compare
33949bb to
855c68e
Compare
|
@codeofdusk - do you intend to continue to work on this? |
|
Sorry to be late to responding to this. Too many emails and these sometimes get lost. The design idea for Because MathCAT can be used in many different projects and some may allow for multiple users, |
|
I'm quite concerned about this PR:
IMO we need to either:
|
Follow-up to #18323, #19373 and #19613 ### Summary of the issue: Since the introduction of MathCAT into NVDA in #18323, NVDA has persisted MathCAT settings to a YAML file . This is at odds with current project configuration standards, which use either configobj ini files, or JSON. This also duplicates settings, as they're also saved to `nvda.ini`. Originally, these were stored in `%AppData%\MathCAT\prefs.yaml`, but as of #19373, they are saved to `mathcat.yaml` in NVDA's config directory. In neither case was `shouldWriteToDisk` checked before saving the preferences. Additionaly, [NV Access's Corporate & Government page](https://www.nvaccess.org/corporate-government/) states "To prevent NVDA users from modifying their configuration or add-ons directly, user write access to [`%APPDATA%\nvda`] must also be restricted". If implemented, this would have stopped users from changing their MathCAT settings in memory, as they had to be saved to YAML to be loaded by MathCAT. In #19613, the logic was changed to programmatically apply these settings with `libmathcat.SetPreference`, which should solve the problem of users without write access to their NVDA config directory being able to change their MathCAT preferences. However, the code that wrote the settings to the YAML file was not removed, nor did it contain any error handling. ### Description of user facing changes: None, but may avoid some errors in the logs. ### Description of developer facing changes: Pyyaml is no-longer a dependency. `mathPres.MathCAT.preferences.MathCATUserPreferences.save` has been renamed to `apply`, and no longer saves the settings to YAML. Settings persistance is handled by NVDA's configuration system. ### Description of development approach: Removed the code that wrote MathCAT preferences to YAML, and any code that only it depended on. Renamed `save` to `apply` using VS Code's refactoring tool. Searched the repo for `yaml` and after confirming that we don't rely on `pyyaml` anywhere else, ran `uv remove pyyaml`. ### Testing strategy: Tested by loading the [Wikipedia page on the quadratic formula](https://en.wikipedia.org/wiki/Quadratic_formula) in firefox. Located some math and read it. Changed settings one at a time and tested that they were applied correctly. Restarted NVDA and ensured that the new settings were still present. ### Known issues with pull request: None known
Link to issue number:
None
Summary of the issue:
NVDA creates a MathCAT directory in the user's roaming appdata (by default, a sibling of the NVDA config directory) which:
Description of how this pull request fixes the issue:
Relocates the MathCAT preferences yaml to NVDA user configuration.
Testing strategy:
Verified that settings are read and written as expected.
Known issues with pull request:
Since this file is generated from NVDA user config, no settings should be lost. Just in case though, we should probably do this migration before 2026.1 is released.
Code Review Checklist: