This is great stuff: Issues:
1) I like the idea of a unified place to parse lzoptions (getLzOption) as you have done in the server, and I think that works just right: if there is an lzoption parameter, then you only accept new-style options; otherwise you fall back to the old style. But I don't think it is right for getInitArg on the client to be looking into lzoptions. getInitArg ought to be _only_ for user code, for user parameters. I think you should add getLzOption to the client code and find all the places in the LFC that are looking for options and update them to use getLzOption. 1a) If the client uses `lzr` it should get updated to use `runtime` and you should include the translation as you do in the server when you have to fall back. Are there other names that should be cleaned up? Like `lzbacktrace` could become just `backtrace`? 2) I _disagree_ with Max's suggestion of using the shorter name `lz`, because that is more likely to collide with a user program query arg. I think we should use a longer name to reduce the possibility of a collision. 3) In the server code, you should only look for `runtime` in the new options and only look for `lzr` in the old params. I think the code is a little confused there. 4) In embednew, ca. line 605, where it is deciding what lps vars to copy to the query string, this is the whole point of lzoptions. It should just copy all the lzoptions into the query string. That way, you can add new options and never have to edit this code again. [I have to admit, I can't really understand the difference between flashvars, query, options, and initargs, but lzoptions should simplify this code, not make it more complex.] Questions: 1) swf/LZDebug looks specifically for the new options. It should use the common getLzOptions call. 2) Is it really worthwhile to check flex version? Maybe there should just be a simple passthrough --additionalFlexCompilerArguments or something. Then we don't have to adjust the compiler each time a customer comes up with a need to tweak the flex compiler? 3) I'd really like it if we could "improve" the options names. I know this is more work, but rather than just move `lzr` from the query params to lzoptions, I'd like to also rename it to be more mnemonic, say `runtime`. Maybe we could just have a mapping table from new to old in the getLzOption method that knows what to look for in the query arg if there is no lzoptions specified? Nits: 1) `substring(i,i+1)` is more conventionally written `charAt(i)` 2) I like how you shared the flexversion string between sc/Compiler and compiler/CompilationEnvironment. I guess we should do that for all those shared flags. On 2010-08-20, at 18:34, Henry Minsky wrote: > svn updated, resolved conflict with stale file > > Change 20100817-hqm-8 by [email protected] on 2010-08-17 11:46:24 EDT > in /Users/hqm/openlaszlo/trunk-clean > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: add unified 'lzoptions' support > > New Features: > > Bugs Fixed: LPP-3479 > > Technical Reviewer: ptw > QA Reviewer: max > Doc Reviewer: (pending) > > Documentation: > > Release Notes: > > Overview: > > + Added "flexversion" compiler option to select between flash 10.0 and 10.1 > > + Added support for passing compiler options in unified "lzoptions" string > > + modified "lzc" command line interpreter to accept 'flexversion' arg > > Details: > > > WEB-INF/lps/lfc/kernel/swf/LzBrowserKernel.lzs > WEB-INF/lps/lfc/kernel/swf9/LzBrowserKernel.lzs > getInitArg() looks for arg in lzoptions value, if present, and prefer > that value over regular query args > > WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs > Added routine to parse lzoptions string > > WEB-INF/lps/lfc/debugger/platform/swf/LzDebug.as > WEB-INF/lps/lfc/debugger/platform/swf9/LzDebug.as > checks for the presence of 'lzconsoledebug' using getInitArg, so that > it can see if the value is in lzoptions > > WEB-INF/lps/config/lps.properties > add default value for flex version = 10.0 > > WEB-INF/lps/server/src/org/openlaszlo/utils/LZHttpUtils.java > for back compatibility treat "runtime" as a synonym for "lzr" in > lzoptions > > WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9External.java > pass "flex_version" option to flex compiler (targets 10.0 vs 10.1) > > WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java > Add "flexversion" constant > > WEB-INF/lps/server/src/org/openlaszlo/server/LPS.java > add method to return default value for flex version > > WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderCompile.java > use accessor LZHttpUtils.getLzOption to get arg values instead of > request.getParameter > > WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderAPP_CONSOLE.java > formats the lzoptions values into the XML data format used by the > dev-console.lzx app > > WEB-INF/lps/server/src/org/openlaszlo/compiler/CompilationEnvironment.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/Compiler.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/Main.java > WEB-INF/lps/server/src/org/openlaszlo/compiler/SWF9Writer.java > add support for 'flex version' arg > > > lps/includes/source/embednew.js > lps/includes/source/build.xml > add parser for lzoptions string > > lps/admin/dev-console.lzx > generate a URL string for the app being loaded that encodes args into > lzoptions format > > > + I did not modify "lzc" command line processor to accept the new lzoptions > format yet. It still requires the old style discrete flags. > > Tests: > smokecheck in swf10 > test/lfc/data in swf10 > > TLF bidi tests > test/tlf/text-test.lzx?lzr=swf10 > test/tlf/focus-input.lzx?lzr=swf10 > > fetching test/hello.lzx using lzoptions > http://127.0.0.1:8080/trunk-clean/test/hello.lzx?lzoptions=lzr(swf10),flexversion(10.1) > http://127.0.0.1:8080/trunk3/test/hello.lzx?lzoptions=runtime(swf8),debug > http://127.0.0.1:8080/trunk3/test/hello.lzx?lzoptions=runtime(dhtml),debug,lzbacktrace > > verify that dev-console displays correct button and checkbox settings for > the selected options > (e.g., runtime and debug/backtrace flags, remote console debug) > > confirm by inspection of as files in compiler apache temp build > directory 'lzswf9', that flex compiler is getting passed the arg > "-target-player=10.1": > /openlaszlo/lib/apache-tomcat-5.5.29/temp/lzswf9/Users/hqm/openlaszlo/trunk-clean/test/build/hello/build.sh > > > Files: > M WEB-INF/lps/lfc/kernel/swf/LzBrowserKernel.lzs > M WEB-INF/lps/lfc/kernel/swf9/LzBrowserKernel.lzs > M WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs > M WEB-INF/lps/lfc/debugger/platform/swf/LzDebug.as > M WEB-INF/lps/lfc/debugger/platform/swf9/LzDebug.as > M WEB-INF/lps/config/lps.properties > M WEB-INF/lps/server/src/org/openlaszlo/utils/LZHttpUtils.java > M WEB-INF/lps/server/src/org/openlaszlo/sc/SWF9External.java > M WEB-INF/lps/server/src/org/openlaszlo/sc/Compiler.java > M WEB-INF/lps/server/src/org/openlaszlo/server/LPS.java > M > WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderCompile.java > M > WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderAPP_CONSOLE.java > M > WEB-INF/lps/server/src/org/openlaszlo/servlets/responders/ResponderOBJECT.java > M > WEB-INF/lps/server/src/org/openlaszlo/compiler/CompilationEnvironment.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/Compiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/Main.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/SWF9Writer.java > M lps/includes/source/embednew.js > M lps/includes/source/build.xml > M lps/admin/dev-console.lzx.swf > M lps/admin/dev-console.lzx > M lps/admin/lps/includes/lfc/LFCdhtml.js > M lps/admin/dev-console.lzx.js > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100817-hqm-8.tar
