Parse "Ctrl++" and "Ctrl+," as the plus and comma keys

Keybinding.Test/SeparatorKeyParsingTests.cs

@@ -0,0 +1,129 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Test;
+
+using ktsu.Keybinding.Core;
+using ktsu.Keybinding.Core.Contracts;
+using ktsu.Keybinding.Core.Models;
+
+/// <summary>
+/// Tests that the '+' and ',' keys can be bound, and that input with a missing key is rejected rather than dropped.
+/// </summary>
+[TestClass]
+public class SeparatorKeyParsingTests
+{
+	private static KeybindingManager CreateManager() =>
+		new(Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString()));
+
+	[TestMethod]
+	public void ChordParse_CtrlPlus_IsCtrlAndPlusKey()
+	{
+		Chord chord = Chord.Parse("Ctrl++");
+
+		Assert.AreNotEqual(Chord.Parse("Ctrl"), chord);
+		Assert.HasCount(2, chord.Notes);
+		Assert.Contains(n => n.Key.ToString() == "+", chord.Notes);
+	}
+
+	[TestMethod]
+	public void ChordParse_CtrlComma_IsCtrlAndCommaKey()
+	{
+		Chord chord = Chord.Parse("Ctrl+,");
+
+		Assert.HasCount(2, chord.Notes);
+		Assert.Contains(n => n.Key.ToString() == ",", chord.Notes);
+	}
+
+	[TestMethod]
+	public void ChordParse_PlusAlone_IsPlusKey()
+	{
+		Chord chord = Chord.Parse("+");
+
+		Assert.HasCount(1, chord.Notes);
+		Assert.AreEqual("+", chord.Notes.Single().Key.ToString());
+	}
+
+	[TestMethod]
+	public void PhraseParse_CtrlComma_IsSingleChordWithCommaKey()
+	{
+		Phrase phrase = Phrase.Parse("Ctrl+,");
+
+		Assert.HasCount(1, phrase.Sequence);
+		Assert.AreEqual(Chord.Parse("Ctrl+,"), phrase.Sequence[0]);
+		Assert.AreNotEqual(Phrase.Parse("Ctrl"), phrase);
+	}
+
+	[TestMethod]
+	public void PhraseParse_CommaKeyFollowedByChord_SplitsCorrectly()
+	{
+		Phrase phrase = Phrase.Parse("Ctrl+,, R");
+
+		Assert.HasCount(2, phrase.Sequence);
+		Assert.AreEqual(Chord.Parse("Ctrl+,"), phrase.Sequence[0]);
+		Assert.AreEqual(Chord.Parse("R"), phrase.Sequence[1]);
+	}
+
+	[TestMethod]
+	public void PhraseParse_PlusKeyInSequence_SplitsCorrectly()
+	{
+		Phrase phrase = Phrase.Parse("Ctrl++, Ctrl+R");
+
+		Assert.HasCount(2, phrase.Sequence);
+		Assert.AreEqual(Chord.Parse("Ctrl++"), phrase.Sequence[0]);
+		Assert.AreEqual(Chord.Parse("Ctrl+R"), phrase.Sequence[1]);
+	}
+
+	[TestMethod]
+	public void Parse_RoundTripsThroughToString()
+	{
+		Phrase phrase = Phrase.Parse("Ctrl+,, Ctrl++");
+
+		Assert.AreEqual(phrase, Phrase.Parse(phrase.ToString()));
+	}
+
+	[TestMethod]
+	[DataRow("Ctrl+")]
+	[DataRow("A++B")]
+	[DataRow("Ctrl++A+")]
+	public void ChordParse_MissingKey_Throws(string value) =>
+		Assert.ThrowsExactly<ArgumentException>(() => Chord.Parse(value));
+
+	[TestMethod]
+	[DataRow("Ctrl+R,")]
+	[DataRow("Ctrl+R,,R")]
+	[DataRow(", R")]
+	public void PhraseParse_MissingChord_Throws(string value) =>
+		Assert.ThrowsExactly<ArgumentException>(() => Phrase.Parse(value));
+
+	[TestMethod]
+	public void ServiceParseChord_CtrlPlus_MatchesChordParse()
+	{
+		using KeybindingManager manager = CreateManager();
+		IKeybindingService service = manager.Keybindings;
+
+		Chord chord = service.ParseChord("Ctrl++");
+
+		Assert.HasCount(2, chord.Notes);
+		Assert.AreEqual(Chord.Parse("Ctrl++"), chord);
+	}
+
+	[TestMethod]
+	public void ServiceParsePhrase_CtrlComma_MatchesPhraseParse()
+	{
+		using KeybindingManager manager = CreateManager();
+		IKeybindingService service = manager.Keybindings;
+
+		Phrase phrase = service.ParsePhrase("Ctrl+,");
+
+		Assert.AreEqual(Phrase.Parse("Ctrl+,"), phrase);
+	}
+
+	[TestMethod]
+	public void ServiceParseChord_MissingKey_Throws()
+	{
+		using KeybindingManager manager = CreateManager();
+		IKeybindingService service = manager.Keybindings;
+
+		Assert.ThrowsExactly<ArgumentException>(() => service.ParseChord("Ctrl+"));
+	}
+}

Keybinding/Models/KeyStringTokenizer.cs

