LGTM. Committed as r755. On Fri, Sep 5, 2008 at 1:24 PM, Sascha Haberling <[EMAIL PROTECTED]>wrote:
> Hi guys, > > sorry for the delay, I've been quite busy the last weeks. Find attached an > updated patch. What changed: > > - Gadget.java: > - Instead of one file, multiple file names can be specified. > - GadgetGenerator.java: > - Mutliple files are read and concatenated. > - If the generator fails to load a file it will break the build > process > > Let me know what you think. I will be checking this thread later tonight > (CET) if you happen to have any comments. > > Thanks! > -- Sascha > > > On Thu, Aug 14, 2008 at 4:31 PM, Miguel Méndez <[EMAIL PROTECTED]> wrote: > >> Okay, I see now. My original reaction was that this could be handled via >> a linker, but that would be more complicated than this approach -- I was >> wrong. >> Gadget.java >> 227 - I'd go ahead and have the annotation return an array of file names. >> It is not any more work than what you already have and it avoids having to >> explain concatenate, etc. Also, the annotation needs to document the >> semantics of the file name in the same manner that ImageBundle does. >> >> On Thu, Aug 14, 2008 at 9:41 AM, Eric Ayers <[EMAIL PROTECTED]> wrote: >> >>> Attached. I looked for the reference to 'additional prefernces' in >>> javadoc but couldn't find it. >>> >>> On Thu, Aug 14, 2008 at 7:05 AM, Sascha Haberling <[EMAIL PROTECTED]> >>> wrote: >>> > Hi Miguel! >>> > >>> > The use case for this is to enable developers to add hand-written HTML >>> > content to the content section of the gadget spec. Just as every normal >>> GWT >>> > application has it's hand-written HTML host page a gadget developer >>> should >>> > be able to define hand-written HTML. Often it doesn't make sense to >>> build >>> > the whole GUI dynamically. A second advantage is that this can improve >>> > (perceived) latency of gadgets as the HTML is rendered before the >>> selection >>> > script is run. >>> > The additional content gets added at the beginning of the Content >>> element's >>> > CDATA section, right before the selection script. This is not supposed >>> to be >>> > used for changing the manifest file for adding prefs etc. >>> > Thanks for pointing String.isEmpty() issue out, I will modify this to >>> be >>> > compliant with 1.5. >>> > >>> > Eric: Where can I find the current patch that includes the changes you >>> made? >>> > >>> > -- Sascha >>> > >>> > On Thu, Aug 14, 2008 at 4:39 AM, Miguel Méndez <[EMAIL PROTECTED]> >>> wrote: >>> >> >>> >> Sorry to slow it down here. Maybe we should clarify a couple of >>> issues >>> >> before we go further. >>> >> >>> >> What's the use case for this? Are you injecting additional content or >>> >> preferences? The javadoc refers to prefs, but the annotation name >>> refers to >>> >> content. Also, it seems strange that we would add the capability to >>> inject >>> >> preferences that are not part of the supported gadget spec. >>> >> Does the additional content get added before the manifest's Content >>> >> element or inside of the Content element's CDATA section? >>> >> >>> >> BTW, we require Java 1.5 as the minimum. Please >>> >> update GadgetGenerator.java line 321 to use String.length == 0 instead >>> of >>> >> String.isEmpty; isEmpty is 1.6. >>> >> On Wed, Aug 13, 2008 at 2:35 PM, Eric Ayers <[EMAIL PROTECTED]> >>> wrote: >>> >>> >>> >>> Hi Sascha, >>> >>> >>> >>> LGTM, with some minor issues I fixed. Miguel & I were talking, and >>> he >>> >>> doesn't want to give up yet on allowing multiple files to be >>> included. >>> >>> I'll give him a chance to comment before committing. >>> >>> >>> >>> GadgetGenerator.java: >>> >>> sorted members & reformatted file w/ eclipse formatter. >>> >>> 349: javadoc comment out of date changed to: >>> >>> * @param filename The path filename of the file to inject. If the >>> name >>> >>> does >>> >>> * not start with a '/', it is assumed to be relative to >>> the >>> >>> same >>> >>> * package as the gadgetClass. >>> >>> >>> >>> On Mon, Aug 11, 2008 at 9:18 AM, Sascha Haberling < >>> [EMAIL PROTECTED]> >>> >>> wrote: >>> >>> > Thanks for your comments, Eric! >>> >>> > >>> >>> > In addition to the inline comments: I just realized that the >>> >>> > lookup-behavior >>> >>> > was different from ImageBundle. I modified it so the file is >>> searched >>> >>> > for in >>> >>> > the same directory as the Gadget class if there is no '/' at the >>> >>> > beginning. >>> >>> > If there is one, then it is searched for in the classpath. >>> >>> > >>> >>> > See attached for an updated patch file. >>> >>> > >>> >>> > >>> >>> > On Mon, Aug 11, 2008 at 2:34 PM, Eric Ayers <[EMAIL PROTECTED]> >>> wrote: >>> >>> >> >>> >>> >> [+gwtc] >>> >>> >> >>> >>> >> GadgetGenerator.java: >>> >>> >> 326: Variable name 'additionalPrefs' looks like a holdover from >>> old >>> >>> >> annotation name. rename to something more appropriate. >>> >>> > >>> >>> > Done. >>> >>> > >>> >>> >> >>> >>> >> 377: I don't think the removal of newlines is going to work out, >>> even >>> >>> >> though I'm the one that suggested it Consider HTML content - >>> removing >>> >>> >> the >>> >>> >> newline in the middle of some plain text or <pre> text would >>> change >>> >>> >> the way >>> >>> >> the application rendered. >>> >>> > >>> >>> > This is true. I also don't think this saves a lot. If we know that >>> the >>> >>> > injected file is HTML we could do a lot more like getting rid of >>> all >>> >>> > the >>> >>> > white-spaces. I don't know what would be the best way to go here. I >>> put >>> >>> > the >>> >>> > new-line back in until we have a better idea. >>> >>> > >>> >>> >> >>> >>> >> Gadget.java >>> >>> >> 218: Javadoc comment is out of date >>> >>> > >>> >>> > Done. >>> >>> > >>> >>> >> >>> >>> >> 224: Is the name of the file really optional on this annotation? >>> >>> > >>> >>> > Not anymore, you are right. This was also a remnant of the >>> >>> > AdditionalPrefs >>> >>> > annotation. Should the process stop here or is it better he way it >>> is >>> >>> > right >>> >>> > now: It will spit out an error in the log but continue without >>> >>> > injecting >>> >>> > anything. >>> >>> > >>> >>> >> >>> >>> >> >>> >>> >> On Wed, Aug 6, 2008 at 2:22 PM, Sascha Haberling >>> >>> >> <[EMAIL PROTECTED]> >>> >>> >> wrote: >>> >>> >>> >>> >>> >>> Hi guys, >>> >>> >>> >>> >>> >>> attached you find a patch for review and discussion for a >>> >>> >>> GALGWT-Gadget >>> >>> >>> feature that lets programmers specify a file that should be >>> injected >>> >>> >>> into >>> >>> >>> the gadget manifest. This is the result after having some >>> discussion >>> >>> >>> with >>> >>> >>> Eric and Bob about this recently. >>> >>> >>> >>> >>> >>> This lets users add an annotation "InjectContent" to their Gadget >>> >>> >>> subclass which has a member "file". The file name specified here >>> will >>> >>> >>> be >>> >>> >>> searched for in the classpath. If found the file's content will >>> be >>> >>> >>> injected >>> >>> >>> into the gadget manifest, right in front of the selecton script. >>> >>> >>> >>> >>> >>> Here are a few points for discussion: >>> >>> >>> >>> >>> >>> Bob mentioned validation: Should the content of that file be >>> checked >>> >>> >>> so >>> >>> >>> that it doesn't mess with the validity of the whole XML file? Or >>> >>> >>> should just >>> >>> >>> the final XML file be checked? I am not sure how we could check >>> the >>> >>> >>> former >>> >>> >>> case as we might need an HTML parser for that? >>> >>> >>> Is it enough to have only one file injected or should it be >>> possible >>> >>> >>> to >>> >>> >>> define a list of files? I think one file is probably enough as >>> the >>> >>> >>> content >>> >>> >>> is hand-written and static anyway. >>> >>> >>> Compression: Right now I remove the new lines from the injected >>> file. >>> >>> >>> But >>> >>> >>> if we consider this to be valid HTML, there is more that we could >>> do >>> >>> >>> like >>> >>> >>> removing some white-spaces. Not sure how to best to this. >>> >>> >>> >>> >>> >>> Any feedback appreciated. >>> >>> >>> -- Sascha >>> >>> >> >>> >>> >> >>> >>> >> >>> >>> >> -- >>> >>> >> Eric Z. Ayers - GWT Team - Atlanta, GA USA >>> >>> >> http://code.google.com/webtoolkit/ >>> >>> > >>> >>> > >>> >>> >>> >>> >>> >>> >>> >>> -- >>> >>> Eric Z. Ayers - GWT Team - Atlanta, GA USA >>> >>> http://code.google.com/webtoolkit/ >>> >> >>> >> >>> >> >>> >> -- >>> >> Miguel >>> > >>> > >>> >>> >>> >>> -- >>> Eric Z. Ayers - GWT Team - Atlanta, GA USA >>> http://code.google.com/webtoolkit/ >>> >> >> >> >> -- >> Miguel >> > > -- Eric Z. Ayers - GWT Team - Atlanta, GA USA http://code.google.com/webtoolkit/ --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
