Skip to content

Make KeyBindings generic not show folder specific (#6545)#6571

Open
cybercop23 wants to merge 2 commits into
xLightsSequencer:masterfrom
cybercop23:general_keybindings
Open

Make KeyBindings generic not show folder specific (#6545)#6571
cybercop23 wants to merge 2 commits into
xLightsSequencer:masterfrom
cybercop23:general_keybindings

Conversation

@cybercop23

Copy link
Copy Markdown
Collaborator

#6545 - now use the same location as preferences/options.

Copilot AI review requested due to automatic review settings June 18, 2026 01:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR changes where xlights_keybindings.xml is stored/loaded so key bindings are no longer show-folder-specific, aligning the storage location with the existing JSON settings (“AppData”/user config directory) behavior.

Changes:

  • Exposes GetSettingsFilePath() as a public helper in XLightsConfigAdapter so other UI code can use the platform AppData settings location.
  • Updates show-folder selection (xLightsFrame::SetDir) to load keybindings from the AppData settings directory instead of the current show folder.
  • Adds a one-time migration that copies an existing show-folder keybindings file into the AppData location if the new file doesn’t exist yet.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src-ui-wx/settings/XLightsConfigAdapter.h Declares GetSettingsFilePath() for reuse outside the adapter implementation.
src-ui-wx/settings/XLightsConfigAdapter.cpp Makes GetSettingsFilePath() non-static so it can be called from other translation units.
src-ui-wx/app-shell/TabSetup.cpp Loads keybindings from AppData and performs a one-time migration from the show folder.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 484 to 486
wxFileName kbf;
kbf.AssignDir(CurrentDir);
kbf.AssignDir(GetSettingsFilePath().parent_path().string());
kbf.SetFullName(XLIGHTS_KEYBINDING_FILE);
Comment on lines +493 to +495
if (legacy.FileExists()) {
wxCopyFile(legacy.GetFullPath(), kbf.GetFullPath());
}
@cybercop23 cybercop23 changed the title Make KeyBindings generic not shoe folder specific (#6545) Make KeyBindings generic not show folder specific (#6545) Jun 18, 2026
@derwin12

Copy link
Copy Markdown
Contributor

I think this is a step backwards ... what if you want specific halloween settings vs christmas settings. Or you host multiple shows. Wonder if there is a better solution here..

@cybercop23

Copy link
Copy Markdown
Collaborator Author

I didn't think you'll want different key bindings for different shows, defeats the purpose. I can add the same logic as options, if there's one in the show folder use that over instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants