Erratic behavior of partial outdents

[avakar]

I've been trying to figure out how indentation works in Coffeescript and I came across this snippet.

if a
  if c
        d
  g
  e

Adding spaces before g alternates the result between

if (a) {
  if (c) {
    d;
  }
  g;
  e;
}

and

if (a) {
  if (c) {
    d;
  }
  g;
}

e;

Looking at the lexer, it seems that the outdebt caused by overindented g is being compared to the indent between if a and if c lines. That's surely unintended -- shouldn't the e line simply remove the outstanding outdebt introduced by g? I.e. if I understand the intent correctly, g and e should be considered to be at the same indent level regardless of the indent of g (as long as it's smaller than d).

[avakar]

Actually, this might be related to #3625 .

[vendethiel]

No, it's not a duplicate of this one. Our lexer is too permissive, and accepts any amount of indent as one indent. That's what you're seeing here. I'm on my phone so I can't look for dups right now, though.

[avakar]

I'm not sure if you're saying that the behavior is intended, I'm pretty sure it's not. To emphasize, adding spaces alternates between the two outputs. As in the output goes back and forth even though g stays at an indent level between d and e.

Here's a graphic (try):

if a
  if c
        d
#-------- <- OUTDENT
   g
#-------- <- OUTDENT
  e
#--------

Add another space and the position of the second outdent changes (try).

if a
  if c
        d
#-------- <- OUTDENT
    g
#--------
  e
#-------- <- OUTDENT

[vendethiel]

From #1275:

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'm pretty sure it's not

It's pretty much about "not breaking backwards compatibility" at this point

[avakar]

Guys, aren't you misreading the issues? Seriously, your quote is completely unrelated to this issue. It's actually not related to #1275 either. You quote

if the dedent doesn't quite make it out to the level of the previous indent, it counts as if it had.

and that's perfectly fine. The issue is that the quoted behavior is not implemented correctly and sometimes the outdent token is placed at an incorrect location. In #1275 the OP also noticed that the behavior is different with odd vs even number of spaces and none of the collaborators addressed that. Looking at the code it's actually worse than that because the placement of the outdent token depends on the indent of the enclosing block, in this case on the indentation difference between if a an if c. In #1275 the guy got lucky because he used 2 spaces for indent so the problem manifested itself as if it depended on the parity of the partial outdent, but in reality the behavior is more complex than that.

So please, if you're ok with that behavior, could you say that explicitly? Right now it just seems that you're not reading the issue carefully enough.

[vendethiel]

No, I get what you mean. I don't think it has anything to do with being odd or even, I just think it's a @baseIndent special case in the lexer (the first indent that's encountered)... You can argue the issue shouldn't have been closed after the initial-indentation-bug PR, yes.

[avakar]

This has nothing to do with @baseindent, the issue occurs no matter how many levels of indentation deep you are. The problem is with this line, lastIndent is likely supposed to be moveOut (and the adjacent five lines need to be fixed accordingly). I think after a partial outdent, which makes @outdebt nonzero, you want to ignore all outdents until you deplete @outdebt. lastIndent is the part of the indent before the outdebt and should not be involved.

[vendethiel]

I'd argue this should have to – we should stick to @baseIndent. But – as the linked issue says – it's probably too late by now to change it. I think a PR that fixes that while keeping backwards compat would be very heloful.

[avakar]

I still don't understand how @baseIndent is involved.

[vendethiel]

it's the first indent the lexer encountered, and it should probably be enforced. Sadly, this'd probably break too much code that relies on a different behavior (or even people doing "unconventional" spacings with array/object literals, that happens a whole lot), so if your proposed fix works without breaking other tests, it'd be nice to have.