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.

Sign up for GitHub

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

Jump to bottom

change(mac): store partial path in UserDefaults #12144

Merged
merged 7 commits into from
Aug 30, 2024
Merged

change(mac): store partial path in UserDefaults #12144

merged 7 commits into from
Aug 30, 2024

Conversation

sgschantz
Copy link
Contributor

@sgschantz sgschantz commented Aug 9, 2024 •
edited
Loading

Rather than store the full path of keyboard files throughout the Keyman settings in the UserDefaults, simply store the relative path from the Keyman-Keyboards directory.

With the encapsulation of the settings/UserDefaults in KMSettingsRepository, this also allowed other references to the settings to access it directly rather than gaining access via the application delegate.

This is dependent on #12106 which changed the location of the Keyman data on disk.

Fixes #2542

For testing purposes, the current values of Keyman's settings in the UserDefaults can be found by executing the following command in a terminal window:

defaults read keyman.inputmethod.Keyman

These settings can be written to a plist file in the current directory with

defaults read keyman.inputmethod.Keyman > ./keyman-userdefaults.plist

These settings can be imported back into the UserDefaults as follows:

defaults import keyman.inputmethod.Keyman ./keyman-userdefaults.plist

Other commands are also useful for testing:
delete all Keyman settings: defaults delete keyman.inputmethod.Keyman
delete a single setting: defaults delete keyman.inputmethod.Keyman KMDataModelVersion
set a single setting: defaults write keyman.inputmethod.Keyman KMDataModelVersion 1
get documentation: 'defaults help'

User Testing

TEST_KEYBOARDS_MOVED_TO_LIBRARY

  1. Use Keyman 17 for Mac and install a couple keyboards
  2. Verify that these keyboards are located in ~/Documents/Keyman-Keyboards
  3. Install and run the new version of Keyman for this PR
  4. Verify that the keyboards files have all been moved to ~/Library/Application

Support/keyman.inputmethod.Keyman/Keyman-Keyboards

TEST_ARMENIAN_OPTION_ENABLED:

  1. Open the Stickies app on the Mac and create a new note
  2. Switch to the Armenian Mnemonic keyboard
  3. Type ew
  4. Confirm that this produces the output: եւ
  5. Type option-vw (this toggles a setting within Keyman for this specific keyboard)
  6. Type ew
  7. Confirm that this same input as earlier now produces the output: և
  8. In a terminal window type the command defaults read keyman.inputmethod.Keyman
  9. Confirm that the Armenian keyboard has an option of option_ligature_ew with a value of 1.

TEST_ARMENIAN_OPTION_DISABLED:

  1. With the current keyboard set to Armenian Mnemonic, and after executing TEST_ARMENIAN_OPTION_ENABLED, type option-vw to disable the option option_ligature_ew
  2. Type ew
  3. Confirm that this produces the output: եւ
  4. In a terminal window type the command defaults read keyman.inputmethod.Keyman
  5. Confirm that the Armenian keyboard has an option of option_ligature_ew with a value of 0.

TEST_SELECTED_KEYBOARD:

  1. Install the keyboard EuroLatin (SIL) and select it as the current keyboard
  2. In a terminal window type the command defaults read keyman.inputmethod.Keyman
  3. Confirm that the property KMSelectedKeyboardKey is set to the value "/sil_euro_latin/sil_euro_latin.kmx"

@sgschantz sgschantz added this to the A18S18 milestone Aug 9, 2024
@sgschantz sgschantz self-assigned this Aug 9, 2024
@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Aug 9, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Aug 9, 2024 •
edited
Loading

User Test Results

Test specification and instructions

Test Artifacts

