镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

A blank NoteName builds an empty Note: the binding shows as "Ctrl+", executes, then silently disappears on the next load #212

Closed

Description

@matt-edmondson

What's wrong

The two Note constructors don't validate input the same way (Keybinding/Models/MusicalTypes.cs, main @ b035bdb):

  • Note(string key) throws ArgumentException for null or whitespace.
  • Note(NoteName key) (lines 19–24, also the [JsonConstructor]) goes straight to NormalizeKey. NormalizeKey trims " " to "", and ValidateKey (line 57) only rejects input when key.Length > 1, so "" passes.

NoteName.Create(""), Create(" ") and Create("\t") all succeed, so an empty Note gets built. That contradicts the constructor's own XML doc, which says it throws "when key is null or whitespace".

Reproduction (ran against the built library)

var blank = new Note(NoteName.Create(" "));          // no throw; ToString() == ""
var chord = new Chord([new Note("Ctrl"), blank]);    // ToString() == "Ctrl+"
Chord.Parse(chord.ToString());                       // throws: Missing key at the end of "Ctrl+"

// via KeybindingManager
manager.BindChord("file.save", chord);               // True
manager.ExecuteChord(chord);                          // runs "file.save"
await manager.SaveAsync();                            // profiles.json: "notes": ["", "CTRL"]

// new manager + InitializeAsync()
manager2.GetChord("file.save");                       // null: the binding is gone

JsonSerializer.Deserialize<Profile>(JsonSerializer.Serialize(profile));
// JsonException: '' is not a valid Note key. The whole profile fails, valid bindings included

Why it matters

A host that maps framework key names to NoteName.Create(name) can end up with an empty name, for example from an unmapped key. Here is what happens to that binding:

  1. It is accepted.
  2. It displays as Ctrl+.
  3. It executes.
  4. It is lost silently on restart, because JsonKeybindingRepository calls new Note(""), which throws, and the chord is skipped.

Direct System.Text.Json deserialization of the Profile goes further: it throws for the whole profile, so its valid bindings are lost too.

This is a different bug from #156 (the unenforced ^[A-Z0-9_]+$ format rule) and from #121, #126 and #187 (null notes).

Suggested fix

  • Reject an empty key once it has been trimmed, either in NormalizeKey or at the top of Note(NoteName). Throw the same ArgumentException("Key cannot be null or whitespace", nameof(key)) that Note(string) throws, so both constructors match their docs.
  • Add test rows asserting that new Note(NoteName.Create(x)) throws ArgumentException for "", " " and "\t".
  • Add a test asserting that deserializing a Note from "" fails with the same message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions