[ic] [interchange] Fix for restrict_allow which was being clobbered by default values.

[Mike Heins]

Quoting Sam Batschelet (samb at endpoint.com):
> 
> On 05/06/2016 05:23 PM, Mike Heins wrote:
> > Quoting Jon Jensen (jon at endpoint.com):
> >> On Fri, 6 May 2016, Mike Heins wrote:
> >>
> >> It merges the two sets of restrict_allow values together.
> > 
> > Hmm. I don't understand the comment then, "was being clobbered by
> > default values". This more looks like a fix for the default values
> > getting clobbered by the $opt, which I would think is the intention
> > of the code.
> > 
> > In other words, if you set it in $opt it should blow away what's in
> > $record, as presumably your setting of the option should indicate that
> > you know what's best. .
> 
> Mike,
> My understanding is $opt->{restrict_allow} was defined by the
> o_default_defined hash or $CGI->{restrict_allow} values.

Maybe, and maybe not. If you didn't set anything in $opt->{restrict_allow},
sure. But if you wanted to disallow "area" or "var", you are now out
of luck.

> And as $opt
> was always defined, if you set restrict_allow via meta_editor,
> $record->{restrict_allow} would never be set by this line.
> 
> $opt->{restrict_allow} ||= $record->{restrict_allow};
> 
> So perhaps the word clobbered was dramatic, but you could not define
> $opt even if it were only the default values with $record.  As Jon said
> the patch takes the $opt values and merges them with $record.  This way
> you can maintain defaults as well as possible input options.  Certainly
> open to better way to handle this.  The commit message would read better
> like such?
> 
> Fix for $record->{restrict_allow} which was being clobbered by $opt values.

I do understand, but the problem is this -- you can never override 
$record->{restrict_allow} with $opt this way. And there is an easy way to
get what you want -- just add "area var" to any restrict_allow value you
set -- problem solved.

> 
> 
> abbreviated logic from how I read this.
> 
> my %o_default_defined = (
>       mv_update_empty         => 1,
>       restrict_allow          => 'page area var',
> );
> 
> sub editor {
>    resolve_options($opt);
>    display({restrict_allow => $opt->{restrict_allow}})
> }
> 
> sub resolve_options {
>     if ($opt->{cgi} && $CGI->{restrict_allow})
>         $opt->{restrict_allow} = $CGI->{restrict_allow}
>     }
> 
>     # defaults only set if no value exists
>     while( my ($k, $v) = each %o_default) {
>         $opt->{$k} ||= $v;
>     }
> }
> 
> sub display {
>   # $opt->{restrict_allow} is defined or default now.
>   # $record->{restrict_allow} from meta_editor
> 
>   # no dice for $record
>   $opt->{restrict_allow} ||= $record->{restrict_allow};
> }

Bottom line is that I don't see the benefit of having these 
shadow values preserved by this code -- if you are setting 
restrict_allow, you might as well add " area var " to it.

When I think about it, there is some question as to whether
you might get unexplained behavior because you don't understand
that "page area var" are not intrinsically allowed but are instead
allowed via a default setting.  But if you know enough to do the
setting, you should quickly be able to diagnose why your [var FOO] tag
doesn't get interpolated.

Remember that you can set $opt with the tag call, too -- it is
not always done via CGI. In the admin, especially.

-- 
Mike Heins
End Point -- Expert Internet Consulting    http://www.endpoint.com/
phone +1.765.253.4194  <mikeh at endpoint.com>

Experience is what allows you to recognize a mistake the second time you
make it. -- unknown