This should allow for much faster execution, stability, interuptability and forwards-compatible-parsing.
I just had a peek at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/transformiix/source/xslt/txInstructions.cpp&rev=220.127.116.11&root=/cvsroot and I don't like aEs.pushString, pushInt and so. Please make the state a collection of information, with method names indicating the purpose of the method. Not a collection of raw data that might or might not regain it's original meaning.
Not sure what you mean.. The execution-state will work as a general-purpose stack where you can push a bunch of datatypes that the instructions can use for whatever they want. What else do you want to call them? The reason there there are different pushes for different datatypes instead of a generel-purpose push(void*) is so that in case of error the execution-state can do proper cleanup.
I hate "general purpose" stack. That's junk data, and pretty useless (or even confusing) for debugging. Stuff like the qname is state information (if it is, I'm not convinced), which should be stored as that. I don't want to delve into each call into the state to find out what data should be what. Please do some real analysis on what information belongs into the state, and which information belongs into the action, and keep that information accessible in the state. If you need a stack of xsl:element QNames, then make a stack of that. Don't mix it with other data. (We should decide whether giving the QName for the endtag to the handler is real information, DEBUG only information or no information at all.)
I don't see that having explicit calls for each "thing" we push is going to help. Why have pushNamespace(String), pushLocalName(String) and pushStringHandlerResult(String) that all do the exact same thing? Yes, things will be more complex then in the current code, but i don't see how we can do interuptability and still have things as safe and clean as they are now.
on the end-tag note: I would like to remove the namespace and nodename arguments from the endElement call so that </LRE> and </xsl:element> only needs to call |aEs.mResultHandler->endElement()|. Module-endElement doesn't need the that information at all and standalone could keep a stack in the output-handler
err... forgot to say (still on the end-tag issue), that i want to do the result-handler change as a separate patch. This could probably be done together with atomizing the result-handler.
Depends on: 187610
Depends on: 104346
when porting xsl:message over to instructions, please drop the special handling for standalone. Both module and standalone can and should use the console service. standalone drivers may choose to register a nsIConsoleListener. Getting the console service could be part of the startup routines, too, having to get a service for each run may turn out bad.
virtual type txToplevelItem::getType() = 0; shouldn't be virtual, but txToplevelItem should hold a member (static?) and an inline non-virtual should just return that. This is one of the classic examples alecf ranted about, IIRC. A virtual function that doesn't really need to be virtual. Not sure if subclasses can overload the value of a static member var of its base class.
putting a member in there would increase the size of all toplevel items so i would rather keep it as is. Putting a _static_ member in there would make all objects have the same type.
I'd hate to see TxObject survive in all this new code. On first glimpse, you don't use old tx datastructures no more, so TxObject should die. txStylesheetCompilerState shouldn't hold the parse methods. As far as parsing expressions and patterns is concerned, it's the context, not the parser. They're no-brainers anyway. I'd say that our macros begin with TX, not TRANSFRMX. Even more for header files that don't have transfrmx in their name :-).
txExecutionState::[push|pop][Int|String] should merge into pushQName and popQName. That's there only use, so do that. Working on txExpandedName. Or, as the handler ain't atomized yet, on something else. That function should have a /* deprecated */, too, as the endElement handler should someday not require the element name anymore. there should be more NS_STATIC_CASTs instead of simple C casts.
hey, changes to xpcom have to go through dougt, and I'm not sure he would have been pleased with the nsError.h stuff - we're not in the habit of putting new modules into XPCOM...
Alec: so how would one go about having module-specific errors then? Imo we even want our own JS exceptions for these someday, in which case we need a XSLT XPCOM error module anyway.
the mechanism just isn't really pluggable right now. I guess I'm just applying the what-if-everyone-... principle. not every module should be notifying XPCOM of its existance.
Well, it does seem like this is the only way until XPCOM actually hands us a way to do this dynamically. The don't-blame-everyone-for-the-shortcomings-of-XPCOM principle ;-).
And it's not likly that we'll run out of errormodules anytime soon. The forumla allows for 32722 errormodules, which given the current rate of 27 in 5 years will make us run out in year 6059 :-)
sorry, make that year 8057 :-)
Created attachment 118509 [details] Final patch This includes everything except the change to nsError.h. Had to zip it since bugzilla didn't like a 647kB attachment :-)
patch landed with the following modifications: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=sicking%25bigfoot.com&whotype=match&sortby=Date&hours=2&date=explicit&mindate=03%2F25%2F2003+17%3A32&maxdate=03%2F25%2F2003+19%3A18&cvsroot=%2Fcvsroot
This fix: nsAString* base = baseURISet ? &baseURI : &mBaseURI; should have been const nsAString& = baseURISet ? baseURI : mBaseURI; IMO.
i don't believe that compiles xp
Then add some casts until it compiles. *Sigh*.
No longer depends on: 104346
it's not a casting problem.
let's live with the nsAString* and close this out.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.