Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split fills #161

Merged
merged 6 commits into from
Mar 3, 2014
Merged

Split fills #161

merged 6 commits into from
Mar 3, 2014

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented Mar 1, 2014

I am not entirely happy with this, since it seems overly complicated, and even I am not quite sure I believe it is really correct. Still, it does help squish a tricky bug (diagrams/diagrams-svg#43) which is actually a problem for some users (timbod7/haskell-chart#19), and passes the tests I could think of (see test/SplitAttr.hs).

See also the discussion at #141 .

@bergey
Copy link
Member

bergey commented Mar 3, 2014

It does seem annoyingly complicated for something conceptually simple. But I think we should put it into 1.1 even if we hope to do something nicer eventually.

@jeffreyrosenbluth, have you had a chance to look at this? You've thought more about diagrams/diagrams-svg#47 and RTree than I have.

@jeffreyrosenbluth
Copy link
Member

Actually I did have a chance to give it a fairly careful read. And although
it is a very nice piece of code, I agree with you both that it feels like
it is overly complicated given the task at hand. But until we find another
solution I also agree that we should include it in 1.1.

On Sun, Mar 2, 2014 at 7:00 PM, Daniel Bergey notifications@github.comwrote:

It does seem annoyingly complicated for something conceptually simple. But
I think we should put it into 1.1 even if we hope to do something nicer
eventually.

@jeffreyrosenbluth https://github.com/jeffreyrosenbluth, have you had a
chance to look at this? You've thought more about diagrams/diagrams-svg#47https://github.com/diagrams/diagrams-svg/pull/47and
RTree than I have.

Reply to this email directly or view it on GitHubhttps://github.com//pull/161#issuecomment-36472618
.

bergey added a commit that referenced this pull request Mar 3, 2014
Move FillColor Attributes to subtrees with only Loops
@bergey bergey merged commit 0d4e019 into master Mar 3, 2014
@bergey bergey deleted the split-fills branch October 10, 2015 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants