diff --git a/Keybinding.Test/ChordRejectsPhraseTests.cs b/Keybinding.Test/ChordRejectsPhraseTests.cs
new file mode 100644
index 0000000..6dcdabd
--- /dev/null
+++ b/Keybinding.Test/ChordRejectsPhraseTests.cs
@@ -0,0 +1,89 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Test;
+
+using ktsu.Keybinding.Core.Models;
+using ktsu.Keybinding.Core.Services;
+
+///
+/// Tests that a chord field rejects a phrase string or a key containing whitespace or a comma, rather than
+/// inventing a key such as "K, CTRL" that can never be pressed.
+///
+[TestClass]
+public class ChordRejectsPhraseTests
+{
+ private static KeybindingService CreateService() => new(new CommandRegistry(), new ProfileManager());
+
+ [TestMethod]
+ [DataRow("Ctrl+K, Ctrl+C")]
+ [DataRow("Ctrl+K,Ctrl+C")]
+ [DataRow("A, B")]
+ [DataRow("Ctrl+,,")]
+ public void ChordParse_PhraseString_Throws(string value)
+ {
+ Assert.ThrowsExactly(() => Chord.Parse(value));
+ Assert.ThrowsExactly(() => CreateService().ParseChord(value));
+ }
+
+ [TestMethod]
+ [DataRow("Page Up")]
+ [DataRow("Ctrl+Page Up")]
+ [DataRow("Ctrl+,x")]
+ public void ChordParse_KeyWithWhitespaceOrComma_Throws(string value)
+ {
+ Assert.ThrowsExactly(() => Chord.Parse(value));
+ Assert.ThrowsExactly(() => CreateService().ParseChord(value));
+ }
+
+ [TestMethod]
+ [DataRow("K, CTRL")]
+ [DataRow("PAGE UP")]
+ [DataRow("A\tB")]
+ [DataRow("A+B")]
+ public void Note_KeyWithWhitespaceOrSeparator_Throws(string key)
+ {
+ Assert.ThrowsExactly(() => new Note(key));
+ }
+
+ [TestMethod]
+ [DataRow(",", ",")]
+ [DataRow("+", "+")]
+ [DataRow(" , ", ",")]
+ public void Note_SeparatorKeyOnItsOwn_IsValid(string key, string expected)
+ {
+ Assert.AreEqual(expected, new Note(key).Key.ToString());
+ }
+
+ [TestMethod]
+ public void ChordParse_CtrlComma_StillParses()
+ {
+ Chord chord = Chord.Parse("Ctrl+,");
+
+ Assert.HasCount(2, chord.Notes);
+ Assert.AreEqual(chord, CreateService().ParseChord("Ctrl+,"));
+ }
+
+ [TestMethod]
+ [DataRow("Ctrl+Alt+S")]
+ [DataRow("Ctrl+,")]
+ [DataRow("Ctrl++")]
+ [DataRow("Shift+F5")]
+ [DataRow(",")]
+ [DataRow("+")]
+ public void ChordParse_ToStringOfValidChord_RoundTrips(string value)
+ {
+ Chord chord = Chord.Parse(value);
+
+ Assert.AreEqual(chord, Chord.Parse(chord.ToString()));
+ }
+
+ [TestMethod]
+ public void PhraseParse_PhraseString_StillSplitsIntoChords()
+ {
+ Phrase phrase = Phrase.Parse("Ctrl+K, Ctrl+C");
+
+ Assert.HasCount(2, phrase.Sequence);
+ Assert.AreEqual(Chord.Parse("Ctrl+K"), phrase.Sequence[0]);
+ Assert.AreEqual(Chord.Parse("Ctrl+C"), phrase.Sequence[1]);
+ }
+}
diff --git a/Keybinding/Models/KeyStringTokenizer.cs b/Keybinding/Models/KeyStringTokenizer.cs
index a349fa4..33633a5 100644
--- a/Keybinding/Models/KeyStringTokenizer.cs
+++ b/Keybinding/Models/KeyStringTokenizer.cs
@@ -20,7 +20,7 @@ internal static class KeyStringTokenizer
///
/// The chord string.
/// The trimmed note strings, or an empty array for whitespace input.
- /// Thrown when a key is missing, as in "Ctrl+" or "A++B".
+ /// Thrown when a key is missing, as in "Ctrl+" or "A++B", or when a ',' follows a key, as in the phrase "Ctrl+K, Ctrl+C".
internal static string[] SplitChord(string value) => Split(value, NoteSeparator, ChordSeparators);
///
@@ -42,6 +42,8 @@ private static string[] Split(string value, char splitOn, string separators)
{
char c = value[i];
+ EnsureNotChordSeparatorInChord(value, i, splitOn, expectKey);
+
if (separators.Contains(c) && !expectKey)
{
// A separator after a key ends the note, and ends the token when it is the one being split on
@@ -86,6 +88,18 @@ private static string[] Split(string value, char splitOn, string separators)
return [.. tokens];
}
+ ///
+ /// In a chord, a ',' after a key starts the next chord of a phrase, which a chord cannot hold.
+ /// A ',' where a key is expected is still the comma key, as in "Ctrl+,".
+ ///
+ private static void EnsureNotChordSeparatorInChord(string value, int index, char splitOn, bool expectKey)
+ {
+ if (splitOn == NoteSeparator && !expectKey && value[index] == ChordSeparator)
+ {
+ throw new ArgumentException($"Unexpected '{ChordSeparator}' at position {index} in \"{value}\": a chord cannot contain a sequence of chords; parse it as a phrase instead", nameof(value));
+ }
+ }
+
///
/// A separator where a key is expected is the key itself, but only when nothing else follows it before the next
/// separator. Otherwise the input has an empty key, which must not be dropped.
diff --git a/Keybinding/Models/MusicalTypes.cs b/Keybinding/Models/MusicalTypes.cs
index a1abbb2..16e514c 100644
--- a/Keybinding/Models/MusicalTypes.cs
+++ b/Keybinding/Models/MusicalTypes.cs
@@ -14,7 +14,7 @@ public sealed class Note : IEquatable
/// Initializes a new instance of the class
///
/// The key that this note represents
- /// Thrown when key is null or whitespace
+ /// Thrown when key is null or whitespace, or contains whitespace, '+' or ',' other than as the single-character key "+" or ","
[JsonConstructor]
public Note(NoteName key)
{
@@ -27,7 +27,7 @@ public Note(NoteName key)
/// Initializes a new instance of the class from a string
///
/// The key string that this note represents
- /// Thrown when key is null or whitespace
+ /// Thrown when key is null or whitespace, or contains whitespace, '+' or ',' other than as the single-character key "+" or ","
public Note(string key)
{
if (string.IsNullOrWhiteSpace(key))
@@ -44,7 +44,19 @@ public Note(string key)
///
/// A key name in any case
/// The canonical key name
- private static string NormalizeKey(string key) => CanonicalizeKey(key.Trim().ToUpperInvariant());
+ private static string NormalizeKey(string key) => CanonicalizeKey(ValidateKey(key.Trim()).ToUpperInvariant());
+
+ ///
+ /// Rejects a key that contains whitespace or a separator, such as "Page Up" or "K, Ctrl", which no key press
+ /// can produce. The separator keys "+" and "," are valid on their own.
+ ///
+ /// A trimmed key name
+ /// The key name, unchanged
+ /// Thrown when the key contains whitespace, '+' or ','
+ private static string ValidateKey(string key) =>
+ key.Length > 1 && key.Any(c => char.IsWhiteSpace(c) || c is ',' or '+')
+ ? throw new ArgumentException($"Key \"{key}\" cannot contain whitespace, '+' or ','", nameof(key))
+ : key;
///
/// Maps a modifier alias to its canonical key name, so that every way of building a note