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

Lyric fontsize #1469

Merged
merged 11 commits into from
Nov 9, 2019
Merged

Lyric fontsize #1469

merged 11 commits into from
Nov 9, 2019

Conversation

rpspringuel
Copy link
Contributor

Note: this includes #1467 because it would conflict with it otherwise. This is only a spacing change (#1261), and so could be included in ctan, but given the above, and that it's really such a minor issue, I've pulled it against develop instead.

Obsolete distances will now generate unrecognized distance errors instead of the obsolescence message
gsp-default.tex is turned into an internal file, not one meant for user access.  To do this we rename `\grecreatedim` to `\gre@createdim` (changing it to the internal namespace) and add a version check to the file.
We also change how scaling is applied when loading a space configuration file.  Instead of rescaling all distances when the space configuration file is loaded, we modify `\grechangedim` so that it individually checks to see if the distance being changed needs to be rescaled.  This allows user `gsp-*.tex` files to be partial, only defining the spacings they want to customize.
Some tests are revealing a fault, these debug messages are to help me identify where.
`\greconffactor` is set to 0 when we're not loading a space configuration file.  `\grechangedim` should only do its rescaling when inside a space configuration file (i.e. `\greconffactor` is not 0).
The value of `spacelinestext` (distance from the lines to the baseline of the lyrics) was previously a fixed value, which meant that especially large lyrics would overlap with low notes (and small lyrics would seem very far from the lines).  By changing it to a font based value, the lyrics should naturally adjust their position based on their size.  The converted value corresponds to keeping the lyrics in the same place for 11pt Alegrya (the test font), as this causes the fewest tests to change.
@henryso
Copy link
Contributor

henryso commented Jun 27, 2019

This looks OK to merge to me. In your opinion, how "breaking" is this to people in the middle of a project? In other words, if you are in the middle of the project, would you want to be cautious when updating? If so, then this is probably left out of CTAN until a release with a proper UPGRADE.md entry or at least better warning.

@rpspringuel
Copy link
Contributor Author

If the project contains only scores at size 17 (the default) or are using their own spacing configurations, then the change isn't breaking at all. For any project which contains scores at other sizes but still use the default spacings (just scaled) then this will make a difference. I think the difference is an improvement, but others might disagree. If you look at some of the ancillary tests which changed you can get a sense for how much of a difference the change makes.

This is already against develop, and thus as is won't go to CTAN until next year. To push it to CTAN earlier would require me to redo the PR (and not include #1467). I can add an UPGRADE.md entry for it.

@henryso
Copy link
Contributor

henryso commented Jun 27, 2019

In that case, I have no issues with this.

@henryso henryso merged commit f4d9b27 into gregorio-project:develop Nov 9, 2019
@rpspringuel rpspringuel deleted the lyric_fontsize branch November 12, 2019 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants