----- Original Message -----
From: <gra...@percival-music.ca>
To: <philehol...@googlemail.com>; <d...@gnu.org>; <em...@philholmes.net>
Cc: <lilypond-devel@gnu.org>; <re...@codereview-hr.appspotmail.com>
Sent: Monday, July 30, 2012 6:54 PM
Subject: Re: Set indent based on instrument name (issue 6457049)
Little nitpicks based on my C++ experience in other projects, with no
knowledge whatsoever of lilypond internals.
http://codereview.appspot.com/6457049/diff/4001/lily/instrument-name-engraver.cc
File lily/instrument-name-engraver.cc (right):
http://codereview.appspot.com/6457049/diff/4001/lily/instrument-name-engraver.cc#newcode111
lily/instrument-name-engraver.cc:111:
Instrument_name_engraver::Get_text_len_from_name (SCM scheme_text)
convention is to use lower-case names for class functions (or methods if
you prefer)
http://codereview.appspot.com/6457049/diff/4001/lily/output-def.cc
File lily/output-def.cc (right):
http://codereview.appspot.com/6457049/diff/4001/lily/output-def.cc#newcode38
lily/output-def.cc:38: Real long_name_len = 0.0;
could these be class member variables instead of global variables?
I don't believe so. I'd be happy to be corrected by someone who understands
this better than I do, but my understanding of c++ (which I guess at based
on c#) says that, in order to access a class member variable, you need to
have an instantiation of the class. The problem we have is that the
knowledge of the instrument name strings is in a different class
(Instrument_name_engraver). We need to persist the value of the longest
instrument name, and pass it to output-def, where the indents are set. If
we instantiated an output-def class in Instrument_name_engraver, we would
lose the instantiation when Instrument_name_engraver dies, so the values
would not be persisted. Hence why I ended up with global variables. The
one thing I thought about on the way home tonight is that these need to be
zeroed when we create a new Score, so I need to find out/be told how to do
that.
... hmm, more generally, I'm not sold on the whole set_inst_name_len
approach. Is there any way you could just pass an additional argument
to line_dimenstions_int , and determine the data you need from that
function?
http://codereview.appspot.com/6457049/
Not AFAICS. It currently requires 2 arguments: Output_def *def, int n, and
there's no way we can satisfy them from Instrument_name_engraver. I think
adding an extra function to access the global variables is an effective
encapsulation of the variable, and works for me.
--
Phil Holmes
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
