Han-Wen Nienhuys <hanw...@gmail.com> writes: > On Sun, May 2, 2010 at 8:04 AM, <d...@gnu.org> 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
