Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions Keybinding.Test/ChordConflictTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,84 @@ public void SetChords_TwoCommandsOneChord_KeepsOneBinding()
Assert.AreEqual(1, manager.Profiles.GetActiveProfile()!.ChordCount);
}

[TestMethod]
public void SetChords_TwoCommandsOneChord_CountsOnlyTheBindingThatRemains()
{
using KeybindingManager manager = new(_testDataDirectory);
manager.Commands.RegisterCommand(new Command("a", "A"));
manager.Commands.RegisterCommand(new Command("b", "B"));
manager.CreateDefaultProfile();

int count = manager.SetChords(new Dictionary<string, Chord> { ["a"] = Chord.Parse("Ctrl+S"), ["b"] = Chord.Parse("Ctrl+S") }, out IReadOnlyList<string> displaced);

Assert.AreEqual(1, count);
Assert.AreEqual(manager.Profiles.GetActiveProfile()!.ChordCount, count);
Assert.HasCount(1, displaced);
Assert.AreEqual("a", displaced[0]);
Assert.AreEqual("b", manager.Keybindings.ExecuteChord(Chord.Parse("Ctrl+S")));
}

[TestMethod]
public void SetChords_TwoCommandsOneChord_WithoutOutParameter_ReturnsTheRemainingCount()
{
using KeybindingManager manager = new(_testDataDirectory);
manager.Commands.RegisterCommand(new Command("a", "A"));
manager.Commands.RegisterCommand(new Command("b", "B"));
manager.CreateDefaultProfile();

int count = manager.SetChords(new Dictionary<string, Chord> { ["a"] = Chord.Parse("Ctrl+S"), ["b"] = Chord.Parse("Ctrl+S") });

Assert.AreEqual(1, count);
}

[TestMethod]
public void SetChords_ChordHeldOutsideTheBatch_ReportsTheCommandThatLostIt()
{
using KeybindingManager manager = new(_testDataDirectory);
manager.Commands.RegisterCommand(new Command("save", "Save"));
manager.Commands.RegisterCommand(new Command("saveAll", "Save All"));
manager.Commands.RegisterCommand(new Command("open", "Open"));
manager.CreateDefaultProfile();
manager.Keybindings.BindChord("save", Chord.Parse("Ctrl+S"));

int count = manager.SetChords(new Dictionary<string, Chord> { ["saveAll"] = Chord.Parse("Ctrl+S"), ["open"] = Chord.Parse("Ctrl+O") }, out IReadOnlyList<string> displaced);

Assert.AreEqual(2, count);
Assert.HasCount(1, displaced);
Assert.AreEqual("save", displaced[0]);
}

[TestMethod]
public void SetChords_CommandRebindByTheSameBatch_IsNotReportedAsDisplaced()
{
using KeybindingManager manager = new(_testDataDirectory);
manager.Commands.RegisterCommand(new Command("a", "A"));
manager.Commands.RegisterCommand(new Command("b", "B"));
manager.CreateDefaultProfile();
manager.Keybindings.BindChord("a", Chord.Parse("Ctrl+S"));

// b takes Ctrl+S from a, and the same batch moves a to Ctrl+D: a swap, not a lost binding
int count = manager.SetChords(new Dictionary<string, Chord> { ["b"] = Chord.Parse("Ctrl+S"), ["a"] = Chord.Parse("Ctrl+D") }, out IReadOnlyList<string> displaced);

Assert.AreEqual(2, count);
Assert.IsEmpty(displaced);
}

[TestMethod]
public void SetChords_NoConflict_ReportsNothingDisplaced()
{
using KeybindingManager manager = new(_testDataDirectory);
manager.Commands.RegisterCommand(new Command("a", "A"));
manager.Commands.RegisterCommand(new Command("b", "B"));
manager.CreateDefaultProfile();
manager.Keybindings.BindChord("a", Chord.Parse("Ctrl+A"));

int count = manager.SetChords(new Dictionary<string, Chord> { ["a"] = Chord.Parse("Ctrl+A"), ["b"] = Chord.Parse("Ctrl+B") }, out IReadOnlyList<string> displaced);

Assert.AreEqual(2, count);
Assert.IsEmpty(displaced);
}

[TestMethod]
public void BindChord_SameCommandSameChord_KeepsTheBinding()
{
Expand Down
48 changes: 45 additions & 3 deletions Keybinding/KeybindingManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,58 @@
/// Sets multiple chord bindings for the active profile
/// </summary>
/// <param name="chords">Dictionary of command ID to chord mappings</param>
/// <returns>Number of chord bindings successfully set</returns>
public int SetChords(IReadOnlyDictionary<string, Chord> chords)
/// <returns>
/// Number of bindings from the batch that remain once it is applied. A chord runs only one command, so when
/// the batch gives one chord to two commands the later entry takes it and only that binding is counted.
/// Use <see cref="SetChords(IReadOnlyDictionary{string, Chord}, out IReadOnlyList{string})"/> to learn which
/// commands lost their chord.
/// </returns>
public int SetChords(IReadOnlyDictionary<string, Chord> chords) => SetChords(chords, out _);

/// <summary>
/// Sets multiple chord bindings for the active profile, and reports the commands that lost a chord to them
/// </summary>
/// <param name="chords">Dictionary of command ID to chord mappings</param>
/// <param name="displacedCommandIds">
/// The IDs of the commands whose chord was taken by a later binding: commands outside the batch that held one
/// of its chords, and entries of the batch whose chord a later entry took. A command that the batch itself
/// rebinds, and that keeps its new binding, is not listed. Each ID appears once, in the order it was displaced.
/// </param>
/// <returns>Number of bindings from the batch that remain once it is applied</returns>
public int SetChords(IReadOnlyDictionary<string, Chord> chords, out IReadOnlyList<string> displacedCommandIds)
{
DisposalHelper.ThrowIfDisposed(_disposed, this);
Ensure.NotNull(chords);

Profile activeProfile = Profiles.GetActiveProfile() ?? throw new InvalidOperationException("No active profile is set");

List<string> displaced = [];
Dictionary<string, Chord> bound = [];

// Bind into the profile resolved above, so a concurrent SetActiveProfile cannot split the batch
return OperationHelper.ExecuteWithCount(chords, (commandId, chord) => Keybindings.BindChord(activeProfile.Id, commandId, chord));
OperationHelper.ExecuteWithCount(chords, (commandId, chord) =>
{
string[] holders = [.. activeProfile.GetAllChords().Where(b => b.Value.Equals(chord) && b.Key != commandId.Trim()).Select(b => b.Key)];
if (!Keybindings.BindChord(activeProfile.Id, commandId, chord))
{
return false;
}

bound[commandId.Trim()] = chord;
foreach (string holder in holders)

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View workflow job for this annotation

GitHub Actions / ci / .NET / Analyze & Release

Loops should be simplified using the "Where" LINQ method

Check warning on line 311 in Keybinding/KeybindingManager.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Loops should be simplified using the "Where" LINQ method

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_Keybinding&issues=AaGhM2EBIodzg0bXKvfV&open=AaGhM2EBIodzg0bXKvfV&pullRequest=215
{
if (!chord.Equals(activeProfile.GetChord(holder)) && !displaced.Contains(holder))
{
displaced.Add(holder);
}
}
Comment on lines +311 to +317

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Leaving this one as it is: it's a style suggestion with no change in behaviour. The guard also reads displaced, which the loop adds to, so the explicit if makes that dependency easier to see than a lazy Where would. Happy to fold it in if a later push touches this method.


Generated by Claude Code


return true;
});

// A command the batch rebinds after it lost its old chord is not displaced, as long as it keeps the new one
displacedCommandIds = [.. displaced.Where(id => !(bound.TryGetValue(id, out Chord? chord) && chord.Equals(activeProfile.GetChord(id))))];
return bound.Count(b => b.Value.Equals(activeProfile.GetChord(b.Key)));
}

/// <summary>
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ var chords = new Dictionary<string, Chord>
{ "edit.paste", manager.Keybindings.ParseChord("Ctrl+V") }
};
int set = manager.SetChords(chords);

// A chord runs one command, so a later binding takes it from an earlier one.
// The count is the bindings that remain; the out overload names the commands that lost their chord.
int kept = manager.SetChords(chords, out IReadOnlyList<string> displaced);
```

### Profile Management
Expand Down
Loading