k0kubun / hamlit

High Performance Haml Implementation
https://rubygems.org/gems/hamlit
Other
980 stars 60 forks source link

add full check option #188

Closed dlwr closed 2 years ago

dlwr commented 2 years ago

(This PR would be related to https://github.com/k0kubun/hamlit/issues/166) Hi, I faced problem that hamlit compile -c missing syntax error of haml like below.

$ cat invalid-haml-syntax.haml
%h1
  title
%div
 %span
$ haml -c invalid-haml-syntax.haml
Syntax error on line 4: Inconsistent indentation: 1 space used for indentation, but the rest of the document was indented using 2 spaces.
  Use --trace for backtrace.
$ bundle exec exe/hamlit compile -c invalid-haml-syntax.haml
Syntax OK

So, I tried to solve this problem by adding full_check option -C to check haml syntax in this PR.

$ bundle exec exe/hamlit compile -C invalid-haml-syntax.haml
Inconsistent indentation: 1 space used for indentation, but the rest of the document was indented using 2 spaces.
$ echo $?
1

I would be greateful to hear about any ideas or suggestions.

k0kubun commented 2 years ago

Why don't you use hamlit compile invalid-haml-syntax.html.haml | ruby -c as explained in the issue?

dlwr commented 2 years ago

Beacuse hamlit compile invalid-haml-syntax.html.haml generates valid ruby code and it can't check haml syntax.

bundle exec exe/hamlit compile invalid-haml-syntax.haml
_buf = [];
;
;
; raise Hamlit::HamlSyntaxError.new(%q[Inconsistent indentation: 1 space used for indentation, but the rest of the document was indented using 2 spaces.], 3); _buf = _buf.join("".freeze)
$ bundle exec exe/hamlit compile invalid-haml-syntax.haml | ruby -c
Syntax OK
k0kubun commented 2 years ago

Oh, you're right. But just eval-ing the compiled code (without BEGIN {return nil}; prepended) doesn't sound like a good idea. It's not what haml -c does, either.

The root cause is here https://github.com/k0kubun/hamlit/blob/8ef1d85c9a55a3866dbc47169dc4df35d40dd9d9/lib/hamlit/parser.rb#L37-L38 Could you consider updating your patch with the following direction?

  1. Define a new option on Hamlit::Engine
  2. Pass the option when hamlit compile -c is used
  3. When the option is set, raise the error in Hamlit::Parser instead of returning the exception.
dlwr commented 2 years ago

Let's continue with direction of https://github.com/k0kubun/hamlit/pull/188#issuecomment-1028590147 in https://github.com/k0kubun/hamlit/pull/189