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

Update ChromeToBrave.py script to include Sparkle update keys #344

Merged
merged 6 commits into from
Nov 21, 2020

Conversation

apizz
Copy link
Collaborator

@apizz apizz commented Aug 6, 2020

Still a work in progress, and need feedback given my limited Python skills ...

Previously, we just had an array of dictionaries with preferences that were all appended to the bottom of the manifest and appended to the 'Misc.' menu segment. Given the addition of other preferences that we do not want to have fall under 'Misc.', my thought is in order to better handle additional Brave-specific preferences moving forward is two-fold:

  1. Restructure the com.brave.Browser-specific.plist to instead be a dictionary where the keys match the segment name.
  2. The segment keys are arrays with the preference keys as contained dictionaries.

Right now, the segment key is being written as needed, but the preferences themselves are not (below). On that front, @relgit I could use some help 😄

Screen Shot 2020-08-06 at 7 07 19 PM
Screen Shot 2020-08-06 at 7 07 27 PM

Closes #342

Converted spaces to tabs because that's what the manifests are formatted ... just for consistency.
Since we have preferences that are going to live in different segment sections, this will make it easier for long-term use
@apizz apizz added the 🛠️ update manifest Update existing manifest label Aug 6, 2020
@apizz apizz requested a review from relgit August 6, 2020 23:11
@apizz apizz self-assigned this Aug 6, 2020
- With changes to Brave domain specific plist formatting, add any key segments (ex. Updates) from the domain file not already present in chrome_segment_titles
- Also ensure that these missing key segments have an array for preferences to live
- Append the pfm_name for each preference to the appropriate segment array
- Append the preference dictionaries to the root pfm_subkeys
@apizz
Copy link
Collaborator Author

apizz commented Aug 9, 2020

@relgit this is now working as expected! Proved to be more challenging to wrap my brain around, but it works to recursively take any preferences given the updated domain specific .plist format (to include the segment title since we don't want to assume these preferences are all going to dump into 'Misc.') and add the key name and preference dictionary in the necessary locations.

The one thing this doesn't do at the moment but would like it to is to not add a preference from the domain specific plist if it already exists in the Chrome manifest. For example, if a preference's pfm_name in the brave specific plist already exists in segments, the corresponding pfm_name should not be written again to the applicable segments array and likewise the preference dictionary should not be written to avoid creating a duplicate item. I doubt that this would happen, but this would add an extra layer of safety.

Let me know what you think and if anything can be improved.

@apizz
Copy link
Collaborator Author

apizz commented Aug 13, 2020

@relgit not trying to bother if you're busy, just proud that I was able to figure this out on my own :). Whenever you have time to review so I can merge this, 👍

@relgit
Copy link
Collaborator

relgit commented Aug 14, 2020

Thanks for waiting @apizz. Looking good.

I'd argue that if a preference key is both in the Chrome and the Brave-specific manifests, then the Brave-specific should prevail. This would allow us to add rudimentary modifications to existing Chrome keys if necessary.

So basically as far as I'm concerned you're good to move ahead and merge.

@apizz
Copy link
Collaborator Author

apizz commented Aug 14, 2020

Awesome, thank you @relgit ! I'm starting to work on a ChromeToEdge PR as well, but is a bit different since there are a number of chrome prefs edge doesn't use as well as prefs edge has that chrome doesn't. Hoping to get your help on that once I PR it.

@apizz
Copy link
Collaborator Author

apizz commented Nov 21, 2020

Ooof I haven't looked at this in a while. I'm going to get this committed since it is in a working state.

@apizz apizz merged commit 2972197 into master Nov 21, 2020
@apizz apizz deleted the brave-patch branch November 21, 2020 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ update manifest Update existing manifest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Sparkle preferences to Brave manifest
2 participants