Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[graham]

LGTM

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[Keith OHara]

On Sun, 01 Jan 2012 14:33:33 -0800,  wrote:


I disagree.

The example in the tracker is overly complicated for what (I still
maintain) is easily explained. However *if* we really do need an example
then the \hideNotes one is not ideal. I struggled myself with that one
in the tracker, it isn't a 'tiny example' by any stretch of the
imagination and I knew perfectly well what was meant by the problem only
*after* I read the text explanation, the picture on the tracker added
nothing for me.



Okay, then.  You are addressing the fact that the beam was not un-hidden 
mid-stroke, because no properties are changed mid-stroke.  So this makes sense 
as a knownissue

The other way looking at the bug report is that \hideNotes seems to have removed the 
automatic beam.  ( What is your man going on about with the "beam properties"?  
I don't see any beam.  Do you see a beam? )

For that aspect, I'll add a few words and beamed notes to the second example of 
NR 1.7.1 \hideNotes :
+ Note heads, stems, flags, and beams are invisible, but other
= objects that are attached to invisible notes are still visible.


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[pkx166h]

See comments below, thanks.


http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1953
Documentation/notation/rhythms.itely:1953: @knownissues
On 2012/01/01 01:55:11, Carl wrote:

I wonder if it would be better to actually include a snippet, instead

of just a

known issue.


See comment below

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1954
Documentation/notation/rhythms.itely:1954: The properties of a beam are
determined at the @emph{start} of its
On 2012/01/01 02:59:06, Keith wrote:

I had no idea what this was about until going back to the tracker

issue.  The

surprising bit is with automatic beams, because you don't see where

they start,

so we need an example.  You might just say :
"If you change properties of beam when a beam has already started,

that beam

will not be affected.  For example, with the input {\hideNotes c8 f8
\unHideNotes c8 f8} the \hideNotes makes transparent the single

automatic beam

across all four notes.  If you want the beam to be visible for the

last two

notes you need to specify it explicitly. {\hideNotes c8 f8

\unHideNotes c8[ f8]}

"
This one is more a warning than a knownissue.


Happy with @warning vs @knownissues

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1954
Documentation/notation/rhythms.itely:1954: The properties of a beam are
determined at the @emph{start} of its
On 2012/01/01 03:08:19, Carl wrote:

On 2012/01/01 02:59:06, Keith wrote:
> I had no idea what this was about until going back to the tracker

issue.  The

> surprising bit is with automatic beams, because you don't see where

they

start,
> so we need an example.  You might just say :
> "If you change properties of beam when a beam has already started,

that beam

> will not be affected.  For example, with the input {\hideNotes c8 f8
> \unHideNotes c8 f8} the \hideNotes makes transparent the single

automatic beam

> across all four notes.  If you want the beam to be visible for the

last two

> notes you need to specify it explicitly. {\hideNotes c8 f8

\unHideNotes c8[

f8]}



This is why I recommend a snippet, rather than just the text.  If we

need to

show a LilyPond example, we should show it with the output, rather

than just

describing it in the text.


I disagree.

The example in the tracker is overly complicated for what (I still
maintain) is easily explained. However *if* we really do need an example
then the \hideNotes one is not ideal. I struggled myself with that one
in the tracker, it isn't a 'tiny example' by any stretch of the
imagination and I knew perfectly well what was meant by the problem only
*after* I read the text explanation, the picture on the tracker added
nothing for me.

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1956
Documentation/notation/rhythms.itely:1956: the beam has been completed
will not take effect until the @emph{next},
On 2012/01/01 01:55:11, Carl wrote:

I think it would be better to eliminate the , and the word "new", but

I don't

feel really strongly about it.


No problem with that.

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[pkx166h]

On 2012/01/01 21:31:50, Keith wrote:

On 2012/01/01 15:06:05, dak wrote:
> On 2012/01/01 14:56:25, Carl wrote:
> > > Can we show this using a single voice with non-chorded notes and

by using

> > > something more 'obvious' than \hideNotes?



\relative c' {
   \voiceTwoStyle
   c8 e \voiceNeutralStyle g c
   \voiceTwoStyle
   c8 e \voiceNeutralStyle g[ c]
}


Seriously?



Remember that the original bug reporter was not trying to "change any

properties

of a beam" but only using a pre-defined function.


Yes but also remember that this is not an attempt to fix the reporters
issue but document it for others that do not necessarily use complex
constructions like \voiceTwoStyle and \voiceNeutralStyle.


> Also \once\hideNotes might be a bit more snappy.



Hey, that works now!  Thanks, David.
To make the desired point, it would need to hide the first note and

the beam,

which would need some explanation to point out what is going on.


Or we could just show a simple case of beams not being colored in when
the override happens mid-beam (so to speak).

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[k-ohara5a5a]

On 2012/01/01 15:06:05, dak wrote:

On 2012/01/01 14:56:25, Carl wrote:
> > Can we show this using a single voice with non-chorded notes and

by using

> > something more 'obvious' than \hideNotes?


\relative c' {
  \voiceTwoStyle
  c8 e \voiceNeutralStyle g c
  \voiceTwoStyle
  c8 e \voiceNeutralStyle g[ c]
}

Remember that the original bug reporter was not trying to "change any
properties of a beam" but only using a pre-defined function.


Also \once\hideNotes might be a bit more snappy.


Hey, that works now!  Thanks, David.
To make the desired point, it would need to hide the first note and the
beam, which would need some explanation to point out what is going on.


http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[dak]

On 2012/01/01 14:56:25, Carl wrote:

> Can we show this using a single voice with non-chorded notes and by

using

> something more 'obvious' than \hideNotes?



Override the beam color to red?


Also \once\hideNotes might be a bit more snappy.

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[Carl . D . Sorensen]

Can we show this using a single voice with non-chorded notes and by

using

something more 'obvious' than \hideNotes?


Override the beam color to red?





http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[pkx166h]

Reviewers: carl.d.sorensen_gmail.com, Keith,

Message:
On 2012/01/01 03:08:19, Carl wrote:

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely

File Documentation/notation/rhythms.itely (right):



http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1954

Documentation/notation/rhythms.itely:1954: The properties of a beam

are

determined at the @emph{start} of its
On 2012/01/01 02:59:06, Keith wrote:
> I had no idea what this was about until going back to the tracker

issue.  The

> surprising bit is with automatic beams, because you don't see where

they

start,
> so we need an example.  You might just say :
> "If you change properties of beam when a beam has already started,

that beam

> will not be affected.  For example, with the input {\hideNotes c8 f8
> \unHideNotes c8 f8} the \hideNotes makes transparent the single

automatic beam

> across all four notes.  If you want the beam to be visible for the

last two

> notes you need to specify it explicitly. {\hideNotes c8 f8

\unHideNotes c8[

f8]}



This is why I recommend a snippet, rather than just the text.  If we

need to

show a LilyPond example, we should show it with the output, rather

than just

describing it in the text.


OK, but I think the example of \hideNotes, (that of the tracker) is too
complicated (or unnecessarily so).

Can we show this using a single voice with non-chorded notes and by
using something more 'obvious' than \hideNotes?



Description:
Doc: NR added @knownissue for beam properties

Used Carl S's wording from comment #3 of Tracker 1566

Also collated to separate @warnings together as they looked odd in the
doc
as two separate 'note' boxes one after the other.

Please review this at http://codereview.appspot.com/5504100/

Affected files:
  M Documentation/notation/rhythms.itely


Index: Documentation/notation/rhythms.itely
diff --git a/Documentation/notation/rhythms.itely  
b/Documentation/notation/rhythms.itely
index  
5719c73ea7e0eca3c1a02cefc77aeff049756ce3..9fb0c1961efefec32f68d5a70a4b6c1cd07e4337  
100644

--- a/Documentation/notation/rhythms.itely
+++ b/Documentation/notation/rhythms.itely
@@ -1899,10 +1899,9 @@ c16 c8

 @warning{If beams are used to indicate melismata in songs, then
 automatic beaming should be switched off with @code{\autoBeamOff}
-and the beams indicated manually.}
-
-@warning{Using @code{@bs{}partcombine} with @code{@bs{}autoBeamOff} can
-produce unintended results.  See the snippet below for more information.}
+and the beams indicated manually.  Using @code{@bs{}partcombine} with
+@code{@bs{}autoBeamOff} can produce unintended results.  See the
+snippets for more information.}

 Beaming patterns that differ from the automatic defaults can be
 created; see @ref{Setting automatic beam behavior}.
@@ -1951,6 +1950,13 @@ Internals Reference:
 @rinternals{beam-interface},
 @rinternals{unbreakable-spanner-interface}.

+@knownissues
+The properties of a beam are determined at the @emph{start} of its
+construction and any additional beam-property changes that occur before
+the beam has been completed will not take effect until the @emph{next},
+new beam starts.
+
+
 @node Setting automatic beam behavior
 @unnumberedsubsubsec Setting automatic beam behavior




___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[Carl . D . Sorensen]

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1954
Documentation/notation/rhythms.itely:1954: The properties of a beam are
determined at the @emph{start} of its
On 2012/01/01 02:59:06, Keith wrote:

I had no idea what this was about until going back to the tracker

issue.  The

surprising bit is with automatic beams, because you don't see where

they start,

so we need an example.  You might just say :
"If you change properties of beam when a beam has already started,

that beam

will not be affected.  For example, with the input {\hideNotes c8 f8
\unHideNotes c8 f8} the \hideNotes makes transparent the single

automatic beam

across all four notes.  If you want the beam to be visible for the

last two

notes you need to specify it explicitly. {\hideNotes c8 f8

\unHideNotes c8[ f8]}

This is why I recommend a snippet, rather than just the text.  If we
need to show a LilyPond example, we should show it with the output,
rather than just describing it in the text.

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Doc: NR added @knownissue for beam properties (issue 5504100)

[k-ohara5a5a]

LGTM


http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely
File Documentation/notation/rhythms.itely (right):

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1903
Documentation/notation/rhythms.itely:1903: @code{@bs{}autoBeamOff} can
produce unintended results.  See the
"can produce unintended results" is a too vague to be helpful.
If I understand, the point is that autoBeamOff affects the current voice
only, which can be surprising when used with features that automatically
create voices like \partcombine.

http://codereview.appspot.com/5504100/diff/1/Documentation/notation/rhythms.itely#newcode1954
Documentation/notation/rhythms.itely:1954: The properties of a beam are
determined at the @emph{start} of its
I had no idea what this was about until going back to the tracker issue.
 The surprising bit is with automatic beams, because you don't see where
they start, so we need an example.  You might just say :
"If you change properties of beam when a beam has already started, that
beam will not be affected.  For example, with the input {\hideNotes c8
f8 \unHideNotes c8 f8} the \hideNotes makes transparent the single
automatic beam across all four notes.  If you want the beam to be
visible for the last two notes you need to specify it explicitly.
{\hideNotes c8 f8 \unHideNotes c8[ f8]} "
This one is more a warning than a knownissue.

http://codereview.appspot.com/5504100/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel