Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(254)

Issue 160048: Make define-builtin-markup{,-list}-command #:category #:properties keywords (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by dak
Modified:
8 years, 2 months ago
Reviewers:
Ian Hulin, carl.d.sorensen, nicolas.sceaux
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Make define-builtin-markup{,-list}-command #:category #:properties keywords 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. All markup commands defined with these macros are adapted to the new syntax.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Address documentation bug and make defining macros work with DOC-string less definitions. #

Patch Set 3 : Fix bad treating of DOC string in define-builtin-markup-list-command #

Patch Set 4 : Get rid of define-builtin-markup-{,list}-command altogether #

Patch Set 5 : This is an attempt to deal with the reported memory leak problem that I can't reproduce myself #

Patch Set 6 : Use weak hash tables to avoid memory leakage #

Total comments: 10

Patch Set 7 : Implement -dmake-index option. Minimal time&memory savings: is this a good idea? #

Patch Set 8 : Fix module system description in CG to better match current situation. #

Total comments: 10
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -24 lines) Patch
M Documentation/contributor/programming-work.itexi View 4 5 6 7 1 chunk +13 lines, -24 lines 10 comments Download

Messages

Total messages: 15
Carl
David, Thank you for posting this on Rietveld. It was much easier for me to ...
9 years, 7 months ago (2009-11-24 11:01:59 UTC) #1
dak
Carl.D.Sorensen@gmail.com writes: > David, > > Thank you for posting this on Rietveld. It was ...
9 years, 7 months ago (2009-11-24 11:42:26 UTC) #2
dak
http://codereview.appspot.com/160048/diff/1/5 File scm/markup.scm (right): http://codereview.appspot.com/160048/diff/1/5#newcode74 scm/markup.scm:74: [ :category category ] On 2009/11/24 11:01:59, Carl wrote: ...
9 years, 7 months ago (2009-11-24 13:44:29 UTC) #3
nicolas.sceaux
Hi, Maybe this patch, as a first step, should focus on the unification of the ...
9 years, 7 months ago (2009-11-26 09:13:58 UTC) #4
dak
On 2009/11/26 09:13:58, nicolas.sceaux wrote: > Hi, > > Maybe this patch, as a first ...
9 years, 7 months ago (2009-11-26 17:16:15 UTC) #5
nicolas.sceaux
Hi, When defining a user markup command, it would be better not to modify at ...
9 years, 6 months ago (2009-12-03 14:58:38 UTC) #6
dak
Thanks for looking this over. This patch set is sort of a moving target due ...
9 years, 6 months ago (2009-12-03 16:14:22 UTC) #7
c_sorensen
On 12/3/09 9:14 AM, "dak@gnu.org" <dak@gnu.org> wrote: > http://codereview.appspot.com/160048/diff/4008/6007#newcode131 > scm/markup.scm:131: ;; Register the new ...
9 years, 6 months ago (2009-12-03 17:34:24 UTC) #8
dak
On 2009/12/03 17:34:24, c_sorensen_byu.edu wrote: > > > > > I have not yet done ...
9 years, 6 months ago (2009-12-03 22:26:04 UTC) #9
dak
On 2009/12/03 22:26:04, dak wrote: > On 2009/12/03 17:34:24, c_sorensen_byu.edu wrote: > > > > ...
9 years, 6 months ago (2009-12-05 08:48:55 UTC) #10
nicolas.sceaux
Le 5 déc. 2009 à 09:48, dak@gnu.org a écrit : > Currently I don't see ...
9 years, 6 months ago (2009-12-05 10:52:52 UTC) #11
dak
On 2009/12/05 10:52:52, nicolas.sceaux wrote: > Le 5 déc. 2009 à 09:48, mailto:dak@gnu.org a écrit ...
9 years, 6 months ago (2009-12-17 11:23:04 UTC) #12
nicolas.sceaux
On 2009/12/17 11:23:04, dak wrote: > Well, patch set 7 does that, but after rethinking ...
9 years, 6 months ago (2009-12-20 12:05:37 UTC) #13
dak
nicolas.sceaux@gmail.com writes: > On 2009/12/17 11:23:04, dak wrote: > >> Well, patch set 7 does ...
9 years, 6 months ago (2009-12-20 19:18:01 UTC) #14
Ian Hulin
9 years, 6 months ago (2009-12-26 20:05:16 UTC) #15
David,
These are mostly stylistic nit-picks.

I'd like the section to state clearly the relationship between Lilypond scoping
and the guile module system.

Cheers,
Ian

http://codereview.appspot.com/160048/diff/13001/13002
File Documentation/contributor/programming-work.itexi (left):

http://codereview.appspot.com/160048/diff/13001/13002#oldcode1271
Documentation/contributor/programming-work.itexi:1271: Maintaining this scoping
when one .ly file can be included in another
Is this paragraph totally irrelevant? It seems to be talking about working
around an issue with scopes when one source file is \included in another.

http://codereview.appspot.com/160048/diff/13001/13002#oldcode1279
Documentation/contributor/programming-work.itexi:1279: With this architecture,
the guile module system is not bypassed:
Retain a re-written version of this para before you go on to talk about avoiding
memory leaks, maybe something like:
"Scoping in Lilypond is layered on the Scheme/Guile module system."

http://codereview.appspot.com/160048/diff/13001/13002
File Documentation/contributor/programming-work.itexi (right):

http://codereview.appspot.com/160048/diff/13001/13002#newcode1244
Documentation/contributor/programming-work.itexi:1244: The Lilypond language has
a concept of scoping, ie you can do
"The Lilyond language has a concept of scoping, i.e. you can write"

http://codereview.appspot.com/160048/diff/13001/13002#newcode1253
Documentation/contributor/programming-work.itexi:1253: @noindent with
@code{\paper}, @code{\midi} and @code{\header} being
@noindent.  @code{\paper}, @code{\midi} and @code{\header} are

http://codereview.appspot.com/160048/diff/13001/13002#newcode1254
Documentation/contributor/programming-work.itexi:1254: nested scope inside the
@file{.ly} file-level scope.  @w{@code{foo = 1}}
"nested scope" --> "scope definitions nested"

http://codereview.appspot.com/160048/diff/13001/13002#newcode1257
Documentation/contributor/programming-work.itexi:1257: This implemented using
modules, with each scope being an anonymous
"This is implemented using modules; each of these scopes is declared as an
anonymous Scheme module which imports definitions from the module declared for
its parent scope."

http://codereview.appspot.com/160048/diff/13001/13002#newcode1258
Documentation/contributor/programming-work.itexi:1258: module that imports its
enclosing scope's module.
See comment for above line.

http://codereview.appspot.com/160048/diff/13001/13002#newcode1261
Documentation/contributor/programming-work.itexi:1261: @code{lily} module,
outside the @file{.ly} level.  In the case of
Lilypond's core, loaded from various @file{.scm} files, is usually defined by
statements in the @code{lily} module, independently of the @file{.ly} level
(top-level).  In the following example

http://codereview.appspot.com/160048/diff/13001/13002#newcode1268
Documentation/contributor/programming-work.itexi:1268: we want to reuse the
built-in definitions, without changes effected in
Here we want to re-use the built-in definitions without changes written in

http://codereview.appspot.com/160048/diff/13001/13002#newcode1272
Documentation/contributor/programming-work.itexi:1272: memory leaks that could
occur when running multiple files.  All
"memory leaks which could occur when compiling more than one file."<new line>
"All"
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b