Re: [PATCH 3/4] Make define-builtin-markup{, -list}-command #:category #:properties keywords

[David Kastrup]

Nicolas Sceaux <address@hidden> writes:

> Le 23 nov. 2009 à 01:03, David Kastrup a écrit :
>
>> The specification of category and properties makes the *-builtin-*
>> variants diverge syntactically from the user specified markup.  Moving
>> those specifications into keyword arguments makes the builtin defining
>> macros upwards compatible with the user specified ones.
>
> Could you publish your patches on retvield?
> It's just a matter of git-cl upload from your branch.

Nope.  The contributor's guide says:

       Then, add the git-cl directory to your PATH, or create a symbolic
    link to the git-cl and upload.py in one of your PATH directories (like
    usr/bin).  git-cl will is then configured by

         git-cl config

    and answering the questions that are asked.

There is no clue just _how_ one should answer the questions or what they
mean.  Let's see where we go:

$ git-cl config
Rietveld server (host[:port]) [codereview.appspot.com]:
CC list: address@hidden
Tree status URL:
ViewVC URL:

address@hidden:/usr/local/tmp/lilypond$ git-cl status
Branches associated with reviews:
      master: None

Current branch: no issue assigned.

$

Does not look really convincing.


address@hidden:/usr/local/tmp/lilypond$ git-cl rebase
Can't locate SVN/Core.pm in @INC (@INC contains: /opt/git/share/perl/5.10.0 
/etc/perl /usr/local/lib/perl/5.10.0 /usr/local/share/perl/5.10.0 
/usr/lib/perl5 /us


Really, that's not something that would appear to helpmy workflow.


git-checkout -d mad-dak

then piping the four postings through git-am should do the trick for
reviewing.  Feel free to post the thing to Rietveld yourself if you feel
like it, but at the moment I really can't be bothered fighting some
none-working tools.

> I think somewhere you missed a change from
> define-builtin-markup-list-command to define-markup-list-command.

I did not change any define-builtin-markup-list-command to
define-markup-list-command in this patch series.

> It would be easier to read and comment on retvield, together with
> other modified files.

Feel free to put it there.

> It also seems that ly/markup-init.ly is impacted, but I don't see the
> patch.

It isn't impacted.  The patch series gives the *-builtin-* commands a
different syntax that is upwards-compatible to the user counterparts.
It does not change the user counterparts at all.

The first two patches in the series are cleanups and would work
independently of the rest of the patch series.  I had posted them
previously on the list and they have basically been ignored.  They are
in the patch series because they touch code _overlapping_ with patch 4,
and it makes no sense in not doing the cleanup.

Patch 3 actually changes changes the *-builtin-* syntax and consequently
renders Lilypond inoperative when applied alone.  That is the core of
the patch series, and it is much better reviewable when kept separate
from patch 4.

Patch 4 changes all the callers to the new *-builtin-* syntax.  This is
just boring legwork without intellectual value, but a lot of it.

This patch series is _only_ concerned with *-builtin-*.  It does a
complete job on that and nothing else.

Eventually the user-level counterparts should accept #:properties and
#:category as well (likely ignoring the latter), but that's a different
topic.  And I am still looking through the module concepts for that.

-- 
David Kastrup