Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
Reviewers: ,

Message:
This is my first effort at making a patch, so I could be doing things
wrong. Full description is in the patch.

Description:
Issue 5187 Add command for Thin Aiken noteheads

Added 2 shapeNoteStyles definitions, aikenThinHeads and
aikenThinHeadsMinor.
Previously, these "thin-variant" Aiken notehead styles were not
accessible in the same way as others.
The standard aikenHeads are better for matching historical music,
while these thin-variants are more readable at lower staff sizes.
This is due to the standard Aiken heads having heavier horizontal lines,
which sometimes leave little interior white space for distinguising
white notes from black ones.

Please review this at https://codereview.appspot.com/326510043/

Affected files (+4, -0 lines):
   M ly/property-init.ly


Index: ly/property-init.ly
diff --git a/ly/property-init.ly b/ly/property-init.ly
index  
4923420ffef5181bf0ade63b3d3f8904075017fa..4337024397cef03c9a23d0746a5e781e5a37b6d6  
100644
--- a/ly/property-init.ly
+++ b/ly/property-init.ly
@@ -483,6 +483,10 @@ predefinedFretboardsOn =

  aikenHeads      = \set shapeNoteStyles = ##(do re miMirror fa sol la ti)
  aikenHeadsMinor = \set shapeNoteStyles = ##(la ti do re miMirror fa sol)
+aikenThinHeads =
+  \set shapeNoteStyles = ##(doThin reThin miThin faThin sol laThin tiThin)
+aikenThinHeadsMinor =
+  \set shapeNoteStyles = ##(laThin tiThin doThin reThin miThin faThin sol)
  funkHeads =
    \set shapeNoteStyles = ##(doFunk reFunk miFunk faFunk solFunk laFunk  
tiFunk)
  funkHeadsMinor =



_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
On Wed, Sep 20, 2017 at 12:37 PM,  <[hidden email]> wrote:
> This is my first effort at making a patch, so I could be doing things wrong.

I gather I am supposed to request write access to the issue tracker on
Sourceforge? Username: karlinhigh
--
Karlin High
Missouri, USA

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Carl Sorensen
In reply to this post by Karlin High
This change to property-init.ly looks good.

But the documentation (Shape Note Heads, in NR 1.1.4 should also be
changed to show the use of either \aikenThinHeads or
\aikenThinHeadsMinor.

Thanks,

Carl


https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
In reply to this post by Karlin High
On 2017/09/20 17:48:57, Carl wrote:
> the documentation (Shape Note Heads, in NR 1.1.4 should also be
changed

Okay. I'll see about getting those changes added here.

https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

James Lowe
In reply to this post by Karlin High
Hello Karlin,

Welcome aboard the Goodship LilyPond!

On Wed, 20 Sep 2017 12:46:58 -0500
Karlin High <[hidden email]> wrote:

> On Wed, Sep 20, 2017 at 12:37 PM,  <[hidden email]> wrote:
> > This is my first effort at making a patch, so I could be doing
> > things wrong.  
>
> I gather I am supposed to request write access to the issue tracker on
> Sourceforge? Username: karlinhigh


While you are waiting on getting that, I have created

https://sourceforge.net/p/testlilyissues/issues/5198/

This gets whatever you have done in the queue for testing.

Regards

James

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043by karlinhigh@gmail.com)

Trevor Daniels
In reply to this post by Karlin High

Karlin High Wednesday, September 20, 2017 6:46 PM

> On Wed, Sep 20, 2017 at 12:37 PM,  <[hidden email]> wrote:
>> This is my first effort at making a patch, so I could be doing things wrong.
>
> I gather I am supposed to request write access to the issue tracker on
> Sourceforge? Username: karlinhigh

Thanks.  You should be good to go now.

Trevor


---
This email has been checked for viruses by AVG.
http://www.avg.com
_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
In reply to this post by Karlin High
> On 2017/09/20 17:48:57, Carl wrote:
> the documentation (Shape Note Heads, in NR 1.1.4 should also be
changed

I am uncertain how to go ahead here. Especially with regards to
coordinating with Rietveld 330180043, Sourceforge 5186 that was adding
an Aiken Thin-Heads example snippet to the Notation Reference manual.
That appears committed on September 16, but I can't find it in 2.19 NR
manual online. (Effects of ongoing work towards 2.20 stable release,
perhaps?) I did a git pull and tried following the CG 5.7.1 test scripts
to build the "notation pitches" section locally. I can see the new
snippet in the downloaded source code and the locally-built document,
but something seems broken. I see the Predefined commands section
appearing as online, then Selected Snippets starts with what looks like
part of my bug report code...

}
% END EXAMPLE

<image for Aiken Thin-Heads snippet>

<code for snippet
applying-note-head-styles-depending-on-the-step-of-the-scale.ly>

It's more normal after that, but the paragraph describing the
*step-of-the-scale.ly snippet is not appearing. I will wait for further
comments from James Lowe the snippet author, or anyone else who can
explain the situation here. Is there a problem with the snippet, or am I
building and viewing it wrongly?

Then, any comments on how much documentation should be written? Would
adding \aikenThinHeads and \aikenThinHeadsMinor to the list of
"Predefined commands" be sufficient? Or should a snippet be included
with two systems of c-major-scale white-head notes, one for each Aiken
variant?

https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

pkx166h
In reply to this post by Karlin High
On 2017/09/20 23:38:35, karlinhigh wrote:
> > On 2017/09/20 17:48:57, Carl wrote:
> > the documentation (Shape Note Heads, in NR 1.1.4 should also be
changed

> I am uncertain how to go ahead here. Especially with regards to
coordinating
> with Rietveld 330180043, Sourceforge 5186 that was adding an Aiken
Thin-Heads
> example snippet to the Notation Reference manual. That appears
committed on
> September 16, but I can't find it in 2.19 NR manual online. (Effects
of ongoing
> work towards 2.20 stable release, perhaps?)

No.

This was checked into Staging which then gets pushed to master. AT this
time Master is on 2.21 which is where the snippet will finally appear
once the 2.21 docs are built and published on the website. They may end
up in 2.20 but that's being managed 'separately' as far as this is
concerned.

Just continue to work off of current master.

Bits of current master may get merged into 2.20 but that's a decision
done outside of normal patch testing.

> I did a git pull and tried following
> the CG 5.7.1 test scripts to build the "notation pitches" section
locally. I can
> see the new snippet in the downloaded source code and the
locally-built
> document, but something seems broken. I see the Predefined commands
section
> appearing as online, then Selected Snippets starts with what looks
like part of
> my bug report code...

> }
> % END EXAMPLE

> <image for Aiken Thin-Heads snippet>

> <code for snippet
> applying-note-head-styles-depending-on-the-step-of-the-scale.ly>

> It's more normal after that, but the paragraph describing the
> *step-of-the-scale.ly snippet is not appearing. I will wait for
further comments
> from James Lowe the snippet author, or anyone else who can explain the
situation
> here. Is there a problem with the snippet, or am I building and
viewing it
> wrongly?

It could be something to do with the scripts when the notation pitch
being built locally, I haven't tried that as I always just do a full
make doc as part or patch testing.


> Then, any comments on how much documentation should be written? Would
adding
> \aikenThinHeads and \aikenThinHeadsMinor to the list of "Predefined
commands" be
> sufficient?

That would be better than nothing.

> Or should a snippet be included with two systems of c-major-scale
> white-head notes, one for each Aiken variant?

Well if you can do an @lilypond example instead of a 'snippet' that is
always preferable.

Snippets are usually for things that are not 'simple' or are not
'standard, out of the box' commands.

Anything that is sandwiched between

@lilypond

@end lilypond

is preferred to anything that is a snippet.

The area between snippet and @lilypond example can sometimes be grey,
but usually if you can use an @lilypond construct (which uses the
Lilypond-book part of the code to generate those images in the docs) and
keep it is as simple as possible then those would be great.

One thing my mentor showed me and was not obvious at the time was that
you can build just the LilyPond binary and then use the lilypond-book
command to 'test' your own @lilypond example and see if it looks OK or
will work, without having to compile the whole document.

See:
http://lilypond.org/doc/v2.19/Documentation/usage/lilypond_002dbook-templates

The texinfo template.

You can then run something like  lilypond-book --pdf mytemplate.tely &&
texi2pdf mytemplate.texi && evince mytemplate.pdf

and you'll get a quick idea of what your @lilypond example will look
like using your commands.

I hope this helps.

James


