I would like to rewrite the CLI part with structopt, if you're fine with that. I think it would make the whole command & argument thing a lot easier to look at, understand and expand on. Also I would like to resolve the panics occurring, when you're missing arguments, along with that.

(Independent from this specific Issue, I think it would make sense to have some kind of discussion platform for the further library design. I would like to contribute some more, but am very unsure if others would be fine with the changes or how I'd do them. And opening an issue for each possible feature I can think of seems a bit much. GitHub Discussions is still in Beta sadly..)

Command line output:

❯ rustmt scale D Locrian
error: Found argument 'D' which wasn't expected, or isn't valid in this context
USAGE:
    rustmt.exe scale <SUBCOMMAND>

For more information try --help

Same thing happens when using something like rustmt scale C Minor

Installed on a Windows 10 machine using cargo install.

Executable

• Added support for inversions using slash syntax
  • C major triad / E is C major in first inversion
  • The implementation is similary error-proof to the other from_regex functions, so C/E and C major /E give the same result as the above
• Added inversion example to chord help

chord.rs

• Added inversion: u8 to Chord
• Added new constructor with_inversion, and made new construct a root-position chord by default
  • @ozankasikci correctly critiqued a similar design pattern I used for descending scales, but I think root position chords are desirable in the vast majority of cases, and that they are the "default" voicing of chords unless otherwise stated
• Modularized chord_intervals into its own function, giving the intervallic layout of a chord
• Added inversion support to from_regex
  • It checks if there is a / in the input, and if so, treats everything after the slash as a PitchClass
    • It uses PitchClass::from_regex, so it is error-safe
  • If a slash is given, followed by a valid pitch class, then:
    • The chord is first constructed in root position
    • The position of the given bass note is determined in the root position chord
    • A new chord is build with the determined inversion
  • Minimal overhead is created for invalid/root position inversions
• impl Notes for Chord generates the notes of the root position chord, rotates them based on the inversion number, and corrects the octaves such that they 1) are non-decreasing and 2) start at self.octave

test_chord.rs

• test_all_chords_in_c now generates and tests all inversions of each chord type on C
• test_inversion_octaves tests against hard-coded octave lists for every inversion of a G major ninth chord

Caveat: There is no standard (to my knowledge) for voicing inverted chords beyond the octave, i.e. placing G in G major 9 / B. For this reason (and to maximize simplicity and flexibility), my implementation enforces a non-decreasing order; G major 9 = B4 D5 F#5 A5 G6

This PR documents everything and adds #![warn(missing_docs)] to ensure that everything is documented.

I also ran rustfmt on the code and introduced a couple useful additions such as implementing FromStr for PitchClass, adding Mode::is_diatonic, and implementing From<Mode> for ScaleType.

After a few iterations of trying different ways, I think it's in a somewhat useful form now. Sadly not as advantages as I hoped it would be, but definitely a lot nicer to look at and more structured. For more improvements on top of this we'd need to solve the issues/implement the ideas I opened issues/discussions for (#25, #26, #27). When doing the #26 refactor, we can then also put the direction of the scale into the from_str and use it in the CLI, at the moment this is a regression, as direction now can't be specified. I want to fix this as soon as this is accepted or it's clear it will, so I can then commit on top of this branch (if that makes any sense). #27 will help with a lot better error messages, that can tell you what exact part of parsing failed.

Original PR is #15 by @jagen31

On top of #15, this PR:

• Resolves the merge with master
• Renames structs in accordance with music theory conventions
• Added some abstractions to reduce and simplify code

Changes from master

• Several renames produce this new scheme:
  • NoteLetter: A, B, C, etc. Just the letters of the white keys on a piano
  • Pitch: A note letter and an accidental
    • New related abstraction PitchSymbol, an enum { Cb, C, Cs, Db...} convertible to Pitch for code brevity
  • Note wraps a Pitch with an octave
• More extensive accidental support in parsing, including double sharps/flats in format

Some notes going forward (pun intended)

• Cb breaks everything as a result of the current design of integer conversions
  • If C4 is flatted, does it produce Cb4 or Cb3. It's enharmonic to B3 but relative to C. Either solution brings up some dependency questions.
• Some extra considerations will be necessary for double flats and sharps to work throughout the library
• The above two can be fixed alongside enharmonic support, which I'll be happy to tackle next

@ozankasikci tests run perfectly, but the actual test code and style consistency could use a good lookover

Changes:

Executable

• Added new -d, --descending option to scale command
  • Returns the scale in descending order
  • Uses the direction property of Scale, so this approach allows descending Melodic Minor to eventually be different from ascending

Interval

• Added static method Interval::invert(&Interval) that returns the inverse of the given interval as a Result
  • This uses the musical set theory definition of inverse
  • Identities: P0→P0, P8→P8, TT→TT
  • Other examples: P5→P4, M2→m7
• Added member function second_note_down_from(self, Note) that is the opposite of second_note_from
  • Correctly accounts for octaves
• Added static method to_notes_reverse(Note, impl IntoIterator<Item = Interval>) that is the opposite ofto_notes`
  • Takes the list of intervals backwards and builds downwards from the given root
• A new folder of tests just for intervals, including extensive testing of inverse intervals

Pitch Class

• Added static method from_interval_down(Self, Interval) that gets the pitch class created by descending the given pitch class by the given interval
  • Uses a modified modulus formula: b+ (c - d)) % b to prevent errors and allow eventual addition of compound intervals (like M13)

Scale

• Modified the new constructor to accept a Direction
• Made a similar pair of changes by adding from_regex_in_direction
• Added check for direction in impl Notes::notes for Scale
• Reworked test of all scales in C
  • They all actually start on C now
  • Tuples of basic descriptors of scales, then the constructors are used to make ascending and descending versions

Added some missing Clone and Displays derives for easier library usage. Needed to clone Notes myself and added the other ones for completion.

Also added PitchClass::into_u8 for cases when you need the number representation, in my case for calculating the MIDI note number.

Hi everyone! This project looks neat. I'm noticing in the examples, however, that the enharmonic spellings for the note names are incorrect. For example,

rustmt scale D Locrian is shown with this output: Notes: 1: D 2: D# 3: F 4: G 5: G# 6: A# 7: C 8: D

But it should be notated as follows:

1. D
2. Eb
3. F
4. G
5. Ab
6. Bb
7. C
8. D

In other words, each letter name in a scale (and in chords) should be notated with the correct name relative to the interval.

This PR just makes the file structure more idiomatic by creating mod.rs files for each module of the library. I think doing a similar thing by creating a main.rs for the binary might have some benefits as well, at least when we get started on a GUI.

Supplements #13

• Adds test test_octave_increment to test_scales.rs
  • Tests that the octave field increases at the right note (C6 as 4th note of G Mixolydian)
  • Depends on second_note_from and verifies scale generation
  • Intended to test #13 -- It will fail until that is merged

Hi! I discovered this repo and found a dangling PR, so here's the test requested by @ozankasikci. Very nice and intuitive project, by the way!

Edit: If your checks are blocking the merge, then just message me when #13 is merged. I can then rebase on master, and then the checks will rerun successfully.

First of all, hi everybody, thanks for making such a cool app 😀

FYI please let me know if there's any problems and I'll be glad to fix 'er up. I'm just getting into both Rust and Music Theory so I'm sure I've botched a couple things here.

Support entering notes in the CLI to get the chord

Entering the chord command followed by notes will return the chord name, or an error if it's an invalid/unknown chord.

$ rustmt chord C E G
C Major Triad

$ rustmt chord E G C
C Major Triad, 1st Inversion

$ rustmt chord C Cs D
Invalid/Unknown Chord!

This change is three parts:

1. Implementing the Display trait for Chord
   • Prints "Root Quality Number, Inversion"
2. Changing Chord::from_string to return a Result instead of panicking
3. Checking the length of CLI arguments for the chord command
   • If any argument is >2 characters long it can't be a note, so the user is entering a chord name and is asking for notes
   • Thus, if all the arguments are <=2 characters they're all notes and the user is asking for a chord name

Inversion support in Chord::from_string

Can't be sure that the first note in a string of notes is the root- for example, somebody could enter E G C. So we should check for inversions, if possible.

I did this by first checking against interval patterns for chords in their root position (using the existing code). For instance, the interval [4, 3] means Major Triad in the root position, e.g. C E G. If there are no matches for root position chords, then we check some interval patterns for inversions. For instance, the interval [3, 5] means Major Triad 1st inversion, e.g. E G C.

Unfortunately the inversion support is pretty sparse so far. We can check Major and Minor Triads and that's about it. I plan on finding more patterns, but some chord positions cannot be deciphered from interval patterns alone. For instance, Augmented Triads have a [4, 4] interval, so you cannot distinguish between the root position or either of the inversions by using the interval pattern alone since it's the same for every position. I'm wondering what the best behavior is in a case like this: just list off all the possible chords?

Is that correct that there are only n inversions generated where n is the number of notes in the chord?

https://github.com/ozankasikci/rust-music-theory/blob/2e09fa3c2574fd41a41650e4f95023ac9f7e3b20/src/chord/chord.rs#L191

As far as I understand, there should be n! of them to produce all the permutations.

AFAIK, according to the music theory, there only exists the invariant that the notes in chords shouldn't contain spaces more than one octave and this reduces the number of inversions of a given chord starting for a given octave exactly n!

I was expecting this to output "DGA", but it outputs "DGD". But then I'm new to rust and basically cobbled together stuff from the tests I didn't understand. Where am I going wrong here?

extern crate rust_music_theory as rustmt;
use rustmt::note::Notes;
use rustmt::chord::Chord;

fn main() {
    let chord = Chord::from_regex("D sus4").unwrap();

    for note in chord.notes().iter() {
        print!("{}", note)
    }
}

I would propose to use the builder pattern for those, because for most fields there are good defaults, we could add new ways to build them and especially for the case of calculating the inversion from a bass_note. I would like to expose that feature to the library side and not only have it in the CLI. I suggest the typed-builder crate for this, as there is no need to write the boilerplate by hand and it also brings the compile time check.

The documentation is a bit outdated atm, after merging #20, the API will change dramatically and the gap will be even larger. We need to;

• Update that crate documentation
• Update the README file

If someone would like to pick that up please let the community know. Otherwise, I'll be working on that in the coming days.