Better support for primitive ops

[gbrochar]

Hello, thank you for your work.

I have run into a little problem, I wrote a function to transpose a midi file by n semi tones :

use midly::MidiMessage;
use midly::Smf;
use midly::TrackEventKind;
use std::cmp::min;

pub fn transpose_smf(smf: &mut Smf, interval: i8) {
    for track in smf.tracks.iter_mut() {
        for event in track.iter_mut() {
            if let TrackEventKind::Midi {
                channel,
                mut message,
            } = event.kind
            {
                message = match (message, interval.is_negative()) {
                    (MidiMessage::NoteOn { key, vel }, false) => MidiMessage::NoteOn {
                        key: key + (interval as u8).into(),
                        vel,
                    },
                    (MidiMessage::NoteOff { key, vel }, false) => MidiMessage::NoteOff {
                        key: key + (interval as u8).into(),
                        vel,
                    },
                    (MidiMessage::NoteOn { key, vel }, true) => {
                        if (interval.abs() as u8) > key {
                            eprintln!("warning: clamping key to 0");
                        }
                        MidiMessage::NoteOn {
                            key: key - min((interval.abs() as u8).into(), key),
                            vel,
                        }
                    },
                    (MidiMessage::NoteOff { key, vel }, true) => {
                        if (interval.abs() as u8) > key {
                            eprintln!("warning: clamping key to 0");
                        }
                        MidiMessage::NoteOff {
                            key: key - min((interval.abs() as u8).into(), key),
                            vel,
                        }
                    }
                    _ => message,
                };
                event.kind = TrackEventKind::Midi { channel, message };
            }
        }
    }
}

But as you can see I cannot simply do "u7 + i8" or something like that. Would implementing such operations with panic in debug mode for overflows (just like rust does) and overflow acting "like C" in release mode ? I would be willing to work on the project.

My desired source code would be something like :

use midly::MidiMessage;
use midly::Smf;
use midly::TrackEventKind;
use std::cmp::min;

pub fn transpose_smf(smf: &mut Smf, interval: i8) {
    for track in smf.tracks.iter_mut() {
        for event in track.iter_mut() {
            if let TrackEventKind::Midi {
                channel,
                mut message,
            } = event.kind
            {
                message = match message {
                    MidiMessage::NoteOn { key, vel } => MidiMessage::NoteOn {
                        key: key + interval,
                        vel,
                    },
                    MidiMessage::NoteOff { key, vel } => MidiMessage::NoteOff {
                        key: key + interval,
                        vel,
                    },
                    _ => message,
                };
                event.kind = TrackEventKind::Midi { channel, message };
            }
        }
    }
}

I can understand the issue with underflowing and thus having to manage the case somehow for either the library or the end user. What are your views on this ? Again I am willing to work on it, even if you give me completely different instructions on how to solve the problem.

thank you :)

[gbrochar]

Juste a little precision : it already overflows without any panic or warning using this code. Is it because of the way you created the u7 type ? underflows panics as mentionned earlier.

At least making it panic on overflow, or allowing it to underflow, to be more coherent, sounds like reasonable to me.

[gbrochar]

New code with #27

fn transpose_note_event(message: &mut MidiMessage, interval: i8) {
    *message = match *message {
        MidiMessage::NoteOn { key, vel } => MidiMessage::NoteOn {
            key: key + interval,
            vel,
        },
        MidiMessage::NoteOff { key, vel } => MidiMessage::NoteOff {
            key: key + interval,
            vel,
        },
        MidiMessage::Aftertouch { key, vel } => MidiMessage::Aftertouch {
            key: key + interval,
            vel,
        },
        _ => *message,
    };
}

[kovaxis]

For now I'd prefer to just force users to unwrap the u7 and then wrap it back. Supporting arithmetic operations on bounded types is a can of worms that I prefer not to open.