Reviewers: ,
Message:
please review
Description:
Implement make-bow-stencil, make-tie-stencil for use in markup-commands
undertie and overtie
issue 3088
In a follow up it is planned to replace make-parenthesis-stencil with
an appropiate setting of make-bow-stencil and to partially rework the
p
Man, sorry, while not being up to your coding skills, I can make
nitpicks…
https://codereview.appspot.com/270640043/diff/1/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/1/scm/stencil.scm#newcode50
scm/stencil.scm:50: @var{bow-height} determines the
thanks for review
https://codereview.appspot.com/270640043/diff/1/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/1/scm/stencil.scm#newcode50
scm/stencil.scm:50: @var{bow-height} determines the heigth of the bow.
On 2015/11/04 00:28:27, simon.albrecht
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm#newcode76
scm/stencil.scm:76: (cons 0 0))
Why not '(0 . 0) here? It does not look like the cons is ever changed,
so why a fresh c
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm#newcode88
scm/stencil.scm:88: (sqrt (+ (* width width) (* height height
On 2015/11/08 21:22:04, dak wrote:
Is there a reason
On 2015/11/08 23:22:51, thomasmorley651 wrote:
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm#newcode88
scm/stencil.scm:88: (sqrt (+ (* width width) (* height height
On 2
On 2015/11/08 23:54:37, dak wrote:
On 2015/11/08 23:22:51, thomasmorley651 wrote:
> https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm
> File scm/stencil.scm (right):
>
>
https://codereview.appspot.com/270640043/diff/20001/scm/stencil.scm#newcode88
> scm/stencil.scm:88: (sqrt (
https://codereview.appspot.com/270640043/diff/40001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/40001/scm/stencil.scm#newcode73
scm/stencil.scm:73: (width (- (car stop) (car start)))
I'd not use `width' (and `height') here but `dx' and `dy': the
ex
On 2015/11/10 20:52:18, dak wrote:
https://codereview.appspot.com/270640043/diff/40001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/40001/scm/stencil.scm#newcode73
scm/stencil.scm:73: (width (- (car stop) (car start)))
I'd not use `width' (and `h
On 2015/11/11 09:56:16, thomasmorley651 wrote:
Well, outer/inner-control rely on the user-settable variables
bow-height and
thickness.
Ok, didn't notice that. I'm not sure it's the best interface to have
those in semi-absolute dimensions but at least with thickness it would
be sort-of consi
On 2015/11/12 15:20:51, dak wrote:
On 2015/11/11 09:56:16, thomasmorley651 wrote:
> Well, outer/inner-control rely on the user-settable variables
bow-height and
> thickness.
Ok, didn't notice that. I'm not sure it's the best interface to have
those in
semi-absolute dimensions but at le
Sorry for yet finding more stuff that, after all, could likely be
improved. At least I did so pretty quickly this time.
https://codereview.appspot.com/270640043/diff/60001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
https://codereview.appspot.com/270640043/diff/
On 2015/11/13 21:51:19, dak wrote:
Sorry for yet finding more stuff that, after all, could likely be
improved. At
least I did so pretty quickly this time.
No problem!
The code became so much better during revision!
I have to work too much in my regular job to upload more than one
patch-set p
Aaand another one.
https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm#newcode66
scm/stencil.scm:66: (if (equal? start stop)
I think that's too optimistic. If you have
start = (0 .
On 2015/11/13 22:38:02, thomasmorley651 wrote:
On 2015/11/13 21:51:19, dak wrote:
> Sorry for yet finding more stuff that, after all, could likely be
improved.
At
> least I did so pretty quickly this time.
No problem!
The code became so much better during revision!
I have to work too much in
On 2015/11/13 23:01:39, dak wrote:
Aaand another one.
https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm
File scm/stencil.scm (right):
https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm#newcode66
scm/stencil.scm:66: (if (equal? start stop)
I think that's t
https://codereview.appspot.com/270640043/diff/60001/scm/define-markup-commands.scm#newcode623
> > scm/define-markup-commands.scm:623: (direction DOWN)
> > Should this markup command be called "undertie" or should it
rather be
"tie",
> > with "undertie" explicitly overriding `direction'?
> >
>
On 2015/11/13 21:51:19, dak wrote:
Sorry for yet finding more stuff that, after all, could likely be
improved. At
least I did so pretty quickly this time.
https://codereview.appspot.com/270640043/diff/60001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
http
On 2015/11/13 23:15:22, thomasmorley651 wrote:
On 2015/11/13 23:01:39, dak wrote:
> Aaand another one.
>
> https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm
> File scm/stencil.scm (right):
>
>
https://codereview.appspot.com/270640043/diff/60001/scm/stencil.scm#newcode66
> scm/
On 2015/11/14 21:25:11, thomasmorley651 wrote:
On 2015/11/13 21:51:19, dak wrote:
> Sorry for yet finding more stuff that, after all, could likely be
improved.
At
> least I did so pretty quickly this time.
>
>
https://codereview.appspot.com/270640043/diff/60001/scm/define-markup-commands.scm
On 2015/11/16 15:57:49, dak wrote:
On 2015/11/14 21:25:11, thomasmorley651 wrote:
> On 2015/11/13 21:51:19, dak wrote:
> > Sorry for yet finding more stuff that, after all, could likely be
improved.
> At
> > least I did so pretty quickly this time.
> >
> >
>
https://codereview.appspot.com/270
LGTM, apart from an oversight, although this is based on just
eye-balling.
https://codereview.appspot.com/270640043/diff/11/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):
https://codereview.appspot.com/270640043/diff/11/scm/define-markup-commands.scm#newcod
On 2015/11/18 23:01:20, thomasmorley651 wrote:
On 2015/11/16 15:57:49, dak wrote:
> If we are sure we'll never need to change the direction based on the
> markup direction (like, say, over/under actual note glyphs?), I'd
just
implement
> commands undertie/overtie that call some internal func
On 2015/11/19 21:29:41, thomasmorley651 wrote:
On 2015/11/18 23:01:20, thomasmorley651 wrote:
> On 2015/11/16 15:57:49, dak wrote:
> > If we are sure we'll never need to change the direction based on
the
> > markup direction (like, say, over/under actual note glyphs?), I'd
just
> implement
24 matches
Mail list logo
