Skip to content

Initial indent at start of a file #2981

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

Closed
xixixao opened this issue May 13, 2013 · 16 comments · Fixed by #3034
Closed

Initial indent at start of a file #2981

xixixao opened this issue May 13, 2013 · 16 comments · Fixed by #3034
Labels

Comments

@xixixao
Copy link
Contributor

xixixao commented May 13, 2013

Is this is a language/compiler feature?

  a
ignored
  also ignored

compiles down to

a;

Or an uncaught syntax error?

@vendethiel
Copy link
Collaborator

seems like we should be avoiding that

@epidemian
Copy link
Contributor

This is an awesome feature actually.

Want to comment a gigantic file? Just type TABnull at the start of the file and you're done!

/sarcasm

@xixixao
Copy link
Contributor Author

xixixao commented May 13, 2013

I had a go at fixing it, but where @Nami-Doc pointed to is not the place to fix it, since leading indent is needed for interpolations (even though that seems like a hack as well). It should be fixed wherever CoffeeScript actually ignores the negatively indented code (and throw error), which is beyond my knowledge of the compiler.

@vendethiel
Copy link
Collaborator

We can easily disable initial indent

@jashkenas any reason we allow that?

@epidemian
Copy link
Contributor

I think allowing initial indentation is quite handy in cases where you copy some code and want to, for example, paste it in coffeescript.org to see how it compiles. The copied code could have some level of indentation, but as long as that indentation is consistent, i don't see why the compiler should not allow it.

What should not be allowed is having an initial indentation in the first line of code and then having some other line going below that level of indentation (i'd expect and "unexpected DEDENT" error or something like that).

@vendethiel
Copy link
Collaborator

Ah, yeah, that's perfectly valid (saves so much time). We can keep track of "initial indent" in the lexer but we need to be sure we're not in an interpolation (we don't even flag the lexer yet), not in a string, not hacking with debts, etc.

It should be fixed wherever CoffeeScript actually ignores the negatively indented code (and throw error), which is beyond my knowledge of the compiler.

I'd say in the grammar. We have :

  • Root:
    • (empty)
    • Body
    • Block TERMINATOR
  • Body:
    • Line
    • Body TERMINATOR Line
    • Body TERMINATOR
  • Line:
    • Expression
    • Statement
  • Block:
    • INDENT OUTDENT
    • INDENT Body OUTDENT

So for ' 1\n2' will be lexed as :

[INDENT 1] [NUMBER 1] [OUTDENT 1] [TERMINATOR \n] [NUMBER 2] [TERMINATOR \n]

and Jison will give us these nodes :

Block
  Value "1"

Because Block isn't expecting a "side-block" (ie Block TERMINATOR Block)

Whereas if we have '1\n2', we'd get

[NUMBER 1] [TERMINATOR \n] [NUMBER 2] [TERMINATOR \n]

Generating the correct AST :

Block
  Value "1"
  Value "2"

@michaelficarra
Copy link
Collaborator

CoffeeScriptRedux handles this in the preprocessor with the notion of "base" indentation. Since the parser can't handle an indentation stack, the preprocessor adds the indent/dedent markers appropriately.

@vendethiel
Copy link
Collaborator

Would be easy if only we weren't creating new lexer instances, strange strings, newline suppression and some more. I tried to patch Lexer::lineToken to set @baseIndent if none was set and check size but sometimes I'd get really nasty error messages, like unexpected eval etc.

@michaelficarra
Copy link
Collaborator

So much statefulness!

@vendethiel
Copy link
Collaborator

Also, considering the order :

           @identifierToken() or
           @commentToken()    or
           @whitespaceToken() or
           @lineToken()       or

we'd need to set @baseIndent if not set in the 3 first, remove the or after @lineToken, set @baseIndent if not set, continue with our ors. Ugh. Coco allows it through the lexer (satyr/coco@4c034fe, test here).

@satyr
Copy link
Collaborator

satyr commented May 14, 2013

Coco seems to ignore it

I specially allowed

  a
b

case for some weird reasons regarding string interpolation.

If we're disallowing it, an easy way is to append an EOF to the token list and make Root require that.

@vendethiel
Copy link
Collaborator

Edited my post with the relevant commit

@xixixao
Copy link
Contributor Author

xixixao commented May 14, 2013

Checking for EOF seems to make the most sense to me (as grammars usually do).

@vendethiel
Copy link
Collaborator

But would not give a really good error message

@xixixao
Copy link
Contributor Author

xixixao commented May 14, 2013

exp.coffee:2:1: error: unexpected IDENTIFIER
ignored
^^^^^^^

It is not entirely clear ( unexpected negative indent ) but it also isn't useless.

For some reason though, it breaks tabs in tabbed code test in literate. Follow up: of course it breaks the tabbed code test, because that test is ignored in current version.

@xixixao
Copy link
Contributor Author

xixixao commented Jan 22, 2014

This was not (fully) fixed by #3034, but it is a duplicate of #2432 and it is caused by bad handling as monitored in #1275. Try on a command line.

@xixixao xixixao mentioned this issue Jan 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants