Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cavum glyphs #1143

Merged
merged 5 commits into from
Jun 23, 2016
Merged

Cavum glyphs #1143

merged 5 commits into from
Jun 23, 2016

Conversation

henryso
Copy link
Contributor

@henryso henryso commented Jun 15, 2016

Implemented expanded set of cavum glyphs (#844).

Note: this is a seriously big and potentially breaking pull request and should not be pulled into 4.2.

Tests obviously change because of this.

Please review and merge if satisfactory.

@eroux
Copy link
Contributor

eroux commented Jun 15, 2016

Thanks, that's quite impressive! I'll review it tonight

@eroux
Copy link
Contributor

eroux commented Jun 22, 2016

I'm getting a warning in this branch, but it may come from another PR:

struct.c: In function ‘gregorio_change_shape’:
struct.c:380:20: warning: variable ‘old_shape’ set but not used [-Wunused-but-set-variable]
     gregorio_shape old_shape;

@eroux
Copy link
Contributor

eroux commented Jun 22, 2016

Well, apart from this tiny warning everything looks really perfect! Thank you very much for this, I think this is a great feature that will allow gregorian chant typography to go one step further. Ok for me to be merged (with or without the fix for the warning, it can be for another PR)

@henryso
Copy link
Contributor Author

henryso commented Jun 22, 2016

It's most likely from here. The cavum changes reduced a lot of "special" glyph transformation code. I will try to fix it tonight and add the tests you requested.

@henryso
Copy link
Contributor Author

henryso commented Jun 23, 2016

I made the changes and added the tests. Please review at your leisure.

@eroux
Copy link
Contributor

eroux commented Jun 23, 2016

Perfect, thanks a lot!

@eroux eroux merged commit 4e17bea into gregorio-project:develop Jun 23, 2016
@henryso henryso deleted the fix-844 branch June 23, 2016 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants