Collect errors/warnings from compile:forms/2 properly.

Reverted the out_dir -> outdir option rename.
Move compiler_options more into the background as an advanced option.

README.markdown

@@ -51,7 +51,7 @@ Result:
     
 Options is a proplist possibly containing:
 
-* `outdir` - Directory to store generated .beam files. If not
+* `out_dir` - Directory to store generated .beam files. If not
   specified, no .beam files will be created.
 
 * `doc_root` - Included template paths will be relative to this
@@ -95,12 +95,6 @@ Options is a proplist possibly containing:
   and returns a binary with the file contents. Defaults to `{file,
   read_file}`. Useful for reading templates from a network resource.
 
-* `compiler_options` - Proplist with extra options passed directly to
-  `compiler:forms/2`. This option can be supplied multiple times. Note
-  that the most common options can be given directly with the list of
-  options to the erlydtl compiler (see options affecting the Erlang
-  compiler, below).
-
 * `force_recompile` - Recompile the module even if the source's
   checksum has not changed. Useful for debugging.
 
@@ -139,26 +133,28 @@ Options is a proplist possibly containing:
   similar use, except that this option does NOT affect whether or not
   a .beam file is saved.
 
-*The following options also affect the Erlang Compiler*
-
-The following options are forwarded to `compile:forms/2`, (in
-addition to those given with the `compiler_options`):
-
 * `return` - Short form for both `return_warnings` and `return_errors`.
+
 * `return_warnings` - If this flag is set, then an extra field
   containing warnings is added to the tuple returned on success.
+
 * `return_errors` - If this flag is set, then an error-tuple with two
   extra fields containing errors and warnings is returned when there
   are errors.
+
 * `report` - Short form for both `report_warnings` and `report_errors`.
+
 * `report_warnings` - Print warnings as they occur.
+
 * `report_errors` - Print errors as they occur.
+
 * `warnings_as_errors` - Treat warnings as errors.
+
 * `verbose` - Enable verbose printing of compilation results.
 
