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

Implemented a toggle-able forced hyphen for empty initial syllables. #657

Merged
merged 4 commits into from
Oct 25, 2015

Conversation

henryso
Copy link
Contributor

@henryso henryso commented Oct 24, 2015

Fixes #653.

I changed the wording a bit from #656 since this is now part of 4.0.

Please review and merge if satisfactory.

@@ -790,16 +790,23 @@ \subsubsection{End of Line Behavior}
\subsubsection{Hyphenation}

\macroname{\textbackslash gresethyphen}{\{\#1\}}{gregoriotex-main.tex}
Tell Gregorio\TeX\ to when to place a hyphen between
syllables in polysyllabic words in a score. This is done by overriding \texttt{maximumspacewithoutdash} so subsequent changes to this dimension will override this command.
Tells Gregorio\TeX\ how to place a hyphen between syllables in polysyllabic words in a score. This is done by overriding \texttt{maximumspacewithoutdash} so subsequent changes to this dimension will override this command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize that this is old code that you haven't actually changed, but it got me thinking.
Given that you've introduced a flag \ifgre@forcehyphen my naive expectation would be that this should be used here instead of altering maximumspacewithoutdash.

Perhaps your flag should be called \ifgre@showhyphen instead, in which case we could revamp the way this setting works so that it doesn't have to alter maximumspacewithoutdash (but that sort of optimization need not be part of this change).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's tackle that as a separate issue.

It would be a little more complex than what you mention. I introduced that flag so I could have an "or" condition that causes the hyphen to appear: either it's the first initial and the hyphen is forced or the spacing requires a hyphen. As such, this flag is reset by \GreSyllable. We could use it for \gresethyphen, which should then need to set its own flag (rather than adjusting maximumspacewithoutdash) which would then be used as an additional clause of the above "or" condition.

This would be a pure-TeX change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that reworking \gresethyphen's internals would be a separate issue. I'm just saying that the name \ifgre@forcehyphen makes me think you're doing what that issue would address, rather than what you're actually addressing here.

Basically the only issue I have right now is in your choice of names, but if you want to stick with it as is, I can confirm that everything works as expected and is ready for merge.

@henryso
Copy link
Contributor Author

henryso commented Oct 25, 2015

@rpspringuel Thanks for clarifying. I renamed the flag.

@rpspringuel
Copy link
Contributor

You missed the occurance in the documentation.

@henryso
Copy link
Contributor Author

henryso commented Oct 25, 2015

Fixed. Thanks.

rpspringuel added a commit that referenced this pull request Oct 25, 2015
Implemented a toggle-able forced hyphen for empty initial syllables.
@rpspringuel rpspringuel merged commit 871f99c into gregorio-project:release-4.0 Oct 25, 2015
@henryso henryso deleted the fix-653 branch October 25, 2015 17:13
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