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

Bug: CommandManager.OverwriteKeyBindings System.ArgumentException #15331

Open
yaira2 opened this issue May 7, 2024 · 10 comments
Open

Bug: CommandManager.OverwriteKeyBindings System.ArgumentException #15331

yaira2 opened this issue May 7, 2024 · 10 comments
Assignees
Labels
area-actions Features or bug reports relating to actions bug Something isn't working 👀 crash Bug reports involving a crash

Comments

@yaira2
Copy link
Member

yaira2 commented May 7, 2024

Description

System.ArgumentException: An element with the same key but a different value already exists. Key: 'Alt+Ctrl+Haut'

Steps To Reproduce

NA

Requirements

  • Fix crash

Files Version

v3.4

Windows Version

Windows 11

Log File

System.Collections.Immutable.ImmutableDictionary`2
HashBucket.Add (TKey key, TValue value, IEqualityComparer`1 keyOnlyComparer, IEqualityComparer`1 valueComparer, KeyCollisionBehavior behavior, OperationResult& result)
System.Collections.Immutable
ImmutableDictionary`2.AddRange (IEnumerable`1 items, MutationInput origin, KeyCollisionBehavior collisionBehavior)
System.Collections.Immutable
ImmutableDictionary`2.AddRange (IEnumerable`1 pairs, Boolean avoidToHashMap)
Files.App.Data.Commands
CommandManager.OverwriteKeyBindings ()
Files.App.Utils.Serialization
BaseObservableJsonSettings.Set[TValue] (TValue value, String propertyName)
Files.App.Services.Settings
ActionsSettingsService.set_ActionsV2 (List`1 value)
Files.App.ViewModels.Settings
ActionsViewModel.ExecuteSaveCommand (ModifiableActionItem item)
CommunityToolkit.Mvvm.Input
RelayCommand`1.Execute (Object parameter)
ABI.System.Windows.Input.ICommand
Vftbl.Do_Abi_Execute_3 (IntPtr thisPtr, IntPtr parameter)
WinRT
ExceptionHelpers.<ThrowExceptionForHR>g__Throw|39_0 (Int32 hr)
ABI.Windows.ApplicationModel.Core
IUnhandledErrorMethods.Propagate (IObjectReference _obj)
Microsoft.AppCenter.Utils
ApplicationLifecycleHelperWinUI.<ctor>b__0_3 (Object sender, UnhandledErrorDetectedEventArgs eventArgs)
@yaira2 yaira2 added the bug Something isn't working 👀 label May 7, 2024
@yaira2 yaira2 added the crash Bug reports involving a crash label May 7, 2024
@0x5bfa
Copy link
Member

0x5bfa commented May 8, 2024

Yeah when user set the same key to different commands, this happens. I’ve ever seen this crash before once.

before adding keybindings to dictionary, we can check if.

@yaira2
Copy link
Member Author

yaira2 commented May 8, 2024

Yeah when user set the same key to different commands

Don't we already prevent duplicate key bindings?

@0x5bfa
Copy link
Member

0x5bfa commented May 8, 2024

If user set via text editor, we cannot prevent beforehand.

@0x5bfa
Copy link
Member

0x5bfa commented May 13, 2024

Oh, sorry. I didn't know this issue was urgent

P.S. Sorry again, closed mistakenly

@0x5bfa 0x5bfa closed this as completed May 13, 2024
@0x5bfa 0x5bfa reopened this May 13, 2024
@0x5bfa
Copy link
Member

0x5bfa commented May 13, 2024

@Josh65-2201 cloud you please mark as Ready again? I’m gonna work on.

@yaira2
Copy link
Member Author

yaira2 commented May 13, 2024

Done 👍

@0x5bfa
Copy link
Member

0x5bfa commented May 13, 2024

What approach is better? This is caused by editing the json wrongly.

  1. Reset everything (emptying the collection)
  2. Reset only the hotkey(s) which is the same as the other, which is set earlier.
  3. Ignore everything, use defaults, then notify through a dialog?.

@yaira2
Copy link
Member Author

yaira2 commented May 13, 2024

Reset everything (emptying the collection)

I don't think we need to go that far

A combination of approach two and three is best, we can display a prompt and ask if the user would like to reset the conflicting hotkeys or if they would like to manually edit the settings file themselves.

@0x5bfa
Copy link
Member

0x5bfa commented May 13, 2024

In that case, I assume Files has to know if the json has been corrected, and close the dialog, as the load is done once on startup. Also, the load of the custom hot keys is performed before MainWindow.Content is set; this issue was a blocker when I tried to prompt a dialog notifying incorrect of json format.

I believe we need to improve json parser classes but can't we just reset only it for now? It sounds like it's not a big deal to remove a single hotkey customization.

@yaira2
Copy link
Member Author

yaira2 commented May 13, 2024

If that's a blocker, we can go with option 2 and reset the conflicting key.

@yaira2 yaira2 added the area-actions Features or bug reports relating to actions label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-actions Features or bug reports relating to actions bug Something isn't working 👀 crash Bug reports involving a crash
Projects
Status: 🔖 Ready to build
Development

No branches or pull requests

2 participants