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

Added support for square brackets around notes. #1175

Merged
merged 2 commits into from
Jul 4, 2016

Conversation

henryso
Copy link
Contributor

@henryso henryso commented Jul 4, 2016

Part of the implementation for #844.

No tests change, but I added a new one to exercise the new feature.

Please review and merge if satisfactory.

@eroux
Copy link
Contributor

eroux commented Jul 4, 2016

Wonderful, thanks a lot! The only thing I would change is to put the extremities of the brackets further from the staff lines (closer to the middle of the lines)... What do you think? (it must be changed in the font right?)

@henryso
Copy link
Contributor Author

henryso commented Jul 4, 2016

I don't think it's as simple as a font change, but I'll try. Edit: Looking at the figures more closely, it looks like we need "long" and "short" brackets based on whether the lowest note starts on a space versus a line.

@eroux
Copy link
Contributor

eroux commented Jul 4, 2016

Ok

@henryso
Copy link
Contributor Author

henryso commented Jul 4, 2016

It looks like we'll need three:

  • a short bracket for typical cases where the extrema are both on a space
  • a long bracket for typical cases where the extrema are both on a line
  • the current size bracket for cases where one extremum is on a space and one is on a line (but shifted up or down accordingly); this case would also handle weird cases below and above the staff.

This is turning out to be a bit complex, so it may take me some time.

@henryso
Copy link
Contributor Author

henryso commented Jul 4, 2016

I made the changes described above. Please review.

@eroux
Copy link
Contributor

eroux commented Jul 4, 2016

I think this is really perfect, thank you very much!

@henryso henryso merged commit ab18884 into gregorio-project:develop Jul 4, 2016
@henryso henryso deleted the fix-844 branch July 4, 2016 21:57
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