[PowerDisplay] Add stable profile IDs#49175
Draft
moooyo wants to merge 4 commits into
Draft
Conversation
Give every saved PowerDisplay profile a stable, auto-incrementing integer id and address profiles by id across the app instead of by name, so duplicate names are allowed and renames are non-lossy. - Models: PowerDisplayProfile.Id (JSON id, 0 = unassigned) + monotonic PowerDisplayProfiles.NextId (JSON nextId); id-keyed SetProfile, GetById, RemoveProfile(int), idempotent EnsureIds() backfill, DisplayName 'Name (#id)'. - App: startup BackfillProfileIds migration (backfills ids, then migrates the LightSwitch name->id mapping); ApplyProfileByIdAsync; the ApplyProfile pipe message now carries the id (int.TryParse). LightSwitchProfileResolver resolves id-first and does not fall back to a stale/deleted id. - Settings UI: apply/edit/delete profiles by id; the editor preserves the id on edit; LightSwitch stores both id and name and selects by id; lists/pickers show DisplayName. LightSwitchSettings.Clone now copies the id fields. Tests: PowerDisplayProfilesTests (id math, EnsureIds, DisplayName) and LightSwitchProfileIdTests (settings id round-trip, Clone preserves ids). Split from the PowerDisplay CLI branch to target main independently; the CLI-specific apply-profile-by-id integration remains with the CLI PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
10 tasks
Contributor
Author
Cross-reference: split out of #48632This PR is split out of #48632 —
Merge relationship: if this PR merges first, #48632 will be rebased onto |
Settings UI now back-fills stable ids (EnsureIds) on profile load in both PowerDisplayViewModel and LightSwitchViewModel, so editing a legacy (id == 0) profile replaces it in place instead of creating a duplicate when the app hasn't migrated yet. Also fixes the transient (#0) DisplayName. Clear a stale LightSwitch profile reference (id/name) that no longer resolves, guarded on a non-empty profile list so a transient load failure can't wipe a valid selection. Remove the now-unused ProfileHelper.RenameAndUpdateProfile dead code, and add a regression test (EnsureIds_ThenEditByAssignedId_ReplacesInsteadOfDuplicating). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move LightSwitchProfileResolver into Settings.UI.Library so the app and Settings UI share one id-first/name-fallback implementation; add HasReference and reuse both in SelectByStoredReference (drops the duplicated "(None)" logic). - Add atomic ProfileHelper.LoadProfilesEnsuringIds() and use it in the two Settings UI load paths (removes duplication and the load/save race window). - Simplify EnsureIds() with Math.Max/LINQ (behavior-preserving). - Remove now-dead name-based ProfileHelper.RemoveProfile(string) and the PowerDisplayProfiles.RemoveProfile(string) overload (delete is id-keyed now). - Add LightSwitchProfileResolverTests (now testable in the shared lib). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- PowerDisplayViewModel: CreateProfile/UpdateProfile became identical once UpdateProfile switched to the id-keyed AddOrUpdateProfile; fold both into a private UpsertProfile(profile, isNew). - LightSwitchViewModel: extract SetSelectedProfile(ref field, value, isDarkMode, name) to remove the symmetric dark/light selection-setter duplication; drop the unused legacy DarkModeProfile/LightModeProfile string VM properties (no bindings) and their RefreshModuleSettings notifications. - MainViewModel.Settings: extract LoadValidProfileById() shared by ApplyProfileByIdAsync and the LightSwitch apply path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or 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
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.
Summary of the Pull Request
Gives every saved PowerDisplay profile a stable, auto-incrementing integer id and makes the app address profiles by that id instead of by name. Duplicate profile names are now allowed (the id disambiguates them) and renames are non-lossy for anything that references a profile.
PR Checklist
PowerDisplayProfilesTests(id math /EnsureIds/DisplayName),LightSwitchProfileIdTests(settings id round-trip +Clonepreserves ids), andLightSwitchProfileResolverTests(id-first resolution / stale-id no-fallback / name→id migration) inPowerDisplay.Lib.UnitTests.#idsuffix is a formatted label, not a resource).Detailed Description of the Pull Request / Additional comments
Model (
PowerDisplay.Models)PowerDisplayProfile.Id— JSONid,int,0= unassigned; plusDisplayName("{Name} (#{Id})",[JsonIgnore], UI only).PowerDisplayProfiles.NextId— JSONnextId, a monotonic counter that only ever increases and is never reused.SetProfileis id-keyed: a profile withId == 0is assigned the next id; an existing id replaces in place (so a rename keeps the same id). Names are not required to be unique.GetById,RemoveProfile(int)/ProfileHelper.RemoveProfileById, and an idempotentEnsureIds()that back-fills ids for legacy profiles and self-healsnextIdpast the highest id in use.ProfileHelper.LoadProfilesEnsuringIds()loads then back-fills ids atomically (single lock). The now-dead name-basedRemoveProfile(string)overloads (helper + container) were removed — deletes are id-keyed.App (
PowerToys.PowerDisplay)BackfillProfileIds()runs inside the existing legacy-monitor-id side-file migration (before its no-monitor early return) so an existingprofiles.jsonis upgraded in place with no data loss, then migrates the LightSwitch mapping from name to id.ApplyProfileByIdAsync(int); theApplyProfileNamed Pipe message now carries the id (parsed withint.TryParse).LightSwitchProfileResolver(see below) resolves id-first, and does not fall back to a stale/deleted id (so a removed profile can't be silently replaced by a same-named different one), plus a one-shot name→id migration.Settings UI
*ProfileIdand the name and selects the current item by id via the sharedLightSwitchProfileResolver(SelectByStoredReference);LightSwitchSettings.Clone()now copies the id fields (previously they were dropped, which would reset the stored ids on the next settings save).DisplayName, so duplicate names are distinguishable in the UI.Cleanup (follow-up commits)
Moved
LightSwitchProfileResolverintoSettings.UI.Libraryso the app (LightSwitchService) and the Settings UI (SelectByStoredReference) share one id-first / name-fallback implementation (Resolve+HasReference), removing a duplicated decision and the hard-coded"(None)"sentinel that previously lived on both sides.LoadProfilesEnsuringIds()collapses the repeated load →EnsureIds→ save in the two Settings UI load paths into a single atomic call (also closing the previous two-lock race window between load and save).Simplified
EnsureIds()(Math.Max+ LINQ, behavior-preserving) and removed the dead name-based remove overloads described above.PowerDisplayViewModel:CreateProfile/UpdateProfilecollapsed into one privateUpsertProfile(they became identical onceUpdateProfilemoved to the id-keyedAddOrUpdateProfile).LightSwitchViewModel: extractedSetSelectedProfile(...)to drop the symmetric dark/light selection-setter duplication, and removed the now-unused legacyDarkModeProfile/LightModeProfilestring VM properties (no bindings) plus their notifications.App
MainViewModel.Settings: extractedLoadValidProfileById()shared byApplyProfileByIdAsyncand the LightSwitch apply path.Validation Steps Performed
x64/Debug, VS MSBuild, exit 0):PowerDisplay.Models/PowerDisplay.Lib,PowerToys.PowerDisplay(app),Settings.UI.Library,PowerToys.Settings(Settings UI), andPowerDisplay.Lib.UnitTests.PowerDisplay.Lib.UnitTests: full suite green (156/156), including 17 profile-id tests — 8 model (PowerDisplayProfilesTests), 2 LightSwitch settings (LightSwitchProfileIdTests), and 7 resolver (LightSwitchProfileResolverTests).