@@ -0,0 +1,106 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Keybinding.Core.Models;
+
+using System.Text;
+
+/// <summary>
+/// Splits chord and phrase strings into tokens, treating a separator character that appears where a key is
+/// expected as the literal key, so "Ctrl++" is Ctrl and Plus and "Ctrl+," is Ctrl and Comma.
+/// </summary>
+internal static class KeyStringTokenizer
+{
+	private const char NoteSeparator = '+';
+	private const char ChordSeparator = ',';
+	private const string ChordSeparators = "+";
+	private const string PhraseSeparators = "+,";
+
+	/// <summary>
+	/// Splits a chord string such as "Ctrl+Alt+S" or "Ctrl++" into note strings.
+	/// </summary>
+	/// <param name="value">The chord string.</param>
+	/// <returns>The trimmed note strings, or an empty array for whitespace input.</returns>
+	/// <exception cref="ArgumentException">Thrown when a key is missing, as in "Ctrl+" or "A++B".</exception>
+	internal static string[] SplitChord(string value) => Split(value, NoteSeparator, ChordSeparators);
+
+	/// <summary>
+	/// Splits a phrase string such as "Ctrl+R, R" or "Ctrl+,, R" into chord strings.
+	/// </summary>
+	/// <param name="value">The phrase string.</param>
+	/// <returns>The trimmed chord strings, or an empty array for whitespace input.</returns>
+	/// <exception cref="ArgumentException">Thrown when a key or chord is missing, as in "Ctrl+" or "A,,B".</exception>
+	internal static string[] SplitPhrase(string value) => Split(value, ChordSeparator, PhraseSeparators);
+
+	private static string[] Split(string value, char splitOn, string separators)
+	{
+		List<string> tokens = [];
+		StringBuilder current = new();
+		bool expectKey = true;
+		bool sawContent = false;
+
+		for (int i = 0; i < value.Length; i++)
+		{
+			char c = value[i];
+
+			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
+				if (c == splitOn)
+				{
+					tokens.Add(current.ToString().Trim());
+					current.Clear();
+				}
+				else
+				{
+					current.Append(c);
+				}
+
+				expectKey = true;
+				continue;
+			}
+
+			if (separators.Contains(c))
+			{
+				EnsureSeparatorIsKey(value, i, separators);
+			}
+
+			current.Append(c);
+			if (!char.IsWhiteSpace(c))
+			{
+				expectKey = false;
+				sawContent = true;
+			}
+		}
+
+		if (!sawContent)
+		{
+			return [];
+		}
+
+		if (expectKey)
+		{
+			throw new ArgumentException($"Missing key at the end of \"{value}\"", nameof(value));
+		}
+
+		tokens.Add(current.ToString().Trim());
+		return [.. tokens];
+	}
+
+	/// <summary>
+	/// 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.
+	/// </summary>
+	private static void EnsureSeparatorIsKey(string value, int index, string separators)
+	{
+		int next = index + 1;
+		while (next < value.Length && char.IsWhiteSpace(value[next]))
+		{
+			next++;
+		}
+
+		if (next < value.Length && !separators.Contains(value[next]))
+		{
+			throw new ArgumentException($"Missing key before '{value[index]}' at position {index} in \"{value}\"", nameof(value));
+		}
+	}
+}

Keybinding/Models/MusicalTypes.cs

@@ -167,7 +167,7 @@
 	private static bool IsModifierNote(Note note)
 	{
 		string key = note.Key.ToString().ToUpperInvariant();
 		return key is "CTRL" or "CONTROL" or "ALT" or "SHIFT" or "META" or "WIN" or "WINDOWS" or "CMD" or "COMMAND";
 	}
 
 	/// <summary>
@@ -262,7 +262,7 @@
 			throw new ArgumentException("Value cannot be null or whitespace", nameof(value));
 		}
 
-		string[] parts = [.. value.Split('+', StringSplitOptions.RemoveEmptyEntries).Select(p => p.Trim())];
+		string[] parts = KeyStringTokenizer.SplitChord(value);
 
 		if (parts.Length == 0)
 		{
@@ -396,8 +396,7 @@
 			throw new ArgumentException("Value cannot be null or whitespace", nameof(value));
 		}
 
-		string[] chordStrings = [.. value.Split(',', StringSplitOptions.RemoveEmptyEntries)
-			.Select(s => s.Trim())];
+		string[] chordStrings = KeyStringTokenizer.SplitPhrase(value);
 
 		return chordStrings.Length == 0
 			? throw new ArgumentException("Invalid phrase format", nameof(value))

Keybinding/Services/KeybindingService.cs

@@ -36,7 +36,7 @@
 			throw new ArgumentException("Profile ID cannot be null or whitespace", nameof(id));
 		}
 
 		ArgumentException.ThrowIfNullOrWhiteSpace(name, nameof(name));
 
 		return _profileManager.CreateProfile(id, name, description);
 	}
@@ -183,7 +183,7 @@
 		}
 
 		// Split by comma to get individual chord strings
-		string[] chordStrings = phraseString.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
+		string[] chordStrings = KeyStringTokenizer.SplitPhrase(phraseString);
 		List<Chord> chords = [];
 
 		foreach (string chordString in chordStrings)
@@ -204,7 +204,7 @@
 		}
 
 		// Split by + to get individual note strings
-		string[] noteStrings = chordString.Split('+', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
+		string[] noteStrings = KeyStringTokenizer.SplitChord(chordString);
 		List<Note> notes = [];
 
 		foreach (string noteString in noteStrings)