@sgschantz sgschantz mentioned this pull request Aug 12, 2024
Merged
@sgschantz
change(mac): use partial paths in settings
b1685f5 
converts partial paths to full paths as needed
and vice-versa when moving between file
system access and referencing keyboards
in UserDefaults
@sgschantz sgschantz linked an issue Aug 16, 2024 that may be closed by this pull request
change(mac): Stop storing keyboards in Documents folder #2542
Closed
@sgschantz
change(mac): some refactoring
5364a74 
Mostly taking advantage of KMSettingsRepository so
that more of the UserDefaults changes happen through it
Still some refactoring remains for active keyboards list
@sgschantz
change(mac): persist options fix when dictionary exists
49bfd08 
testing showed that when an option already existed,
it would be deleted when attempting to update it to
a new value
@sgschantz sgschantz marked this pull request as ready for review August 19, 2024 07:50
@sgschantz sgschantz requested a review from SabineSIL as a code owner August 19, 2024 07:50
@dinakaranr
Copy link

Test Results

I retested this issue with the attached "Keyman 18.0.86-alpha-test" build on the macOS. Here is my observation.

  • TEST_KEYBOARDS_MOVED_TO_LIBRARY (Passed):
  1. Installed the Keyman 17.0.328 version in the MacOS Sonoma.
  2. Installed the "EuroLatin(SIL)", "GFF Amharic", "Khmer Angkor" keyboards
  3. Verified that the installed keyboards are located in the ~/Documents/Keyman-Keyboards.
  4. Uninstalled Keyman older version.
  5. Installed the keyman 18.0.86.img keyboard.
  6. Verified that the keyboards moved into the ~/Library/Application Support/keyman.inputmethod.Keyman/Keyman-Keyboards
    It works well. Thanks.
  • TEST_ARMENIAN_OPTION_ENABLED (Passed):
  1. I have followed steps which mentioned steps 1 to 9.
  2. The Armenian Mnemonic toggles keys when pressing option-v and then w
  3. the և letter appears after toggles happen.
  4. "option_ligature_ew = 1" is showing when executing the "defaults read keyman.inputmethod.Keyman" on the terminal.
    It works well.
  • TEST_ARMENIAN_OPTION_DISABLED (Passed):
  1. I have followed steps which mentioned steps 1 to 5.
  2. The Armenian Mnemonic toggles keys when pressing option-v and then w
  3. the եւ letter appears after toggles happen.
  4. "option_ligature_ew = 0" is showing when executing the "defaults read keyman.inputmethod.Keyman" on the terminal.
    It works well.
  • TEST_SELECTED_KEYBOARD (Passed):
  1. Install the keyboard EuroLatin (SIL) and select it as the current keyboard.
  2. KMSelectedKeyboardKey = "/sil_euro_latin/sil_euro_latin.kmx" when executing the "defaults read keyman.inputmethod.Keyman" on the terminal.
  3. If the current keyboard is changed then the path of the keyboard is changed to "KMSelectedKeyboardKey"
    It works well. Thank you.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Aug 21, 2024
@sgschantz sgschantz requested a review from mcdurdin August 28, 2024 06:07
mcdurdin
mcdurdin approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodAppDelegate.m Outdated

for (NSString *kmxFile in [self getKmxFilesAtPath:keyboardFolderPath]) {
NSString *partialPath = [KMDataRepository.shared buildPartialPathFrom:folderName keyboardFile:[kmxFile lastPathComponent]];
// TODO: encapsulate this in KMSettingsRepository, insertIfNotExists
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: encapsulate this in KMSettingsRepository, insertIfNotExists

Base automatically changed from change/mac/2542-store-keyboards-elsewhere to master August 30, 2024 05:35
@sgschantz sgschantz merged commit 0aa4be5 into master Aug 30, 2024
5 checks passed
@sgschantz sgschantz deleted the change/mac/2542-use-partial-keyboard-paths branch August 30, 2024 05:36
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.102-alpha

@mcdurdin mcdurdin modified the milestones: A18S18, A18S9 Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

@SabineSIL SabineSIL Awaiting requested review from SabineSIL SabineSIL is a code owner

Assignees

@sgschantz sgschantz

Labels
change has-user-test mac/
Projects
Keyman
Archived in project
Milestone
A18S9
Development

Successfully merging this pull request may close these issues.

change(mac): Stop storing keyboards in Documents folder
4 participants
@sgschantz @dinakaranr @keyman-server @mcdurdin