satyr / coco

Unfancy CoffeeScript
http://satyr.github.com/coco/
MIT License
498 stars 48 forks source link

Cascade implicit block hides missing `do` syntax errors #179

Closed qqueue closed 11 years ago

qqueue commented 11 years ago
a
 b
  c
   d
    e

compiles to

var x$, y$, z$, z1$;
x$ = a;
y$ = b;
z$ = c;
z1$ = d;
e;

even though the cascadee isn't used. This hides some typos involving a missing do that were previously caught at compile time, such as:

method
   arg1
   arg2
var x$;
x$ = method;
arg1;
arg2;

and even though this works when run, it hides the fact that |> doesn't actually accept an implicit block:

program = @createProgram! |>
    @attachShader _, vertex-shader
    @attachShader _, fragment-shader
    @linkProgram _
    unless @getProgramParameter _, LINK_STATUS
      throw new Error "couldn't intialize shader program!"
    @useProgram _
var x$, program, _;
x$ = (_ = program = this.createProgram(), this.attachShader(_, vertexShader));
this.attachShader(_, fragmentShader);
this.linkProgram(_);
if (!this.getProgramParameter(_, LINK_STATUS)) {
  throw new Error("couldn't intialize shader program!");
}
this.useProgram(_);

Proposal: cascade blocks in which the cascadee isn't used at least once should be invalid syntax. The error message should be similar to "Unexpected INDENT on line xx:xx, did you forget a 'do' or a cascade usage?"

qqueue commented 11 years ago

Also regarding the last example, it only works because |> binds to the actual _ variable for the rest of the scope, which--though I haven't personally run into this problem yet--also affects the use of underscore.js' default binding:

_ = require \underscore
thing.create! |>
  _.foo!

_.map [1,2,3] -> it.toString! # probably won't work
var _;
_ = require('underscore');
_ = thing.create();
_.foo();
_.map([1, 2, 3], function(it){
  return it.toString();
});
satyr commented 11 years ago

Or maybe ditch this syntax and stick with with as per #172.

effects the use of underscore.js' default binding

That's just a naming issue. You should be doing L = require \lodash anyway. ;)

qqueue commented 11 years ago

Using with does require an arduous 5 extra characters though.

Perhaps for the minimal case of cascading when you don't need the cascadee as the result, |> could be overloaded as the operator, so this would actually compile as expected, without an extra do:

program = @createProgram! |>
    @attachShader (&), vertex-shader
    @attachShader (&), fragment-shader
    @linkProgram (&)
    unless @getProgramParameter (&), LINK_STATUS
      throw new Error "couldn't intialize shader program!"
    @useProgram (&)
x$ = program = this.createProgram()
this.attachShader(x$, vertexShader));
this.attachShader(x$, fragmentShader);
this.linkProgram(x$);
if (!this.getProgramParameter(x$, LINK_STATUS)) {
  throw new Error("couldn't intialize shader program!");
}
this.useProgram(x$);

The need to wrap & in this use case still sucks though, per #170.

But hey, if the issues with & can be resolved, you could even change the, uh, pipee to & and solve the underscore clobbering problem as well.

thing |> & + & |> &trim! |> [&, &, &]

Maybe do LiveScript's .&. wrapping thing for the real bitwise AND.

satyr commented 11 years ago

The two don't merge well. Pipe returns the right hand result where cascade the cascadee (#172).

Maybe do LiveScript's .&. wrapping thing for the real bitwise AND

That's a no-no. We want no more cognitive overhead against JS.

qqueue commented 11 years ago

Pipe returns the right hand result where cascade the cascadee

But the cascade in it's current form is essentially the statement version of the pipe:

thing_pipe = ->
  thing |> do
    _.stuff
    foo _
    bar _, baz

thing_cascade = ->
  thing
    &stuff
    foo (&)
    bar (&), baz
var thing_pipe, thing_cascade;
thing_pipe = function(){
  var _;
  _ = thing;
  return _.stuff, foo(_), bar(_, baz);
};
thing_cascade = function(){
  var x$;
  return (x$ = thing, x$.stuff, foo(x$), bar(x$, baz));
};

The |> do form is practically the same as the cascade, and in statement position could certainly be compiled to statements instead of expressions for readability.

Here's another thought: Change the cascadee and pipee to @, and override the shortcut to this within cascades and pipes. @ doesn't have the problems & does, while retaining the single character benefit, and this can still be referenced by its full name if it needs to.

thing |> @ + @ |> @trim! |> [@, @, @]
satyr commented 11 years ago

But the cascade in it's current form is essentially the statement version of the pipe

Not in the next version. See the conclusion of #172.

Change the cascadee and pipee to @, and override the shortcut to this within cascades and pipes

Seems way too complicated.

qqueue commented 11 years ago

Seems way too complicated

But more, or less complicated than

thing.create!
  & & &

? It's pretty hard to say.

In any case, the main issue of this bug is the tendency of the cascade's implicit block to hide missing do syntax errors for call arguments. Is that implicit cascade going away in favor of #172? If so, this bug is dependent on that, leaving these other items:

  1. pipe clobbers the real _ variable
  2. Should the pipe be able to take an implicit block on the right so |> do is shortened to |>? I still think it's useful when the cascadee shouldn't be the result, and |> is shorter than with.

Do you want separate issues for these?

vendethiel commented 11 years ago

Was & really the best symbol ? I thought about *, which has sense too. Else, well', there's still % or ^ or § or ...

satyr commented 11 years ago

& & &

Only syntactically complicated. this/@ is semantically complicated in plain JS already, so we don't want to mess with them further.

implicit cascade going away in favor of #172?

Nope, as this proposal is resolved.

Do you want separate issues for these?

Yup, as they surely are separate.

I thought about *, which has sense too

Overused already.

there's still % or ^ or § or ...

%: Viable, but no better than & which I preferred. ^: Taken. §: Hard to type.