Re: markup-command and markup-command-list signatures
I am working on a system of markups which allows to specify more flexible formatting rules. WE are using it for things like multi-line embedded scores, mixing them with markup lines, rules about what things / combinations of things should not start / end a line, also there are rules like "no line break between certain words and beginning ebmbedded score", that kind of formatting rules. I had described some of these ideas in my earlier posts on this list. Markup functions being able to return a list of stencils. Much more importantly, markups need to be aware of what was placed before, and what is to follow, therefore when processing the markup-list, we need to pass a continuation at each step, instead of iterating over the list. This kind of ideas. It even sort of works. Well, works enough for production use by non-programmer users. But still far from being a general Lilypond improvement. The other, easier improvements (orphan-avoiding functionality, page-breaking fixes), are making it fine into the upstream repo: for those, going from the happy state of having the user's problem fixed, to the happier state of fixing it for everyone, is of a reasonable proportion compared to the whole amount of work. But with my markup changes, it's much different. Even the first and simplest of these changes (patch 207105), to go from the current state to an actual submittable patch, will take like 2x the time it took to get it to solve the user's probem. For the bigger problems, like the "markup needs to know what's before and what's ahead", or for the integrated text/embedded-score flow, I don't know, could be up to 5x, and now we are suddenly looking at problems of user value, and all the repercussions. So there is development happening which is important to users (= enables a serious academic publication through a top-name publisher), but those contributions can not be used better than just being thrown away by the community. I remember reading on the LKML, kernel guys having somewhat similar problems with vendor patches and integration with the vanilla tree. I don't (yet) know how we should deal with this, just thinking aloud while there is this discussion about the same area of code... > that is, any number of scheme arguments, then any number of single markup > > arguments, then any number of markup-list arguments, even though I don't > > know if having several markup list arguments is useful or not (and if it's > > doable). ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
On Sun, May 2, 2010 at 6:19 PM, David Kastrup wrote: >> For one reason or another, whenever I review code from you it degrades >> into a fight. I am not quite sure why this always happens. > > Because you don't bother taking the contributions of other people > seriously. You don't read the code comments, even after you asked for Thanks for explaining this; it's obviously my fault. I'll go back to where I came from. cheers, -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
Han-Wen Nienhuys writes: > I apologise - I opened the page again, and expected it to always show > the latest patch set, which it didn't obviously. That, coupled with a > lengthy explanation of about using state machines made assume > erroneously that you did not change the .ll code The explanation was not "lengthy", and you obviously did not bother reading my reply through. Neither did you bother reading the comments that you asked for. > The lexer code looks OK now - you could remove the markup_p variables; > ly_module_constant() memoizes the lookup, so the second lookup will > basically be a noop. There are two different lookups in alteration, and I can't find the definition of ly_lily_module_constant. > As for the scheme code, it would be nice if the validity check were > moved in a separate pure function, so the setter does > > (define (set-signature cmd sig) > (if (valid-sig? sig) (set-property .. .) > (ly:error (_ "invalid signature for command))) I'll think about it. > For one reason or another, whenever I review code from you it degrades > into a fight. I am not quite sure why this always happens. Because you don't bother taking the contributions of other people seriously. You don't read the code comments, even after you asked for them. You apply utterly different standards to the code which is everywhere else and written by you and "trusted" people. You reject code that obviously took hours to write and comment by an experienced programmer without even investing the time needed to read the comments. If you think that the code has to be trivial enough to be understood by you in 5 minutes (or instantly), why do you think it took years for someone to actually write it? > Could we mutually assume that when the other party makes comments or > writes improvements, that all of this happens in good faith, so if > something seems wrong, we assume the other party made an innocent > mistake? Could we also assume that the other party's intelligence is not so much below our own that something must be wrong if we need to more than superficially glance at his code? I've actually thrown out all of the C code checking for the validity of the signature since your original comments made abundantly clear that there was no way that I could write straightforward C code checking for a valid sequence of argument types that you would ever accept. Because the _problem_ was at a level for which you were not going to accept code from me. And even though the part you complained about was contained in about 10 lines of code, you did not even bother checking the comments carefully about what it was supposed to do. And proposed substituting it with about 4 lines of trivial code without bothering to entertain the thought that the original code was not looking more complex thatn that because it solved a problem more complex than that? Even discounting that your last batch of comments was for an old version (again, what kind of moron do you think you are dealing with when you can't discern a single change after his explanation that he'll rip everything out and do it differently?), could you at least bother checking what problem some code is trying to solve (it states so in the comments) before rejecting it because it is not more trivial than the problem it solves? That is the main problem I have with you "making comments" and "writing improvements": you don't take the other side serious enough to entertain the thought that he might be doing something for a good reason instead of plain stupidity. You don't bother to actually read through explanations in code comments, or in Rietveld comments. Not carefully enough to understand what they are talking about. And I find this kind of condescension decidedly unrewarding. If you want to be taken seriously, you should extend the same courtesy to others. It very much kills motivation to participate with Lilypond if you don't. And it also does not help that you apply double standards with regard to code you are willing to let into Lilypond, since a new contributor can't take the existing code as a guideline for how to write things in an acceptable way. Try investing at least one minute of thought for every hour of thought that a contribution required. How do you expect _not_ to miss the point when you don't? Not every problem has a trivial solution. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
On Sun, May 02, 2010 at 06:10:21PM +0100, Trevor Daniels wrote: > > Carl Sorensen wrote Sunday, May 02, 2010 5:49 PM > >> On 5/2/10 4:45 AM, "Mark Polesky" wrote: >> >>> * Use bar-checks (`|') only when barring is unclear. >> >> I think we should always use bar-checks when the piece is more than one >> bar >> long. That's a good habit to get into; we ought to start it right >> from the >> first. > > I would agree with this. In fact I put bar checks into > quite a few of the examples in the LM originally, but they > seem to have been removed. We didn't have a consistent policy on bar-checks, and one of the new doc editors was eager to work on the code formatting. I figured it was a fairly harmless way to get experience with git, so I accepted his patches removing barline checks from simple music. I've now expended my 1 hour of lilypond work for today; as mentioned before, I'm only doing 1 hour a day until May 7 due to various university requirements (the biggest of which is my yearly PhD report, which decides if I get any money next year). oh, Valentin -- sorry, but my contributions for the already-delayed Report will have to wait until tomorrow. Or the day after, or the day after, depending on how much time is required for any urgent emails or patch reviewing. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
Graham Percival wrote: >> If anyone is up to it, I'd like someone to look over the >> patch to see if I've made anything worse. > > I started looking, and saw some questionable stuff. I'll > take a longer look later today, after I wake up and have > coffee. (the patch is at http://codereview.appspot.com/1056041 ) Before anyone gets too worked up about this patch, I think I should explain my rationale for proposing these changes. First, if you haven't looked at it yet, you should know that one direct result of the patch is the increase in length of many of the examples (vertically). Graham has already expressed his dissatisfaction with this, since it increases the chances of individual examples being split across pages in the PDF version. But the assumption, that I think things like this are too hard to read on one line, is incorrect: \new Staff { \clef treble c4 } My rationale (and feel free to defend your opposition) is: 1) that we should have a basic "programming style" that most (if not all) of us can agree on, 2) that the programming style should make ly code easier to * read (eg. as on the mailing lists) * modify (eg. being able to move stuff from one voice to another and back without needing to change some note's relative octave each time) * debug (eg. \lilycommands are harder to comment out if they're in the middle of a line of notes) * update (ie. with convert-ly), and 3) that the docs should use that style consistently, because among other things, I think the docs should always present the best example of good formatting. So do I think that this specific case is hard to read? \new Staff { \clef treble c4 } No. But I *do* think that doing this instead is a "good coding habit", that should be consistently employed: \new Staff { \clef "treble" c4 } And do I care about examples of programming code being split across pages in the PDF version? Not reallly. Perhaps it's a small nuisance, but hardly a priority. It happens all the time in published programming books. I think it would be more appropriate to address that with some script to minimize orphans/widows; I think that compromising the formatting of the example code is the wrong approach. So should we wait for GLISS? Honestly, I don't see why we'd need to. I'm not proposing that we formalize everything all at once and right away, but there are some basics that we can probably agree on. I hope this doesn't start a flame war... - Mark ps. Graham, I'll reply to certain things more specifically on the Rietveld page. ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
On Sun, May 2, 2010 at 2:14 PM, David Kastrup wrote: > Han-Wen Nienhuys writes: > >> On Sun, May 2, 2010 at 8:04 AM, wrote: >>> >>> The tokens _are_ pushed one by one in the desired order. So it makes >>> no sense to allocate additional storage just to do the same job. >> >> This is not about saving storage. This block of code does 2 things: >> >> - translation between predicates and token numbers >> - pushing the tokens onto the stack > > That's what the _current_ implementation, Patch set 4, does. Have you > looked at it? Why do you keep complaining about code I already removed > before replying? > > The implementation you complained about did more than you claim: it also > made sure that the order of arguments was Scheme arguments first, markup > arguments second, markup list arguments last. > > It's not like that was not _extensively_ documented. > >> ideally, this would be broken up in two separate blocks of code, so we >> dont have one 70 line block of code, which suggests that something >> much more complicated is going on. > I apologise - I opened the page again, and expected it to always show the latest patch set, which it didn't obviously. That, coupled with a lengthy explanation of about using state machines made assume erroneously that you did not change the .ll code The lexer code looks OK now - you could remove the markup_p variables; ly_module_constant() memoizes the lookup, so the second lookup will basically be a noop. As for the scheme code, it would be nice if the validity check were moved in a separate pure function, so the setter does (define (set-signature cmd sig) (if (valid-sig? sig) (set-property .. .) (ly:error (_ "invalid signature for command))) > Read the comments if you don't feel you can guess from the code. The > comments (to a good degree Patch set 3, which you asked for but > apparently did not look at closely, quite extensively explained what was > done and why). > >> Why are you creating a state machine to apply a predicate=>token map >> over a list? This mapping is independent of the last pushed token > > Wrong. When the only allowed predicate at the current point in the > argument list is markup-list?, there is no point establishing whether > the current predicate is markup? or something else. I need not > distinguish between different invalid predicates. Depending on the > position in the argument list, there is no necessity for a complete > mapping. > >> , so I think a state machine is overkill. >> >> (please enlighten if I am missing some hidden functionality that is >> not clear from your code) > > It established that the order of arguments is correct. For establishing > correct sequencing, a state machine is the way to go. And this > particular state machine was boiled down to very concise code, doing > exactly what the comments said it did. > > But this is academical as I _removed_ all that code already. It is no > longer established at call time whether the signature is valid. That > check has been moved to _definition_ time in markup.scm. lexer.ll > should now only contain code that you can understand immediately without > bothering to look at the comments. > > I have no doubts that the changes in markup.scm will not meet your > approval, either. Feel free to write this differently. > >> The quality of the current code is not an argument to not improve over >> it. The current code is largely from my hand, but there are many >> things I would do differently today for many reasons. > > In the time you spent complaining about code I already removed prior to > your complaint, you could have done some things differently. Or at > least complained about the current code. > > It does not appear that the Rietveld review process is used for more > than annoying would-be contributors. > > In this case, quite successfully. > > Likely the best I can do is file a bug report about the markup command > limitations and include a pointer to the patch set, explaining that it > can't be accepted in its current form because of its descent from > unworthy code. For one reason or another, whenever I review code from you it degrades into a fight. I am not quite sure why this always happens. Could we mutually assume that when the other party makes comments or writes improvements, that all of this happens in good faith, so if something seems wrong, we assume the other party made an innocent mistake? thank you. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
Han-Wen Nienhuys writes: > ideally, this would be broken up in two separate blocks of code, so we > dont have one 70 line block of code, which suggests that something > much more complicated is going on. There were _37_ lines of code (please don't count comment lines _against_ me) that do a more general job than the _51_ lines of code that are there in the Lilypond repository. In addition, there were 20 lines of comments. The version in the Lilypond repository has no comment in that part. The version which you found fit to continue complaining about without looking at it has boiled this down to 23 lines of code, with 22 comment lines. What are you trying to accomplish with that sort of behavior? Could please somebody else bother to look at this patch set and see whether he finds it particularly hard to understand, in particular in comparison to the code it replaces? The least I can expect after putting in that amount of work is a fair assessment. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
Han-Wen Nienhuys writes: > On Sun, May 2, 2010 at 8:04 AM, wrote: >> >> The tokens _are_ pushed one by one in the desired order. So it makes >> no sense to allocate additional storage just to do the same job. > > This is not about saving storage. This block of code does 2 things: > > - translation between predicates and token numbers > - pushing the tokens onto the stack That's what the _current_ implementation, Patch set 4, does. Have you looked at it? Why do you keep complaining about code I already removed before replying? The implementation you complained about did more than you claim: it also made sure that the order of arguments was Scheme arguments first, markup arguments second, markup list arguments last. It's not like that was not _extensively_ documented. > ideally, this would be broken up in two separate blocks of code, so we > dont have one 70 line block of code, which suggests that something > much more complicated is going on. Read the comments if you don't feel you can guess from the code. The comments (to a good degree Patch set 3, which you asked for but apparently did not look at closely, quite extensively explained what was done and why). > Why are you creating a state machine to apply a predicate=>token map > over a list? This mapping is independent of the last pushed token Wrong. When the only allowed predicate at the current point in the argument list is markup-list?, there is no point establishing whether the current predicate is markup? or something else. I need not distinguish between different invalid predicates. Depending on the position in the argument list, there is no necessity for a complete mapping. > , so I think a state machine is overkill. > > (please enlighten if I am missing some hidden functionality that is > not clear from your code) It established that the order of arguments is correct. For establishing correct sequencing, a state machine is the way to go. And this particular state machine was boiled down to very concise code, doing exactly what the comments said it did. But this is academical as I _removed_ all that code already. It is no longer established at call time whether the signature is valid. That check has been moved to _definition_ time in markup.scm. lexer.ll should now only contain code that you can understand immediately without bothering to look at the comments. I have no doubts that the changes in markup.scm will not meet your approval, either. Feel free to write this differently. > The quality of the current code is not an argument to not improve over > it. The current code is largely from my hand, but there are many > things I would do differently today for many reasons. In the time you spent complaining about code I already removed prior to your complaint, you could have done some things differently. Or at least complained about the current code. It does not appear that the Rietveld review process is used for more than annoying would-be contributors. In this case, quite successfully. Likely the best I can do is file a bug report about the markup command limitations and include a pointer to the patch set, explaining that it can't be accepted in its current form because of its descent from unworthy code. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
Carl Sorensen wrote Sunday, May 02, 2010 5:49 PM On 5/2/10 4:45 AM, "Mark Polesky" wrote: * Use bar-checks (`|') only when barring is unclear. I think we should always use bar-checks when the piece is more than one bar long. That's a good habit to get into; we ought to start it right from the first. I would agree with this. In fact I put bar checks into quite a few of the examples in the LM originally, but they seem to have been removed. Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
On 5/2/10 4:45 AM, "Mark Polesky" wrote: > * Use bar-checks (`|') only when barring is unclear. I think we should always use bar-checks when the piece is more than one bar long. That's a good habit to get into; we ought to start it right from the first. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
On Sun, May 2, 2010 at 8:04 AM, wrote: > > http://codereview.appspot.com/969046/diff/7001/8002 > File lily/lexer.ll (right): > > http://codereview.appspot.com/969046/diff/7001/8002#newcode545 > lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. > On 2010/05/01 19:56:08, hanwenn wrote: >> >> wouldnt it be clearer to have a function > >> void translate_markup_signature(SCM predicate_list, >> vector expect_tokens); > >> which generates the [expect_scm, expect_scm, expect_markup] (possibly > > with >> >> expect_no_more_args) in a vector, and then push the tokens one by one > > in the >> >> desired order? > > The tokens _are_ pushed one by one in the desired order. So it makes no > sense to allocate additional storage just to do the same job. This is not about saving storage. This block of code does 2 things: - translation between predicates and token numbers - pushing the tokens onto the stack ideally, this would be broken up in two separate blocks of code, so we dont have one 70 line block of code, which suggests that something much more complicated is going on. Why are you creating a state machine to apply a predicate=>token map over a list? This mapping is independent of the last pushed token, so I think a state machine is overkill. (please enlighten if I am missing some hidden functionality that is not clear from your code) The quality of the current code is not an argument to not improve over it. The current code is largely from my hand, but there are many things I would do differently today for many reasons. >> Also, I think it is better to not use fall-through switches, as they > > are a >> >> uncommon and tricky idiom. > > Sigh. We have essentially a state machine where the state is determined > by the kind of already processed tokens, and depending on the state of > the current token, we transition to a new state with different allowed > new tokens/transitions. > > This sort of state machine can usefully only programmed using goto > (unless one uses explicit state tables which are even less readable). > The switch statement is basically the same as gotoizing a state machine, > with the case labels substituting for goto labels. > > Most state machines (including this one) don't have a structure readily > represented in linear structured code. > > Moving them into a separate procedure does not change that. > > My changes already make for the bulk of comment lines in this file. So > I decided against commenting this even more in violating of the > established standards in the rest of the file. > > Instead I shifted the responsibility of checking the order of arguments > to definition time instead of runtime. It has the advantage of > triggering prospective errors at a more expected point of time. > > http://codereview.appspot.com/969046/diff/7001/8002#newcode545 > lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. > On 2010/05/01 19:56:08, hanwenn wrote: >> >> wouldnt it be clearer to have a function > >> void translate_markup_signature(SCM predicate_list, >> vector expect_tokens); > >> which generates the [expect_scm, expect_scm, expect_markup] (possibly > > with >> >> expect_no_more_args) in a vector, and then push the tokens one by one > > in the >> >> desired order? > >> Also, I think it is better to not use fall-through switches, as they > > are a >> >> uncommon and tricky idiom. > >> I would expect something like > >> if(pred == bla("markup?") >> token = EXPECT_MARKUP; >> else if (pred == bla("markup-list?") >> token = EXPECT_MARKUP_LIST; >> else if (pred == bla("scm?") >> token = EXPECT_SCM; > > > > Done. > > http://codereview.appspot.com/969046/show > -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
On Sun, May 02, 2010 at 12:15:17PM +0100, Trevor Daniels wrote: > > Also need to specify default indenting is 2 spaces. That's a pre-GDP rule. I'm sure it's in the CG already. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
On Sun, May 02, 2010 at 03:45:49AM -0700, Mark Polesky wrote: > So I guess I'm working towards a more formal standard for > LilyPond code formatting. 1) wait for GLISS. 2) write/edit a scheme or python program to do this. 3) can we get bloody 2.14 out the bloody door before starting yet another bloody round of bloody bike-shedding? > If anyone is up to it, I'd like someone to look over the > patch to see if I've made anything worse. I started looking, and saw some questionable stuff. I'll take a longer look later today, after I wake up and have coffee. > Anyway, the patch is at > http://codereview.appspot.com/1056041. Copying&pasting this line produced a 404; I had to delete the final period. Yay for html code directly inside text with punctuation. Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [PATCH] Doc: LM: Reformat ly code.
Hi Mark The only rule I disagree with is * Use `\clef treble' instead of `\clef "treble"', etc. The quotes are required for G_8 so should be present on all for consistency and to avoid confusion. Also need to specify default indenting is 2 spaces. Trevor - Original Message - From: "Mark Polesky" To: "lilypond-devel" Sent: Sunday, May 02, 2010 11:45 AM Subject: [PATCH] Doc: LM: Reformat ly code. So I guess I'm working towards a more formal standard for LilyPond code formatting. I combed through the LM attempting to improve the ly code that's already there. In the process I gleaned what I think are some reasonable standards (some of which are already in CG 4.3.4 -- see below). If anyone is up to it, I'd like someone to look over the patch to see if I've made anything worse. I made very slight changes to a few of the examples for convenience's sake, but these should all be trivial. For example, LM 4.1.4 Tweaking methods, first example. was: c d \override NoteHead #'color = #red e f g \override NoteHead #'color = #green a b c changed it to: c4 d \override NoteHead #'color = #red e4 f \override NoteHead #'color = #green g4 a b c Anyway, the patch is at http://codereview.appspot.com/1056041. Thanks. - Mark * * * * * * * * * * Doc: LM: Reformat ly code. * Except for Scheme code, add exactly two spaces (never tabs) for each additional level of indentation. * Do not exceed one measure per line of code. * Specify durations for the first note of every line. * Specify durations after every `{' or `}'. * Otherwise, specify durations only when they change. * In \markup blocks, keep individual expressions on their own lines. * Keep these commands on their own lines (and in this order): \clef \key \time \partial * Use `\clef treble' instead of `\clef "treble"', etc. * Use bar-checks (`|') only when barring is unclear. * Do not put two `{'s on the same line unless the first `{' closes with a `}' before the second `{' * Put a newline after `{' (and indent) if its '}' is not on the same line. * Put a newline before `}' (and unindent) if its `{' is not on the same line. * Put a single space before and after `{' and `}' unless it begins or ends a line. * Put an empty line after the last \include. * Surround with empty lines things like: #(set-global-staff-size x) #(ly:set-option x y) * Put an empty line before and after multiline variable definitions or multiline blocks that start in the first column. * For music function definitions: - put `functionName =' alone on the first line. - put `#(define-music-function' alone on the second. - indent `(parser location ...)' with 5 spaces. - indent `(type? ...)' with 5 spaces. - indent `#{' and `#}' with 3 spaces. - indent the contents of `#{ ... #}' with 5 spaces. * For multiline variable definitions: - if `=' is immediately followed by `{', put a newline after `{'. - otherwise put a newline after `='. * Always use curly braces with \markup. ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Don't hardcode a limited set of markup signatures. (issue969046)
http://codereview.appspot.com/969046/diff/7001/8002 File lily/lexer.ll (right): http://codereview.appspot.com/969046/diff/7001/8002#newcode545 lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. On 2010/05/01 19:56:08, hanwenn wrote: wouldnt it be clearer to have a function void translate_markup_signature(SCM predicate_list, vector expect_tokens); which generates the [expect_scm, expect_scm, expect_markup] (possibly with expect_no_more_args) in a vector, and then push the tokens one by one in the desired order? The tokens _are_ pushed one by one in the desired order. So it makes no sense to allocate additional storage just to do the same job. Also, I think it is better to not use fall-through switches, as they are a uncommon and tricky idiom. Sigh. We have essentially a state machine where the state is determined by the kind of already processed tokens, and depending on the state of the current token, we transition to a new state with different allowed new tokens/transitions. This sort of state machine can usefully only programmed using goto (unless one uses explicit state tables which are even less readable). The switch statement is basically the same as gotoizing a state machine, with the case labels substituting for goto labels. Most state machines (including this one) don't have a structure readily represented in linear structured code. Moving them into a separate procedure does not change that. My changes already make for the bulk of comment lines in this file. So I decided against commenting this even more in violating of the established standards in the rest of the file. Instead I shifted the responsibility of checking the order of arguments to definition time instead of runtime. It has the advantage of triggering prospective errors at a more expected point of time. http://codereview.appspot.com/969046/diff/7001/8002#newcode545 lily/lexer.ll:545: // loop will be EXPECT_NO_MORE_ARGS. On 2010/05/01 19:56:08, hanwenn wrote: wouldnt it be clearer to have a function void translate_markup_signature(SCM predicate_list, vector expect_tokens); which generates the [expect_scm, expect_scm, expect_markup] (possibly with expect_no_more_args) in a vector, and then push the tokens one by one in the desired order? Also, I think it is better to not use fall-through switches, as they are a uncommon and tricky idiom. I would expect something like if(pred == bla("markup?") token = EXPECT_MARKUP; else if (pred == bla("markup-list?") token = EXPECT_MARKUP_LIST; else if (pred == bla("scm?") token = EXPECT_SCM; Done. http://codereview.appspot.com/969046/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
[PATCH] Doc: LM: Reformat ly code.
So I guess I'm working towards a more formal standard for LilyPond code formatting. I combed through the LM attempting to improve the ly code that's already there. In the process I gleaned what I think are some reasonable standards (some of which are already in CG 4.3.4 -- see below). If anyone is up to it, I'd like someone to look over the patch to see if I've made anything worse. I made very slight changes to a few of the examples for convenience's sake, but these should all be trivial. For example, LM 4.1.4 Tweaking methods, first example. was: c d \override NoteHead #'color = #red e f g \override NoteHead #'color = #green a b c changed it to: c4 d \override NoteHead #'color = #red e4 f \override NoteHead #'color = #green g4 a b c Anyway, the patch is at http://codereview.appspot.com/1056041. Thanks. - Mark * * * * * * * * * * Doc: LM: Reformat ly code. * Except for Scheme code, add exactly two spaces (never tabs) for each additional level of indentation. * Do not exceed one measure per line of code. * Specify durations for the first note of every line. * Specify durations after every `{' or `}'. * Otherwise, specify durations only when they change. * In \markup blocks, keep individual expressions on their own lines. * Keep these commands on their own lines (and in this order): \clef \key \time \partial * Use `\clef treble' instead of `\clef "treble"', etc. * Use bar-checks (`|') only when barring is unclear. * Do not put two `{'s on the same line unless the first `{' closes with a `}' before the second `{' * Put a newline after `{' (and indent) if its '}' is not on the same line. * Put a newline before `}' (and unindent) if its `{' is not on the same line. * Put a single space before and after `{' and `}' unless it begins or ends a line. * Put an empty line after the last \include. * Surround with empty lines things like: #(set-global-staff-size x) #(ly:set-option x y) * Put an empty line before and after multiline variable definitions or multiline blocks that start in the first column. * For music function definitions: - put `functionName =' alone on the first line. - put `#(define-music-function' alone on the second. - indent `(parser location ...)' with 5 spaces. - indent `(type? ...)' with 5 spaces. - indent `#{' and `#}' with 3 spaces. - indent the contents of `#{ ... #}' with 5 spaces. * For multiline variable definitions: - if `=' is immediately followed by `{', put a newline after `{'. - otherwise put a newline after `='. * Always use curly braces with \markup. ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel