-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Inconsistencies in prototype
shorthand (::) and this
shorthand (@)
#1601
Comments
Making it callable to be more consistent is one thing ... but there's never a real-world use case where your prototype object is actually a function you'd want to call ... is there? I certainly can't think of one. |
@jashkenas Agreed; [I couldn't find any use-cases](http://www.google.com/codesearch#search/&q=%5C.prototype%5C(%20lang:%5Ejavascript$%20case:yes&p=6&sq=&type=cs). In fact, it might cause some confusion/issues. Updated the issue. |
@jashkenas: Even if it's not a common use case, it's better to allow implicit calls so that it is consistent with a regular member access. I agree with everything from @geraldalewis's (edit: original) post, including the recommendation. For the unary |
Personally, I'm somewhat a fan of disallowing standalone For readability's sake, I don't know why you wouldn't always choose to write: |
One more consistency worth noting:
They can both have |
@jashkenas: I think standalone |
@jashkenas: standalone |
@michaelficarra You raise some good points. Regardless of its practical viability, the proposal calls for symmetry with
Callable @jashkenas I favored standalone Also, another inconsistency:
|
Note that
|
@satyr That's interesting. What was the rationale for not allowing
? |
@satyr: Nice catch. We should make it so. @geraldalewis: #1089. It really should be allowed. |
Also, |
What's the expected compilation? |
@satyr, oops, that was never valid JS to begin with. Sorry about that, just assumed it was. |
@geraldalewis: what's the status of this issue? Are you still planning to work on it? I'd love to see this all fixed. |
I'm still in favor of proposal
I haven't moved forward with a patch because it didn't look like it had strong support (@jashkenas voiced misgivings about having standalone #1089 ( |
Sounds like we're removing it. And with that logic, |
Yep, I'm afraid I still have strong misgivings about it.
you should equally be able to write:
Again, |
I think this is where standalone- this.
property #this.property;
@
property #parse error
Again, there's an inconsistency with standalone- this. a # this.a;
@ a # this(a); |
Yes, and that's why standalone Does it want to mean |
Now I get it. Even if we patched the inconsistency between In favor of removing standalone- |
A good change for a 1.2 release? Let's do it. |
So I'm still not quite clear on the status of this discussion. Does this mean we're getting rid of both standalone |
Yes please. Let's keep the sigils limited to the cases where the trailing dot makes sense. |
Okay, I'm cool with that. It makes sense. There's going to be some unhappy users though. Can't wait for the backlash... |
+1 We support the removal of ambiguity and thus welcome change. Although this one is particularly bitter-sweet since we have At least it's reasonably easy to replace. Now would be a great time to sanction a single option for the other ambiguous parts of CoffeeScript. |
Totally in favor of these changes. I never used standalone +100 |
Any issues in particular come to mind? |
vs
vs
|
Sure thing, |
OK, so there are clearly a very wide range of opinions here (even just on the issue of the standalone Sound reasonable? |
Late to this party… but… the issue came up in one of our pull requests, so… I agree with @showell and @michaelficarra that we should think of What that doesn't really do is justify the removal of the standalone Now, if only |
Really i don't understand standalone We have To be consistent we should always use So we have 3 choices: f = ->
@ f = ->
this f = ->
return @ imho
|
@artyomtrityak The last comment on this issue was 2 years ago, |
I removed standalone |
Add and remove classes according to the values in SnippetView.model.styles. Style changes trigger SnippetTree#snippetHtmlChanged events which are handled in the Renderer.
A lot of people do like the bare |
I have an extended class I achieve this with |
Skimming through this thread, I don’t see a consensus on any action to be taken as a result of the original proposal. If I’m mistaken on this please comment and I’ll reopen, but at this point if anyone desires further changes to |
The
::
.prototype(.)
shorthand should behave consistently with the@
this(.)
shorthand.Adding symmetry in how each shorthand is implemented will give developers a better mental model of their behavior and reduce issues.
@
currently compiles to a literalthis
, accessthis.
or invocationthis(
depending on context.::
currently compiles to a literal.prototype
or access.prototype.
depending on context.It does not compile to an invocation.
@
will not behave as::
in every context. Below is a breakdown of where they behave consistently and where they behave inconsistently.Consistent
as value (unless :: standalone)
member access (unless :: standalone)
index
applying
::
Inconsistent
invocation
list comprehensions (from #1477 by @dbrans)
instanceof
standalone
standalone member access
Proposal
::
should:@
in all other ways:be callableEdit: See @jashkenas comment(it technically is, but it looks like this line prevents implicit calls)
or
::
should always compile to.prototype.
@
should always compile tothis.
@
should continue linesPersonally, I'm in favor of
#1
Related issues: #1477, #1554 (fixed), #1234 (fixed)
The text was updated successfully, but these errors were encountered: