On 2018/04/25 12:22:18, knupero wrote:
> I don't see the point in starting a formal review after this has
already been
> discussed on the mailing list.
I was hoping, apparently in vain, that the formal process would lead
to a more
reasonable discussion.
You make it appear like "reasonable" means "with the desired outcome". The formal process differs from the discussion not in who participates but rather that it is connected with semi-automated procedures for making a given patch progress into the code base, given that the discussion on the trackers does not suggest the need for further changes. Proceeding to a formal patch suggestion when the discussion on the developer list does not suggest the patch's design being in acceptable state is just a waste of everyone's time. This is different when we are not talking about the design of the patch but rather its execution: in that case the line-by-line review and commenting tools might be helpful for getting the patch progress into a form which is acceptable. The "formal" process is not a substitute for discussion, merely a tool for allowing a more detailed review of the code.
Apparently, however, this process also invites speculation about the amount of work someone is willing to invest, his qualifications and his honesty.
You are using hyperbole. Your original patch suggestion was in https://lists.gnu.org/archive/html/lilypond-devel/2018-04/msg00097.html . This produces "not a note name" errors for things which are, well, not a note name. The error recovery of current grammar rules runs through rules then actually producing lyric elements, and it would do so regardless of whether the results are then used as parts of lyrics or other music expressions. Which is more a side effect of the current grammar refactoring and an oversight in error processing than actually useful behavior. When I asked "how was the output correct?" without checking the output of the current recovery path for strings encountered outside of lyrics mode, you sent a reply https://lists.gnu.org/archive/html/lilypond-devel/2018-04/msg00108.html . This contains a whole lot of verbiage as well as PDFs. At the end of the verbiage, there indeed is a sentence
Attached is a 2nd version of the patch. With that extended version
_all_ examples above
compile without warnings/errors and give the desired result.
which I overlooked. My reply https://lists.gnu.org/archive/html/lilypond-devel/2018-04/msg00114.html however comments on why there is a requirement of a braced music list when switching to Lyrics mode. Instead of replying to that objection, you formally propose the patch which will not work reliably because of the reasons I pointed out. While a careful rereading of the communication makes clear that I was wrong stating:
> In particular since this differs from your > originally proposed patch (where you did not switch to lyrics mode
at all) and
> you disengenuously continued the discussion based on a modified
patch (the one
> proposed here) which you then tacked onto your results without
further
> mention.
For the record: All the test results were obtained with a) an
unpatched lilypond
and b) a lilypond with my originally proposed patch.
The record supports your statement here.
This is easy to verify, and everybody is invited to do so. David: You
did not do
that, apparently.
No, I didn't. However, that the current error recovery path produces lyrics events (regardless in which mode spurious strings appear) does not imply that the flagged behavior would lead to sensible recognition of lyrics input which differs from music input in a whole lot more respects than just accepting words or not. You can, for example, write sentences with punctuation in lyrics mode, something which would not work in the original code triggering errors but still producing lyrics as recovery. So there was no point in testing the patch as I knew perfectly well it could not function comprehensively. That led me to overlooking the second patch which would fail in less obvious manners (without triggering the error path) for input requiring lookahead, as spelled out in the comment in the code and explained by myself in the discussion. I apologize for misinterpreting your actions with regard to the second patch I overlooked. However, your disinterest in accepting any explanation not in line with your desires and just going ahead ignoring input in the expectation that this may somehow lead to more favorable results is annoying, and I might have overreacted as a result of that annoyance. Whether it is inconvenient for your use case or not: mode-changing commands can only accept a limited number of constructs, those known to be delimited well enough that recognizing them does not require further lookahead, or the next token will get scanned as lookahead in the wrong mode, an action which cannot be redone. It would be feasible to create a family of music function calls matching that descriptions. However, doing so would require a thorough understanding of the current parser code and your manner of rejecting input is likely going to make the acquisition of such knowledge comparatively ineffective for you as well as reviewers. That's not necessarily a showstopper: after all, I acquired basically all of my current parser chops at a time where its previous maintainers were essentially no longer available. However, it makes this particular project a bad idea for starting. https://codereview.appspot.com/343820043/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
