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 [nocustos] to suppress an end-of-line custos at that point. #1273

Merged
merged 3 commits into from
Jan 6, 2017

Conversation

henryso
Copy link
Contributor

@henryso henryso commented Jan 6, 2017

Fixes #1271.

None of the current tests fail, and I added a test for this feature.

Please review and merge if satisfactory.

@eroux
Copy link
Contributor

eroux commented Jan 6, 2017

I think it misses an entry into CHANGELOG, and maybe also in the gabc part of the documentation...

The [nocustos] thing can be applied to anything right? That's quite a good idea!

also, currently the test only tests [nocustos] on bars and breaks with custos on regular notes, maybe it could be mixed?

Apart from that, good job, thanks a lot!

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

It's unclear to me what you mean by "mixed". Would you please elaborate?

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

The gabc part of the documentation does not (unfortunately) currently describe the actual syntax of gabc and rather covers major features that have been added (relatively) recently. I don't think there's a good place in the current documentation to add something about this feature. I think it's OK to leave it out until we do finally document the syntax.

@eroux
Copy link
Contributor

eroux commented Jan 6, 2017

Well, I think it's necessary to document it somewhere, and enven though the gabc part of the pdf is not really the best place, at least it's somewhere. When someone will compile a gabc documentation, he'll take this as a reference (among other things), and so he'll document [nocustos], so I think it's helpful to put it there, even though it won't be very useful right now... What do you think?

About mixed, something like

foo... (::[nocustos]) line break
bar... (::) line break with custos
foo... (a[nocustos]b) line break between a and b
bar... (ab) same
foo... (a[nocustos]) (b)
bar... (a) (b)

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

In trying to get (a[nocustos]b) to work, I have noticed that GregorioTeX is very willing to push the b past the end of the staff (and this happens even if not using [nocustos] at all):

image

I'm not sure what's going on here or how to fix it.

@eroux
Copy link
Contributor

eroux commented Jan 6, 2017

well, without the actual gabc code it's difficult to debug, but you can leave it for another issue if there's something wrong, we can merge this one before...

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

I added these "mixed" tests, and you can see the weird pushing past the margin that I mentioned above. Do you have any ideas how I might be able to fix it?

@eroux
Copy link
Contributor

eroux commented Jan 6, 2017

Hmmm I don't know, maybe the very long texts are too twisted for Gregorio to handle... maybe the most simple solution is to just remove them....

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

I adjusted the test to look nice (and also test the cases you wanted tested). However, if I touch it in certain ways, like removing a g from the ...f f f f[nocustos] g g g... sequence, it will again push past the end of the staff.

I still have to work on the documentation.

@henryso
Copy link
Contributor Author

henryso commented Jan 6, 2017

Documentation is in and (I think) the tests are complete. Please review.

@eroux
Copy link
Contributor

eroux commented Jan 6, 2017

Looks perfect, thanks a lot!

@henryso henryso merged commit a4195b3 into gregorio-project:develop Jan 6, 2017
@henryso henryso deleted the fix-1271 branch January 6, 2017 20:53
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