Summary:	ASTERISK-12333: [patch] new CLI commands for handling dialplan variables and name standartization for the old ones
Reporter:	Caio Begotti (caio1982)	Labels:
Date Opened:	2008-07-07 15:16:16	Date Closed:	2008-07-17 08:52:42
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/PBX
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) dialplan_globals4.diff ( 1) dialplan_globals5.diff ( 2) dialplan_globals6.diff
Description:	Some commands like 'core set global' should actually be called 'dialplan set global' as they deal with dialplan variables directly (they even state that in their help messages), so I decided to change them after commenting about it on #asterisk-dev. I'm not sure about the consequences of it since a deprecated warning should be printed, but I will let it to someone more experienced to decide. Since there are CLI commands to set new global and per channel variables I also decided to create equivalent ones to unset these variables as they'd stay in memory forever until a reload or restart is executed. Not good, now I can unset them :-) Ultimately there was not a way to list all those per channel variables set with 'dialplan set chanvar <channel> <name> <value>', except parsing the very long output of DumpChan(). I created a command called 'dialplan show chanvar <channel>', using the same idea from 'show globals' and dumping all variables associated with the given channel. ****** ADDITIONAL INFORMATION ****** 1. I'm pretty rookie on this, specially channel locking, some input on locking and thread-safe code would be just awesome :-) 2. I'd rather use 'dialplan show variables <channel>' and 'dialplan set variable <channel> <name> <value>' instead but I chose to let it be with the old naming already in use.
Comments:	By: Tilghman Lesher (tilghman) 2008-07-08 13:27:17 I would prefer if you did not provide a facility for unsetting variables.  There are some fairly nasty things that could happen, especially if you unset an argument variable (ARG1, ARG2, etc.) or a LOCAL() variable while a subroutine was executing. The way the dialplan currently handles this is to set variables to the empty string, instead, and I would prefer if all such user interfaces used the same convention. By: Michiel van Baak (mvanbaak) 2008-07-10 11:25:10 This patch renames the CLI functions. The old functions should be deprecated first, and be removed a release later. If you want, I can add this to your patch. By: Caio Begotti (caio1982) 2008-07-11 08:53:00 In my new patch I've added the deprecated commands mvanbaak, thanks for reminding me :-) Corydo76, the same issue of unset variables could happen if someone just use the standard set command to change its value to an empty string too. Isn't the same side effect but with an already existing command? It would possibly be correct to not let the set command set an empty value then. I can add it to my new patch before uploading it, do you think it's better? By: Tilghman Lesher (tilghman) 2008-07-11 09:22:57 caio1982:  Setting a variable to the empty string and setting a variable to NULL do two different things.  Please either eliminate the unset or make it set to the empty string. By: Caio Begotti (caio1982) 2008-07-11 12:14:10 Yes, I misunderstood it. Anything else to change or this new patch solves what you guys asked? By: Michiel van Baak (mvanbaak) 2008-07-12 05:27:34 Looks ok to me. If Corydon agrees we can put this in. By: Digium Subversion (svnbot) 2008-07-17 08:52:38 Repository: asterisk Revision: 131606 U   trunk/CHANGES U   trunk/UPGRADE.txt U   trunk/main/pbx.c ------------------------------------------------------------------------ r131606 | tilghman | 2008-07-17 08:52:33 -0500 (Thu, 17 Jul 2008) | 7 lines Change several 'core' commands to be 'dialplan' commands (with appropriate deprecation, of course) (closes issue ASTERISK-12333) Reported by: caio1982 Patches:       dialplan_globals6.diff uploaded by caio1982 (license 22) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=131606