Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(210)

Issue 11427044: adding markup-commands \oval and \ellipse (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by thomasmorley651
Modified:
10 years, 7 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

adding markup-commands \oval and \ellipse \oval and \ellipse are drawing an oval respectively an ellipse around their argument. Also adding a reg-test for markup-commands framing their argument.

Patch Set 1 #

Patch Set 2 : distinguish padding of x and y-axis #

Patch Set 3 : remove tabs #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -0 lines) Patch
A input/regression/markup-frame-text.ly View 1 chunk +15 lines, -0 lines 0 comments Download
M scm/define-markup-commands.scm View 1 2 1 chunk +56 lines, -0 lines 1 comment Download

Messages

Total messages: 6
thomasmorley651
Please review. (git-cl seems to work - currently)
10 years, 8 months ago (2013-07-17 21:05:39 UTC) #1
thomasmorley651
distinguish padding of x and y-axis
10 years, 8 months ago (2013-07-18 07:44:21 UTC) #2
thomasmorley651
remove tabs
10 years, 8 months ago (2013-07-18 07:49:17 UTC) #3
Graham Percival
LGTM
10 years, 8 months ago (2013-07-21 23:17:47 UTC) #4
david.nalesnik
LGTM Thanks for adding these, Harm.
10 years, 8 months ago (2013-07-22 23:46:09 UTC) #5
dak
10 years, 8 months ago (2013-07-23 10:00:54 UTC) #6
https://codereview.appspot.com/11427044/diff/9001/scm/define-markup-commands.scm
File scm/define-markup-commands.scm (right):

https://codereview.appspot.com/11427044/diff/9001/scm/define-markup-commands....
scm/define-markup-commands.scm:416: (x-padding 0.75)
I am not enthused about the use of x-padding and y-padding here (no other
commands have them).  Boxes (of all kind) have box-padding (for both x and y),
circles have circle-padding.

It does make sense to have padding names that don't change when the padded
element changes.  It is somewhat annoying that overrides encompass all enclosed
material (particularly for things like word-space and similar) rather than just
the top level.

At any rate, having things like omnidirectional oval-padding and ellipse-padding
would not likely make me much happier.  So I'm just expressing my displeasure at
a dissatisfactory state that is not really the fault of this patch.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b