Mailing List Archive: Interchange: users

One-line patch for more_link_template

Index | Next | Previous | View Threaded

carl at endpoint

Oct 13, 2009, 5:40 PM

Post #1 of 7 (1187 views)
Permalink

One-line patch for more_link_template

IC-Users,

Here's a one-line patch for sub more_link_template (Interpolate.pm),
that prevents the links from having empty/useless 'mv_arg=' added to
the link by tag_area().

http://github.com/carlbailey/interchange/commit/b4b6f8b42d93ed77842d81c11a16fcbbffb77180

If you're like me you always shy away from looking at the nastiness
that are more-link URL's, but a recent client problem forced the
issue, and there I was. More discussion in the commit message. Your
comments welcome, as always.

Carl
. . . . . . . . . . . . . . . . . . .
Carl Bailey
t: 919-323-8025
carl [at] endpoint
. . . . . . . . . . . . . . . . . . .

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

peter at pajamian

Oct 13, 2009, 9:45 PM

Post #2 of 7 (1113 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On 10/13/2009 05:40 PM, Carl Bailey wrote:
> IC-Users,
>
> Here's a one-line patch for sub more_link_template (Interpolate.pm),
> that prevents the links from having empty/useless 'mv_arg=' added to
> the link by tag_area().
>
> http://github.com/carlbailey/interchange/commit/b4b6f8b42d93ed77842d81c11a16fcbbffb77180
>
> If you're like me you always shy away from looking at the nastiness
> that are more-link URL's, but a recent client problem forced the
> issue, and there I was. More discussion in the commit message. Your
> comments welcome, as always.

From the patch:
+ my $url = tag_area("scan/MM=$arg", undef, {
form => $form_arg,
secure => $CGI::secure,
});

... a better version:
my $url = tag_area(undef, undef, {
search => "MM=$arg",
form => $form_arg,
match_security => 1,
});

Peter

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

jon at endpoint

Oct 17, 2009, 2:12 PM

Post #3 of 7 (1117 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On Tue, 13 Oct 2009, Peter wrote:

> ... a better version:
> my $url = tag_area(undef, undef, {
> search => "MM=$arg",
> form => $form_arg,
> match_security => 1,
> });

Carl,

Were you able to try this and did you have any comment? It would be nice
to avoid hardcoding the "scan" URL part, I guess.

Jon

--
Jon Jensen
End Point Corporation
http://www.endpoint.com/

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

carl at endpoint

Oct 18, 2009, 8:28 AM

Post #4 of 7 (1080 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On Oct 17, 2009, at 5:12 PM, Jon Jensen wrote:

> On Tue, 13 Oct 2009, Peter wrote:
>
>> ... a better version:
>> my $url = tag_area(undef, undef, {
>> search => "MM=$arg",
>> form => $form_arg,
>> match_security => 1,
>> });
>
> Carl,
>
> Were you able to try this and did you have any comment? It would be
> nice
> to avoid hardcoding the "scan" URL part, I guess.
>
> Jon

I've been looking at this for a while now, trying to understand why
the above would be better. From what I can tell, there are three ways
to call tag_area() in this case:

1) tag_area('scan', "MM=$mv_arg", { form => $form_arg, match_security
=> 1, }
2) tag_area(undef, undef, { search => "MM=$arg", form => $form_arg,
match_security => 1, }
3) tag_area("scan/MM=$mv_arg", undef, { form => $form_arg,
match_security => 1, }

All three calls return the identical URL.

Options 1 and 2 both wind up making a call to escape_scan() to compose
the URL. That's necessary in tag_area(), when the routine can not
know what search variables will be coming its way. But in the case of
sub more_link_template(), the call is always identical, using only a
single scan directive, MM. So hard-coding it seems more efficient by
eliminating the call to escape_scan(). Since none of the three
options above is inherently more readable than the others, I vote for
option 3 for the sake of efficiency, but I am ready to be educated if
I have missed a reason why one of the other choices is better.

Replacing "secure => $CGI::secure," with "match_security => 1" is not
much of a change. All that happens in vendUrl() is that when it
detects match_security, it sets $opt->{secure} = $CGI::secure.
However, since using match_security is more self-documenting, I think
this is a good enhancement, and will make the change in git-hub.

Regards,
Carl
. . . . . . . . . . . . . . . . . .
Carl Bailey
End Point Corp.
t: 919-323-8025
. . . . . . . . . . . . . . . . . .

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

peter at pajamian

Oct 18, 2009, 1:59 PM

Post #5 of 7 (1083 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On 10/18/2009 08:28 AM, Carl Bailey wrote:
> I've been looking at this for a while now, trying to understand why
> the above would be better. From what I can tell, there are three ways
> to call tag_area() in this case:
>
> 1) tag_area('scan', "MM=$mv_arg", { form => $form_arg, match_security
> => 1, }
> 2) tag_area(undef, undef, { search => "MM=$arg", form => $form_arg,
> match_security => 1, }
> 3) tag_area("scan/MM=$mv_arg", undef, { form => $form_arg,
> match_security => 1, }
>
> All three calls return the identical URL.
>
> Options 1 and 2 both wind up making a call to escape_scan() to compose
> the URL. That's necessary in tag_area(), when the routine can not
> know what search variables will be coming its way. But in the case of
> sub more_link_template(), the call is always identical, using only a
> single scan directive, MM. So hard-coding it seems more efficient by
> eliminating the call to escape_scan(). Since none of the three
> options above is inherently more readable than the others, I vote for
> option 3 for the sake of efficiency, but I am ready to be educated if
> I have missed a reason why one of the other choices is better.

Hard coding things like this is generally not a good idea because it
removes the ability to change things in the future in one central
location. In this particular case it means that search parameters could
be stored internally rather than hard-coded into the URL as they are
now, also they scan specialpage could be changed for whatever reason.

> Replacing "secure => $CGI::secure," with "match_security => 1" is not
> much of a change. All that happens in vendUrl() is that when it
> detects match_security, it sets $opt->{secure} = $CGI::secure.
> However, since using match_security is more self-documenting, I think
> this is a good enhancement, and will make the change in git-hub.

Again it comes down to hard-coding things which is bad. If it's
determined at some future time that we want to try to get rid of globals
such as $CGI::secure in favor of object and method calls, then if you
use match_security the core change to the area tag will cover you, but
if you use $CGI::secure then that will have to be changed as well.

In general the idea is that you should think twice about hard coding
things. If you instead stick to using the provided attributes in a more
flexible manner then it helps to prevent problems down the road.

Peter

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

carl at endpoint

Oct 19, 2009, 8:39 AM

Post #6 of 7 (1061 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On Oct 18, 2009, at 4:59 PM, Peter wrote:

> On 10/18/2009 08:28 AM, Carl Bailey wrote:
>> I've been looking at this for a while now, trying to understand why
>> the above would be better. From what I can tell, there are three
>> ways
>> to call tag_area() in this case:
>>
>> 1) tag_area('scan', "MM=$mv_arg", { form => $form_arg, match_security
>> => 1, }
>> 2) tag_area(undef, undef, { search => "MM=$arg", form => $form_arg,
>> match_security => 1, }
>> 3) tag_area("scan/MM=$mv_arg", undef, { form => $form_arg,
>> match_security => 1, }
>>
>> All three calls return the identical URL.
>>
>> Options 1 and 2 both wind up making a call to escape_scan() to
>> compose
>> the URL. That's necessary in tag_area(), when the routine can not
>> know what search variables will be coming its way. But in the case
>> of
>> sub more_link_template(), the call is always identical, using only a
>> single scan directive, MM. So hard-coding it seems more efficient by
>> eliminating the call to escape_scan(). Since none of the three
>> options above is inherently more readable than the others, I vote for
>> option 3 for the sake of efficiency, but I am ready to be educated if
>> I have missed a reason why one of the other choices is better.
>
> Hard coding things like this is generally not a good idea because it
> removes the ability to change things in the future in one central
> location. In this particular case it means that search parameters
> could
> be stored internally rather than hard-coded into the URL as they are
> now, also they scan specialpage could be changed for whatever reason.
>
>> Replacing "secure => $CGI::secure," with "match_security => 1" is not
>> much of a change. All that happens in vendUrl() is that when it
>> detects match_security, it sets $opt->{secure} = $CGI::secure.
>> However, since using match_security is more self-documenting, I think
>> this is a good enhancement, and will make the change in git-hub.
>
> Again it comes down to hard-coding things which is bad. If it's
> determined at some future time that we want to try to get rid of
> globals
> such as $CGI::secure in favor of object and method calls, then if you
> use match_security the core change to the area tag will cover you, but
> if you use $CGI::secure then that will have to be changed as well.
>
> In general the idea is that you should think twice about hard coding
> things. If you instead stick to using the provided attributes in a
> more
> flexible manner then it helps to prevent problems down the road.
>
>
> Peter
>
> _______________________________________________
> interchange-users mailing list
> interchange-users [at] icdevgroup
> http://www.icdevgroup.org/mailman/listinfo/interchange-users

Not worth arguing further. I have updated github to use Peter's
better version.
http://github.com/carlbailey/interchange/commit/fd129761dabbf4e461538cae96af730bc6db4ca2

Carl
. . . . . . . . . . . . . . . . . . .
Carl Bailey
t: 919-323-8025
carl [at] endpoint
. . . . . . . . . . . . . . . . . . .

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

jon at endpoint

Oct 22, 2009, 4:22 PM

Post #7 of 7 (1005 views)
Permalink

Re: One-line patch for more_link_template [In reply to]

On Mon, 19 Oct 2009, Carl Bailey wrote:

> Not worth arguing further. I have updated github to use Peter's
> better version.
> http://github.com/carlbailey/interchange/commit/fd129761dabbf4e461538cae96af730bc6db4ca2

I committed a squashed version of this with indenting fixed and simple
double-quoting that the original had.

Jon

--
Jon Jensen
End Point Corporation
http://www.endpoint.com/

_______________________________________________
interchange-users mailing list
interchange-users [at] icdevgroup
http://www.icdevgroup.org/mailman/listinfo/interchange-users

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads