OCamlPro / ocp-indent

Indentation tool for OCaml, to be used from editors like Emacs and Vim.
http://www.typerex.org/ocp-indent.html
Other
200 stars 63 forks source link

Directives are not parsed well (at least in the nlfork) #314

Closed baloone closed 2 years ago

baloone commented 3 years ago

The following line does not cover all cases https://github.com/OCamlPro/ocp-indent/blob/ddba3237aebaf938dc913b8aed709f6ee1854d5d/src/approx_lexer.mll#L259 For instance this whole line

#warnings "+a";; let a = 0;;

is parsed as directive

AltGr commented 3 years ago

There is a confusion between "line directives", used to refer to the original source after preprocessing, and toplevel directives; the latter are actually only handled at the parsing stage, and composed of multiple lexer tokens: see the entrypoint which is different from the one for .ml files. Ocp-indent has no specific code to handle it, it's just separated by the ;;

AltGr commented 3 years ago

Ah, but there is indeed an issue on the nlfork branch where the confusion apparently came from: the lexing of line directives was wrongly simplified from:

  | "#" [' ' '\t']* (['0'-'9']+ as _num) [' ' '\t']*
    ("\"" ([^ '\010' '\013' '"' ] * as _name) "\"")?
      [^ '\010' '\013'] * newline
      { update_loc lexbuf None 1 false 0;
        LINE_DIRECTIVE
      }

to

  | "#" ([^ '\010' '\013'] * as directive) newline
      { let st = update_loc st lexbuf 1 0 in
        ({st with stack = Newline :: st.stack}, LINE_DIRECTIVE directive)
      }

which would indeed gulp directives. (this is in approx_lexer.mll)

AltGr commented 3 years ago

@baloone I encourage you to try a fix (by reverting the pattern to the original one) and testing try-ocaml with that. Basically ocp-indent just ignores line directives because they make no sense for its use, which is why the original token has no arguments.