Issue 222810043: make pretty-print available in ly files
|
Created:
9 years ago by david.nalesnik Modified:
8 years, 2 months ago CC:
lilypond-devel_gnu.org Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/ Visibility:
Public. |
Descriptionmake pretty-print available in ly files
Patch Set 1 #
Total comments: 3
MessagesTotal messages: 14
Please review. Thanks!
Sign in to reply to this message.
https://codereview.appspot.com/222810043/diff/1/ly/init.ly File ly/init.ly (right): https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 ly/init.ly:38: #(use-modules (ice-9 pretty-print)) Uh, where is the point? Why wouldn't a user include a module he wants himself? Other modules are included because LilyPond itself uses them generously. But this would not appear to be the case here.
Sign in to reply to this message.
https://codereview.appspot.com/222810043/diff/1/ly/init.ly File ly/init.ly (right): https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 ly/init.ly:38: #(use-modules (ice-9 pretty-print)) On 2015/04/07 16:13:44, dak wrote: > Uh, where is the point? Why wouldn't a user include a module he wants himself? > Other modules are included because LilyPond itself uses them generously. But > this would not appear to be the case here. It's a convenience for people who work with Scheme in .ly files, the same motivation behind use of pretty-printing with \displayMusic. I know that Harm frequently uses pretty-print, and I was prompted to make this change at his suggestion. (See comments here: https://codereview.appspot.com/217260043 ) If you think this should be reverted we could do that of course; however, I think its inclusion is helpful.
Sign in to reply to this message.
https://codereview.appspot.com/222810043/diff/1/ly/init.ly File ly/init.ly (right): https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 ly/init.ly:38: #(use-modules (ice-9 pretty-print)) On 2015/04/07 17:43:07, david.nalesnik wrote: > On 2015/04/07 16:13:44, dak wrote: > > Uh, where is the point? Why wouldn't a user include a module he wants > himself? > > Other modules are included because LilyPond itself uses them generously. But > > this would not appear to be the case here. > > It's a convenience for people who work with Scheme in .ly files, the same > motivation behind use of pretty-printing with \displayMusic. I know that Harm > frequently uses pretty-print, and I was prompted to make this change at his > suggestion. (See comments here: https://codereview.appspot.com/217260043 ) > > If you think this should be reverted we could do that of course; however, I > think its inclusion is helpful. I really appreciate having it available in .ly-files And I can't see it does any harm.
Sign in to reply to this message.
On 2015/04/07 23:25:09, thomasmorley651 wrote: > https://codereview.appspot.com/222810043/diff/1/ly/init.ly > File ly/init.ly (right): > > https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 > ly/init.ly:38: #(use-modules (ice-9 pretty-print)) > On 2015/04/07 17:43:07, david.nalesnik wrote: > > On 2015/04/07 16:13:44, dak wrote: > > > Uh, where is the point? Why wouldn't a user include a module he wants > > himself? > > > Other modules are included because LilyPond itself uses them generously. > But > > > this would not appear to be the case here. > > > > It's a convenience for people who work with Scheme in .ly files, the same > > motivation behind use of pretty-printing with \displayMusic. I know that Harm > > frequently uses pretty-print, and I was prompted to make this change at his > > suggestion. (See comments here: https://codereview.appspot.com/217260043 ) > > > > If you think this should be reverted we could do that of course; however, I > > think its inclusion is helpful. > > I really appreciate having it available in .ly-files > > And I can't see it does any harm. How would it be unavailable without this patch? Any unneeded modules we load take up memory and load time. Something like (srfi srfi-1) is loaded anyway for the sake of the LilyPond core: loading it in init.ly is basically only reexporting the already loaded interface (which can lead to naming resolution conflicts nevertheless, but the current set seems reasonably clean in that regard). If there is a significantly relevant manner in which the use of the prettyprint module can be considered encumbered in our current setup (perhaps safe mode?), then we should try addressing _that_ instead of preloading everything that somebody might at some point of time consider useful.
Sign in to reply to this message.
On 2015/04/08 08:11:35, dak wrote: > On 2015/04/07 23:25:09, thomasmorley651 wrote: > > https://codereview.appspot.com/222810043/diff/1/ly/init.ly > > File ly/init.ly (right): > > > > https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 > > ly/init.ly:38: #(use-modules (ice-9 pretty-print)) > > On 2015/04/07 17:43:07, david.nalesnik wrote: > > > On 2015/04/07 16:13:44, dak wrote: > > > > Uh, where is the point? Why wouldn't a user include a module he wants > > > himself? > > > > Other modules are included because LilyPond itself uses them generously. > > But > > > > this would not appear to be the case here. > > > > > > It's a convenience for people who work with Scheme in .ly files, the same > > > motivation behind use of pretty-printing with \displayMusic. I know that > Harm > > > frequently uses pretty-print, and I was prompted to make this change at his > > > suggestion. (See comments here: https://codereview.appspot.com/217260043 ) > > > > > > If you think this should be reverted we could do that of course; however, I > > > think its inclusion is helpful. > > > > I really appreciate having it available in .ly-files > > > > And I can't see it does any harm. > > How would it be unavailable without this patch? Any unneeded modules we load > take up memory and load time. I wouldn't have thought it's problematic. Point taken, though. > Something like (srfi srfi-1) is loaded anyway for > the sake of the LilyPond core: loading it in init.ly is basically only > reexporting the already loaded interface (which can lead to naming resolution > conflicts nevertheless, but the current set seems reasonably clean in that > regard). agreed > If there is a significantly relevant manner in which the use of the prettyprint > module can be considered encumbered in our current setup (perhaps safe mode?), > then we should try addressing _that_ instead of preloading everything that > somebody might at some point of time consider useful. How about an alternative? We have 'write-me' in lily-library.scm, pretty-print is loaded there anyway. We could change 'write-me' to something at the lines of: (define*-public (write-me x #:optional (message "")) "Return @var{x}. Display @var{message} and write @var{x}. Handy for debugging, possibly turned off." (display message) (newline)(pretty-print x) (newline) x) It would even extend the capability of pretty-print.
Sign in to reply to this message.
On 2015/04/08 09:13:53, thomasmorley651 wrote:
> We have 'write-me' in lily-library.scm, pretty-print is loaded there anyway.
>
> We could change 'write-me' to something at the lines of:
>
> (define*-public (write-me x #:optional (message ""))
> "Return @var{x}. Display @var{message} and write @var{x}.
> Handy for debugging, possibly turned off."
> (display message) (newline)(pretty-print x) (newline) x)
>
> It would even extend the capability of pretty-print.
I think one problem we might have is that GUILE contains a proper interface for
pretty-printing of Smobs, so we probably could just integrate with the regular
pretty-printing facilities anyway. But the documentation of the respective
scm_print_state structure in the GUILE manual basically boils down to "oh, you
would probably just want to ignore this anyway rather than worry your pretty
little head over it".
value->lily-string is probably the most useful reasonably generic printing
facility right now. But nobody really knows it.
Sign in to reply to this message.
On 2015/04/08 10:21:55, dak wrote: > value->lily-string is probably the most useful reasonably generic printing > facility right now. But nobody really knows it. At least, I wasn't aware of it, because of (in a ly-file) #(display value->lily-string) -> Unbound variable: value->lily-string
Sign in to reply to this message.
On 2015/04/08 10:43:09, thomasmorley651 wrote: > On 2015/04/08 10:21:55, dak wrote: > > > value->lily-string is probably the most useful reasonably generic printing > > facility right now. But nobody really knows it. > > At least, I wasn't aware of it, because of (in a ly-file) > > #(display value->lily-string) > -> Unbound variable: value->lily-string Like with much of the Scheme-level printing you'll need #(use-modules (scm display-lily)) It's not been around for long in its current form (and name) anyway. It became an exported function under that name with commit 8d8e8aec6388fbb08ed2219884b82ecf53a9dbcd Author: David Kastrup <dak@gnu.org> Date: Wed Jan 1 20:54:15 2014 +0100 Provide value->lily-string function because I really needed that facility in order to make nice error messages. It's in 2.19.1 and 2.18.0 I think, as part of issue 3770. At some point of time, somebody needs to make some sort of plan along the lines of "what kind of functions and programming facilities are needed for working with LilyPond in a coherent and predictable and comfortable manner" and put it into code. It is quite too often that one runs into the "I cannot believe LilyPond does not have something obvious for such an obvious task like $x" situation.
Sign in to reply to this message.
On 2015/04/08 11:14:54, dak wrote:
> On 2015/04/08 10:43:09, thomasmorley651 wrote:
> > On 2015/04/08 10:21:55, dak wrote:
> >
> > > value->lily-string is probably the most useful reasonably generic printing
> > > facility right now. But nobody really knows it.
> >
> > At least, I wasn't aware of it, because of (in a ly-file)
> >
> > #(display value->lily-string)
> > -> Unbound variable: value->lily-string
>
> Like with much of the Scheme-level printing you'll need
> #(use-modules (scm display-lily))
>
> It's not been around for long in its current form (and name) anyway. It
became
> an exported function under that name with
> commit 8d8e8aec6388fbb08ed2219884b82ecf53a9dbcd
> Author: David Kastrup <mailto:dak@gnu.org>
> Date: Wed Jan 1 20:54:15 2014 +0100
>
> Provide value->lily-string function
>
> because I really needed that facility in order to make nice error messages.
> It's in 2.19.1 and 2.18.0 I think, as part of issue 3770.
But doesn't this just return a string? Also, it requires a parser argument.
>
> At some point of time, somebody needs to make some sort of plan along the
lines
> of "what kind of functions and programming facilities are needed for working
> with LilyPond in a coherent and predictable and comfortable manner" and put it
> into code. It is quite too often that one runs into the "I cannot believe
> LilyPond does not have something obvious for such an obvious task like $x"
> situation.
And of course documenting it in a convenient way, like "Resources for ..." Can't
think of a nice summary word and "easier tampering" or "hacking" wouldn't send
the right message.
It's worth noting that the ~y option IS available within ly files, though not in
scm files, where format has been redefined as simple-format. So something like
this produces nice results:
#(format #t "~y" (procedure-source grob::rhythmic-location))
==>
(lambda (grob)
"Return a pair consisting of the measure number and moment within
the measure of grob @var{grob}."
(let* ((item (if (grob::has-interface
grob
'spanner-interface)
(ly:spanner-bound grob LEFT)
grob))
(col (ly:item-get-column item)))
(if (ly:grob? col)
(ly:grob-property col (quote rhythmic-location))
'())))
______________
So I guess the way to proceed now is to make a patch reverting this commit, and
then just push it to staging as soon as it passes tests, rather than going
through a review cycle?
Sign in to reply to this message.
On 2015/04/08 16:06:47, david.nalesnik wrote: > On 2015/04/08 11:14:54, dak wrote: > > commit 8d8e8aec6388fbb08ed2219884b82ecf53a9dbcd > > Author: David Kastrup <mailto:dak@gnu.org> > > Date: Wed Jan 1 20:54:15 2014 +0100 > > > > Provide value->lily-string function > > > > because I really needed that facility in order to make nice error messages. > > It's in 2.19.1 and 2.18.0 I think, as part of issue 3770. > > But doesn't this just return a string? Also, it requires a parser argument. I mean that when we pass through scheme-expr->lily-string, the Scheme expression is not formatted with nice identation, line breaking, as with pretty-print.
Sign in to reply to this message.
On 2015/04/08 16:06:47, david.nalesnik wrote: > It's worth noting that the ~y option IS available within ly files, though not in > scm files, where format has been redefined as simple-format. Well, as "ergonomic-simple-format". The full-featured format is available as "fancy-format" (scm/lily.scm).
Sign in to reply to this message.
The patch was pushed after much of the discussion above. Shall I make another patch removing the functionality>
Sign in to reply to this message.
On 2016/01/23 22:11:03, david.nalesnik wrote: > The patch was pushed after much of the discussion above. Shall I make another > patch removing the functionality> Ugh--I mean pushed *before* much of the discussion.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
