cappuccino / cappuccino

Web Application Framework in JavaScript and Objective-J
https://cappuccino.dev/
GNU Lesser General Public License v2.1
2.21k stars 333 forks source link

Define functions that are missing for CGContextCanvas [+1] #1403

Closed btandresen closed 11 years ago

btandresen commented 12 years ago

I found a few functions that are defined, as documented, but only when the canvas feature is not available:

I've added definitions for these to the canvas-enabled backend.

This AppController.j can be used to test all three functions.

aparajita commented 12 years ago

Please provide a little test app.

btandresen commented 12 years ago

Here's a test case: https://gist.github.com/1376833

On Firefox 7.0.1/Mac and Safari 5.1.1/Mac, using Cappuccino 0.9.5, I get an error that CGContextAddLines is not defined. With the patch, the function is found, and the test case draws a diamond.

aparajita commented 12 years ago

Please write a test case that tests all of the new functions you added.

btandresen commented 12 years ago

Test case updated to test all three functions.

cappbot commented 12 years ago

Assignee: aparajita. Label: #new. What's next? A reviewer should examine this issue.

thewalkingtoast commented 12 years ago

I have verified this as working. Needs linting as capp_lint is throwing an unterminated var block error when evaluating

var sign = (a * d < 0.0 || b * c > 0.0) ? -1.0 : 1.0, \
            a2 = (ATAN2(b, d) + ATAN2(-sign * c, sign * a)) / 2.0, \
            cos = COS(a2),\
            sin = SIN(a2);\
        \

+1

+#needs-review +#accepted +AppKit

cappbot commented 12 years ago

Assignee: aparajita. Vote: 1. Labels: #accepted, AppKit. What's next? A reviewer should examine this issue.

aparajita commented 12 years ago

Thanks for taking the time to test it. That capp_lint warning is a false positive, capp_lint can't parse #defines yet.

Needs some small code changes.

needs-improvement

aparajita commented 12 years ago

By the way, if you want to put a multi-line code block in github comments, use javascript on a line by itself to start the code block and on a line by itself to end it.

thewalkingtoast commented 12 years ago

What changes need to be made? capp_lint reported no other errors in either file than the one I mentioned above.

cappbot commented 12 years ago

Assignee: aparajita. Vote: 1. Labels: #accepted, #needs-improvement, AppKit. What's next? The code for this issue has problems with formatting or fails a capp_lint check, has bugs, or has non-optimal logic or algorithms. It should be improved upon.

thewalkingtoast commented 12 years ago

+#needs-review

ahankinson commented 11 years ago

All of the review issues seem to have been taken care of.

milestone=0.9.7 +#ready-to-commit

cappbot commented 11 years ago

Assignee: aparajita. Milestone: 0.9.7. Vote: 1. Labels: #accepted, #ready-to-commit, AppKit. What's next? The changes for this issue are ready to be committed by aparajita.

aparajita commented 11 years ago

-#ready-to-commit +#needs-improvement

We should probably close this PR and get a new one from @aradabaugh, his branch has the fixes mentioned in the review.

cappbot commented 11 years ago

Assignee: aparajita. Milestone: 0.9.7. Vote: 1. Labels: #accepted, #needs-improvement, AppKit. What's next? The code for this issue has problems with formatting or fails a capp_lint check, has bugs, or has non-optimal logic or algorithms. It should be improved upon.

ahankinson commented 11 years ago

Ah! Good catch. I didn't notice that his branch didn't get folded into the PR.

Also, I noticed that while @btandresen provided an AppController.j to test the functions, it's not an actual test case. I'm not up-to-snuff on how to write test cases for CG* methods, so it would be great if the new PR had one attached.

aparajita commented 11 years ago

@ahankinson Any chance you can turn this into a proper pull request from a separate branch and fix the issues I commented on?

ahankinson commented 11 years ago

Sure. It may have to wait a couple days, though. I've got some catching up on non-Capp work to do.

btandresen commented 11 years ago

Thank you, gentlemen!

On Tue, Feb 19, 2013 at 8:03 AM, Andrew Hankinson notifications@github.comwrote:

Sure. It may have to wait a couple days, though. I've got some catching up on non-Capp work to do.

— Reply to this email directly or view it on GitHubhttps://github.com/cappuccino/cappuccino/pull/1403#issuecomment-13780256.

thewalkingtoast commented 11 years ago

Close this PR. I submitted a new one with the fixes and a test.

aparajita commented 11 years ago

Thank you!

wont-fix

cappbot commented 11 years ago

Assignee: aparajita. Milestone: 0.9.7. Vote: 1. Labels: #wont-fix, AppKit. What's next? A reviewer or core team member has decided against acting upon this issue.