https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Carl Sorensen
In reply to this post by Karlin High
On 2017/09/20 23:38:35, karlinhigh wrote:
> > On 2017/09/20 17:48:57, Carl wrote:
> > the documentation (Shape Note Heads, in NR 1.1.4 should also be
changed

> I am uncertain how to go ahead here.

Literally, just add one line of code to the example in Shape Note Heads,
in NR 1.1.4., that uses
either \aikenThinHeads or \aikenThinHeadsMinor (no need to include
both).  No change in the text
is necessary.


> Then, any comments on how much documentation should be written? Would
adding
> \aikenThinHeads and \aikenThinHeadsMinor to the list of "Predefined
commands" be
> sufficient? Or should a snippet be included with two systems of
c-major-scale
> white-head notes, one for each Aiken variant?

All we want is a single demonstration of the command, not an exhaustive
demonstration.
The documentation policy is to show, don't tell, and to remove as much
as possible.  We
assume the reader to be intelligent and able to extrapolate from given
examples.  The NR is,
by design, NOT a tutorial.

Literally, I think you should add two lines to the example -- one line
for the \aikenThinHeads,
and one line that is a duplicate of the scale shown for all of the other
heads.

THanks,

Carl




https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
On Thu, Sep 21, 2017 at 9:28 AM,  <[hidden email]> wrote:

> Literally, just add one line of code to the example in Shape Note Heads,
> in NR 1.1.4., that uses
> either \aikenThinHeads or \aikenThinHeadsMinor (no need to include
> both).  No change in the text
> is necessary.
>
> All we want is a single demonstration of the command, not an exhaustive
> demonstration.
> The documentation policy is to show, don't tell, and to remove as much
> as possible.  We
> assume the reader to be intelligent and able to extrapolate from given
> examples.  The NR is,
> by design, NOT a tutorial.
>
> Literally, I think you should add two lines to the example -- one line
> for the \aikenThinHeads,
> and one line that is a duplicate of the scale shown for all of the other
> heads.
>
> THanks,
>
> Carl
>
> https://codereview.appspot.com/326510043/

All right, I'm giving up and asking for help. I'm attaching the patch
I tried to git-cl up to Rietveld, and it picked up lots of other
things from I believe the git pull -r I did just prior. I went to
Rietveld and deleted it.

Now, to get just this patch uploaded -- was I supposed to make a new
local branch first?
--
Karlin High
Missouri, USA
(aka "The Show-Me State")

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel

0002-DOC-NR-1.1.4-Shape-Note-Heads-Issue-5187-Add-Thin-Ai.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
On Fri, Sep 22, 2017 at 2:37 PM, Karlin High <[hidden email]> wrote:
> All right, I'm giving up and asking for help.

I got good help from Carl Sorenson; thanks! Changed patch for review,
now contains both code and documentation.
--
Karlin High
Missouri, USA

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Thomas Morley-2
2017-09-23 0:29 GMT+02:00 Karlin High <[hidden email]>:
> On Fri, Sep 22, 2017 at 2:37 PM, Karlin High <[hidden email]> wrote:
>> All right, I'm giving up and asking for help.
>
> I got good help from Carl Sorenson; thanks! Changed patch for review,
> now contains both code and documentation.
> --
> Karlin High
> Missouri, USA



Hi Karlin,

at the time I started using git I was pointed to
https://git-scm.com/book/en/v2

Served me well (and still does)

Cheers,
  Harm

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Karlin High
On Fri, Sep 22, 2017 at 5:46 PM, Thomas Morley <[hidden email]> wrote:
> Hi Karlin,
>
> at the time I started using git I was pointed to
> https://git-scm.com/book/en/v2
>
> Served me well (and still does)
>
> Cheers,
>   Harm

Thanks for the reference! I started reading that (or a prior version)
a year or three ago. Sometimes I have trouble absorbing information
before encountering a need for it - use it or lose it, I'm afraid.
Before this, I haven't had projects that exceeded the version-control
abilities of the "Save As" command - with git I'm still in the mode of
xkcd.com/1597. But digging around in the LilyPond source code gives me
a feeling similar to looking up at the General Sherman Tree. I can see
that git is vital to projects like this.

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

David Nalesnik
In reply to this post by Karlin High
Hi Karlin,

On Fri, Sep 22, 2017 at 5:29 PM, Karlin High <[hidden email]> wrote:
> On Fri, Sep 22, 2017 at 2:37 PM, Karlin High <[hidden email]> wrote:
>> All right, I'm giving up and asking for help.
>
> I got good help from Carl Sorenson; thanks! Changed patch for review,
> now contains both code and documentation.
> --

Glad you got this squared away!  Just for reference you might look at
the email I got when I had a similar problem myself:

https://www.mail-archive.com/lilypond-devel@.../msg51407.html

Best,
David

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Reply | Threaded
Open this post in threaded view
|

Re: Issue 5187 Add command for Thin Aiken noteheads (issue 326510043 by karlinhigh@gmail.com)

Carl Sorensen
In reply to this post by Karlin High
LGTM

https://codereview.appspot.com/326510043/

_______________________________________________
lilypond-devel mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/lilypond-devel