Submitted as r1896.
On Sep 21, 2006, at 1:02 PM, P T Withington wrote:
> [Resent: corrected Adam's email]
>
> Approved, since this is blocking people, but I have some questions/
> suggestions on StyleSheetCompiler:
>
> 1) You have several (TODO) comments which you seem to have already
> done. I highly encourage you to use our conventional notation to
> make these more accurate and easier to find:
>
> TODO: [2006-09-21 ben] (LPP-2638) Read stylesheet from file
>
> and to remove when the Jira bug is resolved. :)
OK, rearranged comments and removed most of them when they were
already done.
Filed one bug from this, LPP-2733, need to recompile app if any
referenced css files change.
>
> 2) Is there a reason you did not use the FileResolver that is used
> for other src and href attributes to resolve the href for stylesheets?
Lack of knowledge! In the next round of spiffing this, I shall.
>
> 3) Is it really an error to have an empty stylesheet? That would
> seem to be only worth a warning to me. Hm, I see you are catching
> all the style sheet errors and just logging them. That doesn't
> seem quite right either. I would think that you would really want
> to signal an error if your style sheet path or content is invalid.
I do signal an error if your css file is not found -- see test/style/
errors/cssfilenotfound.lzx
You're right, I should give an error if the content is not valid, and
I don't currently do this. Filed as LPP-2734.
>
> 4) You could fix "TODO: move this out out global scope [bshine
> 9.17.06]" by simply wrapping your script in a function that is
> immediately evaluated:
>
> mEnv.compileScript("(function () {" + script + "})()");
Done -- and all the tests still pass.
>
> 5) You'll want to look at ScriptCompiler.writeObject. Basically,
> this will turn a Java Map or List into (a string representation of)
> a Javascript Object or Array. This will save you a lot of string
> manipulation, let you accumulate your selectors and properties more
> naturally in Java and simplify your code.
Oh my. Given that I've already done the string manipulation, would
you recommend changing this code to use ScriptCompiler.writeObject,
or should I just keep it in mind for next time?
-ben
_______________________________________________
Laszlo-dev mailing list
[email protected]
http://www.openlaszlo.org/mailman/listinfo/laszlo-dev
