Skip to content
New issue

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

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

Already on GitHub? # to your account

Sub-block terminator can cause parent block termination #1275

Closed
ejones opened this issue Apr 10, 2011 · 10 comments · Fixed by #3320
Closed

Sub-block terminator can cause parent block termination #1275

ejones opened this issue Apr 10, 2011 · 10 comments · Fixed by #3320
Labels

Comments

@ejones
Copy link

ejones commented Apr 10, 2011

This occurs, apparently, when the line that terminates the sub-block expression is indented by exactly one space EDIT: an odd number of spaces...? from the initial indentation of the parent block, and the contents of the sub-block are indented past that, and the line following the terminator is at the original indentation:

# These don't work
->
  (
    a
   )
  b
->
  (
      a
     )
  b
###
(function() {
  return a;
});
b;
###

# These work correctly:
->
  (
    a
  )
  b
->
  (
     a
    )
  b
->
  (
    a
   )
   b
###
(function() {
  a;
  return b;
});
###
@satyr
Copy link
Collaborator

satyr commented Apr 10, 2011

exactly one space

Any amount of under-dedentation works actually:

$ coffee -bsp
->
  ->
      a
    b
  c

(function() {
  (function() {
    return a;
  });
  return b;
});
c;

Unlike Python, Coffee doesn't force {in,de}dent counts to match.

@michaelficarra
Copy link
Collaborator

Unlike Python, Coffee doesn't force {in,de}dent counts to match.

I don't really see how this behaviour is helpful. Its purpose has always confused me and I have never (intentionally) used it. Is it time for a change?

@jashkenas
Copy link
Owner

Right -- if the dedent doesn't quite make it out to the level of the previous indent, it counts as if it had. This is just to make it a little more forgiving -- but if the less strict syntax causes more problems than it solves, perhaps we should remove it.

I can guarantee we'll have a bunch of folks come out of the woodwork with freshly broken programs if we make that change, however.

@michaelficarra
Copy link
Collaborator

It would be something for which I'd be okay holding out until 2.0 so some backwards-incompatibilities are expected. It's not immediately problematic.

@ejones
Copy link
Author

ejones commented Apr 10, 2011

@satyr - as of the version on coffeescript.org and npm's coffee-script, your example actually gets processed correctly:

(function() {
  (function() {
    return a;
  });
  b;
  return c;
});

I did discover, though, that in these cases, its not specifically one but rather any odd number of spaces.

Although I acknowledge the non-matcing indent-dedent rule (and disagree with it), the parser's behaviour is nevertheless in these cases inconsistent, or seemingly so.

@autotelicum
Copy link

A warning of odd/insufficient spacing would be backwards compatible (cf. examples in closed issue #1512).

@dstepanov
Copy link

One year later.

I think this should be fixed as soon as possible. Backwards compatibility is important, but this is obviously a bug there are way too many duplicates (mine: #2462). How long will it take to 2.0?

@michaelficarra
Copy link
Collaborator

2 months or so

@aleator
Copy link

aleator commented Feb 27, 2013

Heh. It's probably faster for you guys to fix the issue than to keep hunting all the duplicates ;) Sorry about the last one, I thought it was some if specific thing..

jashkenas added a commit that referenced this issue Jan 22, 2014
@balupton
Copy link

I don't really see how this behaviour is helpful. Its purpose has always confused me and I have never (intentionally) used it. Is it time for a change?

+1 the stricter the compiler the better.

Out of all those examples, the only one I would expect to work is this one:

->
  (
    a
  )
  b

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants