On Mon, 2005-06-27 at 09:39 +0000, Chris Cannam wrote: > Stephen Torri: > > Let me know if there is a better way to send a patch. > > Well, the best thing in the long run is probably to give you access to > commit to an experimental branch in CVS. > > I haven't actually had time to try this yet, only to look, so that > places me in an excellent position to make some uninformed comments: > > - I'm troubled by the addition of new classes named Chord, Note, and > Bar. We already have the former two in the Rosegarden namespace with > well-understood meanings (Chord is in Sets.h, Note in > NotationTypes.h). I don't think we have Bar already per se, but the > word is widely used in the code. I'm not even quite sure what is meant > by Bar in the context of guitar chords.
Because there was an already existing Chord and Note that I place all my work in the 'guitar' namespace. Perhaps its an opportunity to merge the two. I would have to look at the original Chord and compare it against my new Chord class. What I see is that the Note class depends on the instrument being played. In the case of the tab chord editor we are expecting the instrument to be a stringed instrument like a guitar, mandolin, etc. So we keep track of which string is played and at what fret. With regards to a piano a note is played on a single string where there is no concept of a fret. For the Bar class it is a representation of how a set of strings (2 or more) are played with one finger. The finger is laid across these strings. A Bar is a specific technique to guitar playing. I do not know how to play a bass, mandolin, etc so I am not sure if they use Bars as a part of a chord. I guess if we want to combine all these classes with what has been done previously then we must figure out a way to remove the specifics (e.g. instrument used) from the desired action (e.g. Note play at a particular point on the instrument). > - I'm assuming the code in guitar/ is yours and tabtrack comes from > elsewhere? Where? - KGuitar, NoteEdit? It looks like it does a number > of things RG's notation code does elsewhere, and perhaps is "awaiting > integration", but I might be misunderstanding. The tabtrack, as you call it, was originally from KGuitar. I eventually disregarded KGuitar's data structures for storing a chord information but kept the tab display (called Fingering in KGuitar). The code that displays the tabtrack, notes, bars and other notation is what I copied from KGuitar. So I will need to note those segments of code borrowed. > - Our usual naming style is likeThis or m_likeThis for local or class > scoped variables, LikeThis for class scoped enum members and > constants, LIKE_THIS for global or namespaced constants only (with a > few dubious exceptions, of course). Ok. I will review the code and make changes. > - Is the XML usage strongly bound to libxml? Just because we > obviously already use the Qt XML parser elsewhere. The QT parser does not validate an input XML file against the associated DTD. It will not tell you were something is wrong in the file and give a message saying what it was expecting to see. LibXML provides that validation which has made it incredibly easy to fix problem XML files. If you can show me how to do that with Qt XML parser I will switch the code over to it. > - The patch contains a few spurious bits, including diffs to > moc-generated files and diffs for the KDE warnings that we > provisionally decided not to include yet (or did we? Anyone?) I guess since I am not using CVS to create my diffs that you will get extra files that you do not want in the patch. ------------------------------------------------------- SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click _______________________________________________ Rosegarden-devel mailing list [email protected] - use the link below to unsubscribe https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