-See
-[Erlang compiler documentation](http://www.erlang.org/doc/man/compile.html#forms-2)
-for documentation of these options with regard to the beam compiler.
+* `compiler_options` - Proplist with extra options passed directly to
+  `compiler:forms/2`. This can prove useful when using extensions to
+  add extra defines etc when compiling the generated code.
 
 
 Helper compilation

include/erlydtl_ext.hrl

@@ -20,7 +20,7 @@
           custom_tags_modules = [],
           reader = {file, read_file},
           module = undefined,
-          compiler_options = [verbose, report_errors],
+          compiler_options = [],
           binary_strings = true,
           force_recompile = false,
           locale = none,

src/erlydtl_compiler.erl

@@ -59,16 +59,18 @@
 -include("erlydtl_ext.hrl").
 
 compile(FileOrBinary, Module) ->
-    compile(FileOrBinary, Module, []).
+    compile(FileOrBinary, Module, [verbose, report_errors]).
 
 compile(Binary, Module, Options0) when is_binary(Binary) ->
-    Options = process_opts("/<text>", Options0),
-    Context = init_dtl_context("<text>", Module, Options),
+    File = "/<text>",
+    Options = process_opts(File, Options0),
+    Context = init_dtl_context(File, Module, Options),
     compile(Context#dtl_context{ bin = Binary });
 
 compile(File, Module, Options0) ->
     Options = process_opts(File, Options0),
     Context = init_dtl_context(File, Module, Options),
+    print("Compile template: ~s~n", [File], Context),
     compile(Context).
 
 compile_dir(Dir, Module) ->
@@ -110,7 +112,7 @@ compile_dir(Dir, Module, Options0) ->
 parse(Data) ->
     parse(Data, #dtl_context{}).
 
-format_error(no_outdir) ->
+format_error(no_out_dir) ->
     "Compiled template not saved (need outdir option)";
 format_error(unexpected_extends_tag) ->
     "The extends tag must be at the very top of the template";
@@ -128,6 +130,8 @@ format_error({write_file, Error}) ->
     io_lib:format(
       "Failed to write file: ~s",
       [file:format_error(Error)]);
+format_error(compile_beam) ->
+    "Failed to compile template to .beam file";
 format_error(Other) ->
     io_lib:format("## Error description for ~p not implemented.", [Other]).
 
@@ -139,7 +143,7 @@ format_error(Other) ->
 process_opts(Source, Options) ->
     Options1 = proplists:normalize(
                  update_defaults(Options),
-                 [{aliases, [{out_dir, outdir}]}
+                 [{aliases, [{outdir, out_dir}]}
                  ]),
     [{compiler_options, [{source, Source}]}
      |compiler_opts(Options1, [])].
@@ -153,8 +157,7 @@ compiler_opts([CompilerOption|Os], Acc)
       CompilerOption =:= report_warnings;
       CompilerOption =:= report_errors;
       CompilerOption =:= warnings_as_errors;
-      CompilerOption =:= verbose;
-      element(1, CompilerOption) =:= outdir ->
+      CompilerOption =:= verbose ->
     compiler_opts(Os, [CompilerOption, {compiler_options, [CompilerOption]}|Acc]);
 compiler_opts([O|Os], Acc) ->
     compiler_opts(Os, [O|Acc]);
@@ -162,13 +165,7 @@ compiler_opts([], Acc) ->
     lists:reverse(Acc).
 
 update_defaults(Options) ->
-    Options1 = maybe_add_env_default_opts(Options),
-    case proplists:get_value(compiler_options, Options1) of
-        undefined ->
-            [{compiler_options, (#dtl_context{})#dtl_context.compiler_options}
-             |Options1];
-        _ -> Options1
-    end.
+    maybe_add_env_default_opts(Options).
 
 %% shamelessly borrowed from:
 %% https://github.com/erlang/otp/blob/21095e6830f37676dd29c33a590851ba2c76499b/\
@@ -318,7 +315,7 @@ compile_forms(Forms, Context) ->
                fun (_, _, C) ->
                        case Info of
                            [Ws] when length(Ws) > 0 ->
-                               add_warning({compile_beam, Ws}, C);
+                               add_warnings(Ws, C);
                            _ -> C
                        end
                end
@@ -326,13 +323,13 @@ compile_forms(Forms, Context) ->
         error ->
             add_error(compile_beam, Context);
         {error, Es, Ws} ->
-            add_error({compile_beam, Es, Ws}, Context)
+            add_warnings(Ws, add_errors(Es, Context))
     end.
 
 maybe_write(Module, Bin, Context) ->
-    case proplists:get_value(outdir, Context#dtl_context.all_options) of
+    case proplists:get_value(out_dir, Context#dtl_context.all_options) of
         undefined ->
-            add_warning(no_outdir, Context);
+            add_warning(no_out_dir, Context);
         OutDir ->
             BeamFile = filename:join([OutDir, atom_to_list(Module) ++ ".beam"]),
             print("Template module: ~w -> ~s\n", [Module, BeamFile], Context),
@@ -1768,9 +1765,6 @@ custom_tags_modules_ast(Name, InterpretedArgs, #dtl_context{ custom_tags_modules
                                     Context#dtl_context{ custom_tags_modules = Rest })
     end.
 
-print(Fmt, Args, #dtl_context{ verbose = true }) -> io:format(Fmt, Args);
-print(_Fmt, _Args, _Context) -> ok.
-
 call_ast(Module, TreeWalkerAcc) ->
     call_ast(Module, erl_syntax:variable("_Variables"), #ast_info{}, TreeWalkerAcc).
 
@@ -1801,39 +1795,59 @@ call_ast(Module, Variable, AstInfo, TreeWalker) ->
     with_dependencies(Module:dependencies(), {{CallAst, AstInfo}, TreeWalker}).
 
 
-add_error(Error, #dtl_context{ errors=Es }=Context) ->
+print(Fmt, Args, #dtl_context{ verbose = true }) -> io:format(Fmt, Args);
+print(_Fmt, _Args, _Context) -> ok.
+
+
+add_error(Error, #dtl_context{
+                    errors=#error_info{ report=Report, list=Es }=Ei,
+                    parse_trail=[File|_] }=Context) ->
+    Item = get_error_item(Report, "", File, Error),
     Context#dtl_context{
-      errors=log_error_info(
-               "", error_info(Error),
-               Es, Context)
+      errors=Ei#error_info{ list=[Item|Es] }
      }.
 
+add_errors(Errors, Context) ->
+    lists:foldl(
+      fun (E, C) -> add_error(E, C) end,
+      Context, Errors).
+
 add_warning(Warning, #dtl_context{ warnings=warnings_as_errors }=Context) ->
     add_error(Warning, Context);
-add_warning(Warning, #dtl_context{ warnings=Ws }=Context) ->
+add_warning(Warning, #dtl_context{
+                        warnings=#error_info{ report=Report, list=Ws }=Wi,
+                        parse_trail=[File|_] }=Context) ->
+    Item = get_error_item(Report, "Warning: ", File, Warning),
     Context#dtl_context{
-      warnings=log_error_info(
-                 "Warning: ",
-                 error_info(Warning),
-                 Ws, Context) 
+      warnings=Wi#error_info{ list=[Item|Ws] }
      }.
 
-error_info({Line, ErrorDesc}) when is_integer(Line) ->
-    {Line, ?MODULE, ErrorDesc};
-error_info({Line, Module, _ErrorDesc}=ErrorInfo)
-  when is_integer(Line), is_atom(Module) -> ErrorInfo;
-error_info(ErrorDesc) -> {none, ?MODULE, ErrorDesc}.
-
-log_error_info(Prefix, {Line, Module, ErrorDesc}=ErrorInfo,
-               #error_info{ report=Report, list=L }=Ei,
-               #dtl_context{ parse_trail=[File|_] }) ->
-    if Report ->
+add_warnings(Warnings, Context) ->
+    lists:foldl(
+      fun (W, C) -> add_warning(W, C) end,
+      Context, Warnings).
+
+get_error_item(Report, Prefix, File, Error) ->
+    case Error of
+        {Line, ErrorDesc}
+          when is_integer(Line) ->
+            new_error_item(Report, Prefix, File, Line, ?MODULE, ErrorDesc);
+        {Line, Module, ErrorDesc}
+          when is_integer(Line), is_atom(Module) ->
+            new_error_item(Report, Prefix, File, Line, Module, ErrorDesc);
+        {_, InfoList} when is_list(InfoList) -> Error;
+        ErrorDesc ->
+            new_error_item(Report, Prefix, File, none, ?MODULE, ErrorDesc)
+    end.
+    
+new_error_item(Report, Prefix, File, Line, Module, ErrorDesc) ->
+    if Report  ->
             io:format("~s:~s~s~s~n",
                       [File, line_info(Line), Prefix,
                        Module:format_error(ErrorDesc)]);
        true -> nop
     end,
-    Ei#error_info{ list=[{File, ErrorInfo}|L] }.
+    {File, [{Line, Module, ErrorDesc}]}.
 
 line_info(none) -> " ";
 line_info(Line) when is_integer(Line) ->
@@ -1845,11 +1859,11 @@ pack_error_list(Es) ->
 collect_error_info([], [], Acc) ->
     lists:reverse(Acc);
 collect_error_info([{File, ErrorInfo}|Es], Rest, [{File, FEs}|Acc]) ->
-    collect_error_info(Es, Rest, [{File, [ErrorInfo|FEs]}|Acc]);
+    collect_error_info(Es, Rest, [{File, ErrorInfo ++ FEs}|Acc]);
 collect_error_info([E|Es], Rest, Acc) ->
     collect_error_info(Es, [E|Rest], Acc);
 collect_error_info([], Rest, Acc) ->
     case lists:reverse(Rest) of
-        [{File, ErrorInfo}|Es] ->
-            collect_error_info(Es, [], [{File, [ErrorInfo]}|Acc])
+        [E|Es] ->
+            collect_error_info(Es, [], [E|Acc])
     end.

tests/src/erlydtl_unittests.erl

@@ -1220,7 +1220,7 @@ tests() ->
          [{extension_module, erlydtl_extension_test}], <<"ok">>},
         {"proper error message", <<"{{ bar # }}">>, [{bar, "ok"}], [],
          [{extension_module, erlydtl_extension_test}],
-         {error, [{"<text>", [{1,erlydtl_extension_test,"Unexpected '#' in code at column 8"}]}], []}},
+         {error, [{"/<text>", [{1,erlydtl_extension_test,"Unexpected '#' in code at column 8"}]}], []}},
         %% accept identifiers as expressions (this is a dummy functionality to test the parser extensibility)
         {"identifiers as expressions", <<"{{ foo.bar or baz }}">>, [{baz, "ok"}], [],
          [{extension_module, erlydtl_extension_test}], <<"ok">>}
@@ -1231,6 +1231,8 @@ tests() ->
         [{record_info, [{testrec, record_info(fields, testrec)}]}],
         <<"Baz">>}
       ]}
+     %% {"error reporting",
+     %%  [{"return warnings"}]}
     ].
 
 run_tests() ->
@@ -1304,7 +1306,7 @@ format_error(Name, Class, Error) ->
 
 compile_test(DTL, Opts) ->
     Options = [force_recompile,
-               return_errors,
+               return_errors, return_warnings,
                {custom_filters_modules, [erlydtl_contrib_humanize]}
                |Opts],
     timer:tc(erlydtl, compile, [DTL, erlydtl_running_test, Options]).
@@ -1324,7 +1326,7 @@ process_unit_test({Name, DTL, Vars, RenderOpts, Output}) ->
     process_unit_test({Name, DTL, Vars, RenderOpts, [], Output});
 process_unit_test({Name, DTL, Vars, RenderOpts, CompilerOpts, Output}) ->
     case compile_test(DTL, CompilerOpts) of
-        {Tcompile, {ok, _}} ->
+        {Tcompile, {ok, _, _}} ->
             Tc = [{compile, Tcompile}],
             case render_test(Vars, RenderOpts) of
                 {TrenderL, {ok, IOList}} ->
@@ -1362,7 +1364,7 @@ process_unit_test({Name, DTL, Vars, RenderOpts, CompilerOpts, Output}) ->
                             test_pass([{render_binary, TrenderB}|TrL]);
                         {TrenderB, Err} ->
                             test_fail(Name, "Render error (with binary variables): ~p", [Err],
-                                     [{render_binary, TrenderB}|TrL])
+                                      [{render_binary, TrenderB}|TrL])
                     end;
                 {TrenderL, Output} ->
                     test_pass([{render_list, TrenderL}|Tc]);