Correct IScriptsManager resolution in Settings dialog

[RussKie]

Resolves #11253

As reported in #11239 (comment) (/cc: @SlugFiller)

[mstv]

works for me

BTW: The ScriptsSettingsPage takes much time for loading (not caused by the DI refactorings).

[RussKie]

I think, it's always been quite slow. There could be few opportunities to improve by making the scripts loading async, as well as moving away from xml serialisation.

[SlugFiller]

Well, the settings page opens now. But the script settings aren't being saved. Going to try to investigate.

[SlugFiller]

Okay, I figured it out. GitUI.ScriptsEngine.IScriptsManager does not offer an interface for altering the scripts. Therefore, in GitUI.CommandsDialogs.SettingsDialog.Pages.ScriptsSettingsPage.PageToSettings it creates a scripts list that is never used.

Also, I hate everything about the way GitUI.ScriptsEngine.GetScripts.FixAmbiguousHotkeyCommandIdentifiers is implemented. Instead of using the HashSet to find empty ids (in a defragmenting fashion), it does a max operation on the original set. Even if only doing max, it doesn't try to simply store the max and increment it.

Last, but not least, it should probably not be inside a function, so it can be used to do a sanity check on saved settings, and not just loaded ones.

Finally, the having a proxy version of the script binding interface should have probably been reserved for being able to deserialize older versions of the XML (e.g. to rename options, or change their type).

[SlugFiller]

nvm, looks like you got it covered

[RussKie]

This was the cause of the regression.

[SlugFiller]

Confirmed saving settings works now

[mstv]

saving works but many tests fail

[RussKie]

/fml I'll have a look tonight

[RussKie]

The PR build succeeded though the master is now failing.... I will have to have a look tomorrow or the day after.