Make markers.edit apply its documented color parameter

Found while testing crates/engine/src/cmd/event.rs:338 (edit_marker; params doc at line 97)

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
NetterSilver
2026-10-15 17:54:03 +02:00
co-authored by Claude Haiku 5.5
parent 3a28a65769
commit eb4e2d21ba
+40 -1
View File
@@ -94,7 +94,7 @@ pub fn specs() -> Vec<CommandSpec> {
add_marker
),
cmd!("markers.delete", "Delete Memory Location", [], None, "{number | id}", always, delete_marker),
cmd!("markers.edit", "Edit Memory Location", [], None, "{number | id, name?, comments?, at?, color?}", always, edit_marker),
cmd!("markers.edit", "Edit Memory Location", [], None, "{number | id, name?, comments?, at?, color?: [r,g,b] | null}", always, edit_marker),
cmd!(noundo "markers.recall", "Recall Memory Location", [], Some("Period + n + Period"), "{number}", always, recall_marker),
cmd!(noundo "markers.next", "Go to Next Marker", [], None, "{}", always, |e, _| step_marker(e, 1)),
cmd!(noundo "markers.previous", "Go to Previous Marker", [], None, "{}", always, |e, _| step_marker(e, -1)),
@@ -337,8 +337,23 @@ fn delete_marker(e: &mut Engine, p: &Value) -> Result<Value> {
fn edit_marker(e: &mut Engine, p: &Value) -> Result<Value> {
let i = marker_index(e, p, "markers.edit")?;
let at = position_param(e, "markers.edit", p, "at")?;
let color = match p.get("color") {
None => None,
Some(Value::Null) => Some(None),
Some(Value::Array(a)) if a.len() == 3 => {
let ch = |i: usize| a.get(i).and_then(Value::as_u64).and_then(|v| u8::try_from(v).ok());
match (ch(0), ch(1), ch(2)) {
(Some(r), Some(g), Some(b)) => Some(Some([r, g, b])),
_ => return Err(bad("markers.edit", "`color` components must be integers 0..=255")),
}
}
Some(_) => return Err(bad("markers.edit", "`color` must be [r,g,b] or null")),
};
let s = e.session_mut();
let Some(m) = s.markers.get_mut(i) else { return Err(bad("markers.edit", "gone")) };
if let Some(c) = color {
m.color = c;
}
// `name` is the documented new name; when it was the selector it already matches, so this is a no-op.
if let Some(n) = str_param(p, "rename").or_else(|| str_param(p, "name")) {
m.name = n.to_string();
@@ -443,6 +458,30 @@ mod tests {
assert_eq!((bridge.name.as_str(), bridge.comments.as_str()), ("Bridge", "last"));
}
/// `markers.edit` documents `color`: it sets the memory location's RGB colour, undoably; `null` clears it.
#[test]
fn edit_marker_sets_documented_color() {
let mut e = demo_engine();
let n = e.session().markers.first().unwrap().number;
let color = |e: &crate::Engine| e.session().markers.iter().find(|m| m.number == n).unwrap().color;
assert_eq!(color(&e), None);
e.execute("markers.edit", &json!({"number": n, "color": [255, 0, 0]})).unwrap();
assert_eq!(color(&e), Some([255, 0, 0]));
assert!(e.undo());
assert_eq!(color(&e), None);
assert!(e.redo());
assert_eq!(color(&e), Some([255, 0, 0]));
e.execute("markers.edit", &json!({"number": n, "color": null})).unwrap();
assert_eq!(color(&e), None);
for bad in [json!([1, 2]), json!([1, 2, 300]), json!("red"), json!([1, 2, -3])] {
assert!(e.execute("markers.edit", &json!({"number": n, "color": bad})).is_err());
assert_eq!(color(&e), None);
}
}
/// Constant replaces the tempo over [start, end) only; the tempo at the old end tick carries on after it.
#[test]
fn tempo_constant_is_bounded_by_its_range() {