> On July 2, 2014, 10:14 a.m., Tilghman Lesher wrote: > > /trunk/res/ael/pval.c, line 3390 > > <https://reviewboard.asterisk.org/r/3698/diff/1/?file=61842#file61842line3390> > > > > There are implications to how the language compilation works that are > > satisfied by using MSet. Unless you're going to revisit how AEL works, I'd > > suggest keeping this as always running MSet in the future. > > Matt Jordan wrote: > Can you explain this a bit more? I'm not sure how MSet is required here. > In each of the cases here, it appears as if a single variable is being set, > which - from a cursory look - should be able to be satisfied using Set > instead of MSet. I could easily be missing something here however. > > Tilghman Lesher wrote: > Consider this line from configs/extensions.conf.sample: > > CONSOLE-AEL="Console/dsp"; > > This get translated to: > > MSet(CONSOLE-AEL=$["Console/dsp"]) > > The expression parser will evaluate a string as itself, and MSet will > remove the quotes. If the quotes are not removed, consider what would happen > when you tried to Dial that variable: > > Dial("Console/dsp") > > The Dial app will parse that and attempt to find a technology called > '"Console', which will fail. > > Now, let's suppose that you modified the AEL app to remove the quotes. > Sounds plausible, right? Except that you now need to consider what happens > if the first character in a string is a digit: > > MSet(foo=$[1234-office]) > > This would get a parse error, and the evaluation would come back to the > empty string, almost certainly not what the person writing AEL intended. So > fine, let's remove the expression parser, too: > > time_left = ${somevar} - ${someothervar} > > Now, this (other) expression will fail, whereas with the expression > parser, it would perform as needed. Even worse, consider that you just won't > know at compile time (without a LOT of work) whether a particular argument > (due to variable interpolation) might be interpreted as an arithmetic > expression or simply as a string. Bottom line, unless you're going to go > through the AEL code and figure out another way of parsing AEL expressions, > all of which have to happen at compile time, with loosely typed variables, > you're best to leave in MSet.
Thanks for the explanation. When looking at this I was considering whether or not what was passed was a multiple variable behavior, and hadn't considered the quoting aspect. Given that, it sounds like the right thing to do right now is: (1) Remove the deprecation warning from MSet. If we use something internally, we can't deprecate it. (2) Make AEL use MSet by default - Matt ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3698/#review12437 ----------------------------------------------------------- On July 1, 2014, 7:54 p.m., Matt Jordan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/3698/ > ----------------------------------------------------------- > > (Updated July 1, 2014, 7:54 p.m.) > > > Review request for Asterisk Developers. > > > Repository: Asterisk > > > Description > ------- > > Per the discussion on the asterisk-dev mailing list [1], this patch removes > the following: > > * cdr_sqlite > * chan_gtalk > * chan_jingle > * chan_h323 > * res_jabber > * app_saycountpl > * app_readfile > * app_dahdibarge > > It removes the following applications/functions: > > * WaitMusicOnHold > * SetMusicOnHold > * SIPCHANINFO > > And it removes the colon delimiter from the SIPPEER function. > > It also removes all compatibility options that were configurable from > asterisk.conf, as these all applied to compatibility with Asterisk 1.4 > systems. > > Corey pointed out a number of other deprecated applications/functions, and > those should get removed as well - but I wanted to get round #1 up and going > since the channel drivers in particular are a bit odious with the media > rework going on. > > [1] http://lists.digium.com/pipermail/asterisk-dev/2014-June/068363.html > > > Diffs > ----- > > /trunk/utils/conf2ael.c 417729 > /trunk/utils/ael_main.c 417729 > /trunk/res/res_musiconhold.c 417729 > /trunk/res/res_jabber.c 417729 > /trunk/res/res_agi.c 417729 > /trunk/res/ael/pval.c 417729 > /trunk/pbx/pbx_realtime.c 417729 > /trunk/main/pbx.c 417729 > /trunk/main/asterisk.c 417729 > /trunk/include/asterisk/options.h 417729 > /trunk/configs/jingle.conf.sample 417729 > /trunk/configs/jabber.conf.sample 417729 > /trunk/configs/gtalk.conf.sample 417729 > /trunk/configs/asterisk.conf.sample 417729 > /trunk/channels/h323/noexport.map 417729 > /trunk/channels/h323/compat_h323.cxx 417729 > /trunk/channels/h323/compat_h323.h 417729 > /trunk/channels/h323/cisco-h225.cxx 417729 > /trunk/channels/h323/cisco-h225.h 417729 > /trunk/channels/h323/cisco-h225.asn 417729 > /trunk/channels/h323/chan_h323.h 417729 > /trunk/channels/h323/caps_h323.cxx 417729 > /trunk/channels/h323/caps_h323.h 417729 > /trunk/channels/h323/ast_ptlib.h 417729 > /trunk/channels/h323/ast_h323.cxx 417729 > /trunk/channels/h323/ast_h323.h 417729 > /trunk/channels/h323/TODO 417729 > /trunk/channels/h323/README 417729 > /trunk/channels/h323/Makefile.in 417729 > /trunk/channels/h323/INSTALL.openh323 417729 > /trunk/channels/h323/ChangeLog 417729 > /trunk/channels/chan_sip.c 417729 > /trunk/channels/chan_jingle.c 417729 > /trunk/channels/chan_h323.c 417729 > /trunk/channels/chan_gtalk.c 417729 > /trunk/channels/Makefile 417729 > /trunk/apps/app_readfile.c 417729 > /trunk/apps/app_dahdibarge.c 417729 > /trunk/addons/app_saycountpl.c 417729 > /trunk/addons/Makefile 417729 > /trunk/UPGRADE.txt 417729 > /trunk/CHANGES 417729 > > Diff: https://reviewboard.asterisk.org/r/3698/diff/ > > > Testing > ------- > > Asterisk compiles without the various modules and loads correctly. > > > Thanks, > > Matt Jordan > >
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
