Re: which-page (issue 6352049)
LGTM http://codereview.appspot.com/6352049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: which-page (issue 6352049)
On 2012/06/29 10:13:42, t.daniels_treda.co.uk wrote: Thomas Morley wrote Friday, June 29, 2012 10:32 AM Of course all features (old and new) should be documented. But, AFAICT, non of the possibilities offered in /ly/titling-init.ly (i.e. first-page, last-page, not-first-page, part-last-page) is documented anywhere in LM or NR. Well, they should be documented, but I propose to open a new issue to do so. Eluze already opened an issue for this - 2579 - and I'm part-way through a fix to the docs. I'll upload it for review shortly, and I'd appreciate your comments and help. Will do. I found only one multi-page-file in the regression-tests containing a feature of titling-init.ly (i.e. `not-first-page'): input/regression/page-breaks.ly But I think this file tests something else. So I'd suggest to open a new issue for that, too. Fine - please do. Regtest added now. -Harm http://codereview.appspot.com/6352049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: which-page (issue 6352049)
2012/6/28 d...@gnu.org: A feature should be documented, or it will not be discoverable. Of course all features (old and new) should be documented. But, AFAICT, non of the possibilities offered in /ly/titling-init.ly (i.e. first-page, last-page, not-first-page, part-last-page) is documented anywhere in LM or NR. The only place they are used in the docs is: Documentation/snippets/book-parts.ly Well, they should be documented, but I propose to open a new issue to do so. If feasible, a regtest should be added in input/regression. Whether this is possible for essentially multi-page features, I don't know, so it is possible that this particular feature does not really lend itself to a regtest. I found only one multi-page-file in the regression-tests containing a feature of titling-init.ly (i.e. `not-first-page'): input/regression/page-breaks.ly But I think this file tests something else. So I'd suggest to open a new issue for that, too. http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly File ly/titling-init.ly (right): http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly#newcode104 ly/titling-init.ly:104: #(define ((which-page nmbr) layout props arg) which-page seems like a less than optimal name for the intended use patterns: \on-the-fly #(which-page 3) ... reads less natural than \on-the-fly #(on-page 3) ... or \on-the-fly #(at-page 3) ... Done http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly#newcode105 ly/titling-init.ly:105: (if (= (chain-assoc-get 'page:page-number props 0) nmbr) The functions below seem to prefer -1 for something coming without page number. Perhaps it would make sense to conform with that kind of usage. Done http://codereview.appspot.com/6352049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: which-page (issue 6352049)
On Fri, Jun 29, 2012 at 11:32:53AM +0200, Thomas Morley wrote: 2012/6/28 d...@gnu.org: A feature should be documented, or it will not be discoverable. Well, they should be documented, but I propose to open a new issue to do so. That sounds sensible. In the past we've said that new features do not need to be documented in the same patch that adds it. If feasible, a regtest should be added in input/regression. Whether this is possible for essentially multi-page features, I don't know, so it is possible that this particular feature does not really lend itself to a regtest. I found only one multi-page-file in the regression-tests containing a feature of titling-init.ly (i.e. `not-first-page'): input/regression/page-breaks.ly But I think this file tests something else. So I'd suggest to open a new issue for that, too. Here I disagree slightly; it would be really good if new features were covered by the regtests as soon as they were added, so that we have a chance of noticing when they break. It doesn't need to be in a new regtest; adding it to an exising regtest is fine. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: which-page (issue 6352049)
Thomas Morley wrote Friday, June 29, 2012 10:32 AM Of course all features (old and new) should be documented. But, AFAICT, non of the possibilities offered in /ly/titling-init.ly (i.e. first-page, last-page, not-first-page, part-last-page) is documented anywhere in LM or NR. Well, they should be documented, but I propose to open a new issue to do so. Eluze already opened an issue for this - 2579 - and I'm part-way through a fix to the docs. I'll upload it for review shortly, and I'd appreciate your comments and help. I found only one multi-page-file in the regression-tests containing a feature of titling-init.ly (i.e. `not-first-page'): input/regression/page-breaks.ly But I think this file tests something else. So I'd suggest to open a new issue for that, too. Fine - please do. Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
which-page (issue 6352049)
Reviewers: , Message: It's possible to add a markup to a footer/header of a specified page, using #(which-page page-number). Multiple settings are possible. Please review as http://codereview.appspot.com/6352049 -Harm Description: which-page It's possible to add a markup to a footer/header of a specified page, using (which-page page-number). Multiple settings are possible. Please review this at http://codereview.appspot.com/6352049/ Affected files: M ly/titling-init.ly Index: ly/titling-init.ly diff --git a/ly/titling-init.ly b/ly/titling-init.ly index f6daacddb399cf6bac89b3bcccbb291bab242f82..9f3c644a9cbbdfb170498fdfbf3432bd5a601d43 100644 --- a/ly/titling-init.ly +++ b/ly/titling-init.ly @@ -101,6 +101,11 @@ book last one. (interpret-markup layout props arg) empty-stencil)) +#(define ((which-page nmbr) layout props arg) + (if (= (chain-assoc-get 'page:page-number props 0) nmbr) + (interpret-markup layout props arg) + empty-stencil)) + %% Bookpart first page and last page predicates #(define (part-first-page layout props arg) (if (= (chain-assoc-get 'page:page-number props -1) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: which-page (issue 6352049)
A feature should be documented, or it will not be discoverable. If feasible, a regtest should be added in input/regression. Whether this is possible for essentially multi-page features, I don't know, so it is possible that this particular feature does not really lend itself to a regtest. http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly File ly/titling-init.ly (right): http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly#newcode104 ly/titling-init.ly:104: #(define ((which-page nmbr) layout props arg) which-page seems like a less than optimal name for the intended use patterns: \on-the-fly #(which-page 3) ... reads less natural than \on-the-fly #(on-page 3) ... or \on-the-fly #(at-page 3) ... http://codereview.appspot.com/6352049/diff/1/ly/titling-init.ly#newcode105 ly/titling-init.ly:105: (if (= (chain-assoc-get 'page:page-number props 0) nmbr) The functions below seem to prefer -1 for something coming without page number. Perhaps it would make sense to conform with that kind of usage. http://codereview.appspot.com/6352049/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
