Re: [Erlhive-devel] sgte in erlhive (continued)

[Filippo P. <pa...@sg...>]

Hi Ulf,
I can't access the web from my lapotop until monday. I'm on holidays right
now :-) and on a dialup connection.
However for now you can find my replies and some comments inline.

> Several of the modules use the file module. This shouldn't be an obstacle,
> as file.erl is almost fully supported in Erlhive (only a few dets-specific
> functions are not).
> 
> However, in Erlhive, calls to library modules such as file and lists must
> be dotted. Using .file and .lists etc actually works even in normal code,
> but it may offend some people, and is officially unsupported.
> 
> You can instead add this to the beginning of the module:
> 
> -ifdef(ERLHIVE).
> -import(.file).
> -import(.lists).
> -endif.
> 

I'm not sure I understand what you mean here.
Should I change sgte code with something like this?

-ifdef(ERLHIVE).
-import(.file, [read_file/1]).
-else
-import(file, [read_file/1]).
-endif.

And then in the code simply use read_file instead of file:read_file?

> Also, there are some calls to os:cmd(), which need some attention.
> For example, in sgte_gettext.erl:
> 
> gettext_init(TargetDir, SrcFiles, Domain) ->
>     %% Create target directory and parent directories
>     %% when missing.
>     os:cmd("mkdir -p " ++ TargetDir),
> 
> This call can be replaced with filelib:ensure_dir(TargetDir).

Ok I'll change this.

> 
> This one is trickier:
> 
> po_header() ->
>     Date = os:cmd("date +%F\ %T\ %z"),
> 
> The call to 'date' doesn't work on Solaris (at least not Solaris 8), since
it
> doesn't support the %F option. It would need to be rewritten in some
> portable, erlangy, way.
> 
> I have no problem supporting os:getenv("TZ") as part of the
solution, even
> though this isn't supported right now. Then, the call can be replaced by:
> 
> {{Y,Mo,D},{H,Mi,S}} = calendar:local_time(),
> lists:flatten(io_lib:fwrite("~4.4.0w-~2.2.0w-~2.2.0w
> ~2.2.0w:~2.2.0w:~2.2.0w ~s",
>                                  
[Y,Mo,D,H,Mi,S,os:getenv("TZ")])).
> 
> (or some, slightly more verbose, but more efficient, solution...)

This shouldn't be a big issue. 
The call to date is used to generate the header of a gettext template file.
I'm not a big expert of gettext, but the header should be basically a
comment for human readers. 
Removing the timezone should not affect gettext functionality. 
I'll check this better on monday.

I also have some patches to add (submitted by Sean Hinde). 
The compiled format of templates has changed from strings to binaries.
I also had several calls in the render to lists:flatten. With the changes
submitted by sean this are removed since Yaws already manages deep lists of
binaries and strings. This should improve performance a lot.

Deep list could break code for non yaws users, so I'll have to figure out a
way not to break things.

> 
> With those small changes, sgte should compile from within an erlhive
> user account. If erlhive were to support it "natively", sgte.erl
and
> sgte_gettext.erl
> would have to be transformed, using erlhive_xform.erl, and the other
modules
> could simply be allowed as they are.
> 
The changes to make it run in user space are not a problem for me. 
However IMVHO a template language is one of the fundamental pieces to
develop web applications. So if you want to support it "natively" I'd be
happy. 

cheers,
filippo

________________________________________________
Message sent using UebiMiau 2.7.9