Display compile errors in Try CoffeeScript more clearly #1728

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
6 participants
@banksJeremy
Contributor

banksJeremy commented Sep 23, 2011

The error message the display for a compile error in Try CoffeeScript isn't very noticeable, and is often cut off because of the lack of room.

This change displays the compile error in red where the compiled source code would be, and dims the "Run" button to indicate that it's disabled.

You can see this running on my gh-pages branch.

@michaelficarra

This comment has been minimized.

Show comment Hide comment
@michaelficarra

michaelficarra Sep 23, 2011

Collaborator

Very nice, big +1 from me. Code LGTM as well. One nitpick: why name the variable $results?

Collaborator

michaelficarra commented Sep 23, 2011

Very nice, big +1 from me. Code LGTM as well. One nitpick: why name the variable $results?

@banksJeremy

This comment has been minimized.

Show comment Hide comment
@banksJeremy

banksJeremy Sep 23, 2011

Contributor

In stuff I've been working on recently, the $- prefix is used to indicate variables containing jQuery objects. Should I change it to results?

Contributor

banksJeremy commented Sep 23, 2011

In stuff I've been working on recently, the $- prefix is used to indicate variables containing jQuery objects. Should I change it to results?

@michaelficarra

This comment has been minimized.

Show comment Hide comment
@michaelficarra

michaelficarra Sep 23, 2011

Collaborator

I don't really care, but it would be more consistent, so probably, yeah.

Collaborator

michaelficarra commented Sep 23, 2011

I don't really care, but it would be more consistent, so probably, yeah.

@vendethiel

This comment has been minimized.

Show comment Hide comment
@vendethiel

vendethiel Feb 13, 2014

Collaborator

@michaelficarra / @jashkenas That'd be cool to have.

Collaborator

vendethiel commented Feb 13, 2014

@michaelficarra / @jashkenas That'd be cool to have.

@michaelficarra

This comment has been minimized.

Show comment Hide comment
@michaelficarra

michaelficarra Feb 14, 2014

Collaborator

Agreed. If someone wants to rebase this, I'd gladly merge it.

Collaborator

michaelficarra commented Feb 14, 2014

Agreed. If someone wants to rebase this, I'd gladly merge it.

@jashkenas

This comment has been minimized.

Show comment Hide comment
@jashkenas

jashkenas Feb 14, 2014

Owner

Yeah, this looks great. I never liked shoving them up there in the corner in the first place.

Owner

jashkenas commented Feb 14, 2014

Yeah, this looks great. I never liked shoving them up there in the corner in the first place.

@xixixao

This comment has been minimized.

Show comment Hide comment
@xixixao

xixixao Feb 17, 2014

Contributor

+1 to get rid of the capitalization, try

"wrong" "code"

to see how it messes up the tokens in the error message.

Contributor

xixixao commented Feb 17, 2014

+1 to get rid of the capitalization, try

"wrong" "code"

to see how it messes up the tokens in the error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment