Mailing List Archive

Sure, glad to open a discussion here; I personally fail to see the usefulness of this division as documented, plus the code itself had been inconsistent when it came to how it applied the DisplayErrors criteria (some call sites checked either/or and some were both/and). Whatever the decision ends up being should be made consistent, plus there are probably additional things that can/should be made to respect this directive as well.

Best,

David

> On Sep 8, 2017, at 6:00 PM, Jon Jensen <jon@endpoint.com> wrote:
>
> David,
>
> I don't mind the logic change personally, but the documentation shows clearly that the old behavior was intended.
>
> Unlike many identical pairs of global & catalog directives, this one was designed so that the global one is a universal switch to allow or deny the *ability* to use DisplayErrors in all catalogs:
>
> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_20.html
> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_98.html
> http://docs.icdevgroup.org/cgi-bin/online/confs/DisplayErrors.html
>
> So this probably needs either wider discussion and buy-in along with documentation updates, or to be reverted to match the docs.
>
> Jon
>
>
>
> On Fri, 8 Sep 2017, David Christensen wrote:
>
>> commit 096f9b97470c0106594573dda4114e5adf25e81c
>> Author: David Christensen <david@endpoint.com>
>> Date: Fri Sep 8 17:30:23 2017 -0500
>>
>> Correct logic of DisplayErrors
>>
>> Most directives (and other places with this specific directive) use 'or' logic to allow you to
>> define directives in either `interchange.cfg` or `catalog.cfg` to enable a default-off behavior at
>> either the Interchange server level or at the level of a specific catalog.
>>
>> As originally coded, the DisplayErrors directive would have only worked when *both*
>> `interchange.cfg` and `catalog.cfg` had this enabled.
>>
>> Change this so it shows up when either the current catalog or the global config has this set.
>>
>> Since the old default was 'no', someone intended this information to be output, so this is the
>> correct behavior, considering that I only fixed the output of this message in the previous commit.
>>
>> lib/Vend/Dispatch.pm | 6 +++---
>> 1 files changed, 3 insertions(+), 3 deletions(-)
>> ---
>> diff --git a/lib/Vend/Dispatch.pm b/lib/Vend/Dispatch.pm
>> index eeb463e..cea6640 100644
>> --- a/lib/Vend/Dispatch.pm
>> +++ b/lib/Vend/Dispatch.pm
>> @@ -690,7 +690,7 @@ Sorry, there was an error in processing this form action. Please
>> report the error or try again later.
>> EOF
>> $template .= "\n\nError: %s\n"
>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
>> ;
>> $template = get_locale_message(500, $template, $err);
>> logError($err);
>> @@ -1680,7 +1680,7 @@ EOF
>> and not ($Vend::admin and ! $::Variable->{MV_TRACK_ADMIN});
>> # END TRACK
>>
>> - if($Vend::Cfg->{DisplayErrors} and $Global::DisplayErrors) {
>> + if($Vend::Cfg->{DisplayErrors} or $Global::DisplayErrors) {
>> $SIG{"__DIE__"} = sub {
>> my $msg = shift;
>> put_session() if $Vend::HaveSession;
>> @@ -1889,7 +1889,7 @@ Sorry, there was an error in processing this form action. Please
>> report the error or try again later.
>> EOF
>> $template .= "\n\nError: %s\n"
>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
>> ;
>> $template = get_locale_message(500, $template, $err);
>> logError($err);
>
>
> --
> Jon Jensen
> End Point Corporation
> https://www.endpoint.com/
>
> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users

--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171

On Fri, 8 Sep 2017, David Christensen wrote:

> I personally fail to see the usefulness of this division as documented,

That's straightforward.

Interchange's global/catalog distinction was grounded in the idea that the
whole daemon would have certain important configuration made by an expert,
and individual catalogs would be done by developers who were perhaps less
expert in matters of security and stability.

Thus the ability to reload a catalog without restarting the whole
Interchange daemon.

If DisplayErrors is off globally, that means the admin (the global
overlord) doesn't want any catalogs to have it on.

The reason you didn't get it, and I don't mind if it changes, is that I
think almost nobody runs Interchange this way anymore.

Most installations I'm aware of (and I'm not just talking about End
Point's, but also those we've worked on that were set up by others) have
one Interchange daemon per company/business and one catalog for each site.

The global and catalog configurations and code are almost always managed
by the same people.

> plus the code itself had been inconsistent when it came to how it
> applied the DisplayErrors criteria (some call sites checked either/or
> and some were both/and).

Sure, there were bugs one way or the other.

> Whatever the decision ends up being should be made consistent, plus
> there are probably additional things that can/should be made to respect
> this directive as well.

Right-o.

I'd be interested to hear what other users think.

Jon

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users

I can tell you that the original intent was because of the multi-customer
orientation of the original IC / Minivend. The messages that DisplayErrors
give can give specific server information that you wouldn't want a random
user to enable.

I know that End Point tends to think anyone that is in control of a catalog
is a server admin, and that may even be mostly true these days. But it was
far from true in Minivend's early days.

Quoting David Christensen (david@endpoint.com):
> Sure, glad to open a discussion here; I personally fail to see the usefulness of this division as documented, plus the code itself had been inconsistent when it came to how it applied the DisplayErrors criteria (some call sites checked either/or and some were both/and). Whatever the decision ends up being should be made consistent, plus there are probably additional things that can/should be made to respect this directive as well.
>
> Best,
>
> David
>
> > On Sep 8, 2017, at 6:00 PM, Jon Jensen <jon@endpoint.com> wrote:
> >
> > David,
> >
> > I don't mind the logic change personally, but the documentation shows clearly that the old behavior was intended.
> >
> > Unlike many identical pairs of global & catalog directives, this one was designed so that the global one is a universal switch to allow or deny the *ability* to use DisplayErrors in all catalogs:
> >
> > http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_20.html
> > http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_98.html
> > http://docs.icdevgroup.org/cgi-bin/online/confs/DisplayErrors.html
> >
> > So this probably needs either wider discussion and buy-in along with documentation updates, or to be reverted to match the docs.
> >
> > Jon
> >
> >
> >
> > On Fri, 8 Sep 2017, David Christensen wrote:
> >
> >> commit 096f9b97470c0106594573dda4114e5adf25e81c
> >> Author: David Christensen <david@endpoint.com>
> >> Date: Fri Sep 8 17:30:23 2017 -0500
> >>
> >> Correct logic of DisplayErrors
> >>
> >> Most directives (and other places with this specific directive) use 'or' logic to allow you to
> >> define directives in either `interchange.cfg` or `catalog.cfg` to enable a default-off behavior at
> >> either the Interchange server level or at the level of a specific catalog.
> >>
> >> As originally coded, the DisplayErrors directive would have only worked when *both*
> >> `interchange.cfg` and `catalog.cfg` had this enabled.
> >>
> >> Change this so it shows up when either the current catalog or the global config has this set.
> >>
> >> Since the old default was 'no', someone intended this information to be output, so this is the
> >> correct behavior, considering that I only fixed the output of this message in the previous commit.
> >>
> >> lib/Vend/Dispatch.pm | 6 +++---
> >> 1 files changed, 3 insertions(+), 3 deletions(-)
> >> ---
> >> diff --git a/lib/Vend/Dispatch.pm b/lib/Vend/Dispatch.pm
> >> index eeb463e..cea6640 100644
> >> --- a/lib/Vend/Dispatch.pm
> >> +++ b/lib/Vend/Dispatch.pm
> >> @@ -690,7 +690,7 @@ Sorry, there was an error in processing this form action. Please
> >> report the error or try again later.
> >> EOF
> >> $template .= "\n\nError: %s\n"
> >> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
> >> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
> >> ;
> >> $template = get_locale_message(500, $template, $err);
> >> logError($err);
> >> @@ -1680,7 +1680,7 @@ EOF
> >> and not ($Vend::admin and ! $::Variable->{MV_TRACK_ADMIN});
> >> # END TRACK
> >>
> >> - if($Vend::Cfg->{DisplayErrors} and $Global::DisplayErrors) {
> >> + if($Vend::Cfg->{DisplayErrors} or $Global::DisplayErrors) {
> >> $SIG{"__DIE__"} = sub {
> >> my $msg = shift;
> >> put_session() if $Vend::HaveSession;
> >> @@ -1889,7 +1889,7 @@ Sorry, there was an error in processing this form action. Please
> >> report the error or try again later.
> >> EOF
> >> $template .= "\n\nError: %s\n"
> >> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
> >> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
> >> ;
> >> $template = get_locale_message(500, $template, $err);
> >> logError($err);
> >
> >
> > --
> > Jon Jensen
> > End Point Corporation
> > https://www.endpoint.com/
> >
> > _______________________________________________
> > interchange-users mailing list
> > interchange-users@icdevgroup.org
> > http://www.icdevgroup.org/mailman/listinfo/interchange-users
>
> --
> David Christensen
> End Point Corporation
> david@endpoint.com
> 785-727-1171
>
>
>



> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users


--
People who would hamper free speech always assume that they're designing
a world in which only their enemies will have to shut up.
--Nicholas Christakis

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users

Sure; in any case, we should rectify this in one way or the other. Any strong feelings from anyone here to keep the original intent?

Best,

David

> On Sep 11, 2017, at 2:48 AM, M Heins <mick@dlxmail.com> wrote:
>
> I can tell you that the original intent was because of the multi-customer
> orientation of the original IC / Minivend. The messages that DisplayErrors
> give can give specific server information that you wouldn't want a random
> user to enable.
>
> I know that End Point tends to think anyone that is in control of a catalog
> is a server admin, and that may even be mostly true these days. But it was
> far from true in Minivend's early days.
>
> Quoting David Christensen (david@endpoint.com):
>> Sure, glad to open a discussion here; I personally fail to see the usefulness of this division as documented, plus the code itself had been inconsistent when it came to how it applied the DisplayErrors criteria (some call sites checked either/or and some were both/and). Whatever the decision ends up being should be made consistent, plus there are probably additional things that can/should be made to respect this directive as well.
>>
>> Best,
>>
>> David
>>
>>> On Sep 8, 2017, at 6:00 PM, Jon Jensen <jon@endpoint.com> wrote:
>>>
>>> David,
>>>
>>> I don't mind the logic change personally, but the documentation shows clearly that the old behavior was intended.
>>>
>>> Unlike many identical pairs of global & catalog directives, this one was designed so that the global one is a universal switch to allow or deny the *ability* to use DisplayErrors in all catalogs:
>>>
>>> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_20.html
>>> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_98.html
>>> http://docs.icdevgroup.org/cgi-bin/online/confs/DisplayErrors.html
>>>
>>> So this probably needs either wider discussion and buy-in along with documentation updates, or to be reverted to match the docs.
>>>
>>> Jon
>>>
>>>
>>>
>>> On Fri, 8 Sep 2017, David Christensen wrote:
>>>
>>>> commit 096f9b97470c0106594573dda4114e5adf25e81c
>>>> Author: David Christensen <david@endpoint.com>
>>>> Date: Fri Sep 8 17:30:23 2017 -0500
>>>>
>>>> Correct logic of DisplayErrors
>>>>
>>>> Most directives (and other places with this specific directive) use 'or' logic to allow you to
>>>> define directives in either `interchange.cfg` or `catalog.cfg` to enable a default-off behavior at
>>>> either the Interchange server level or at the level of a specific catalog.
>>>>
>>>> As originally coded, the DisplayErrors directive would have only worked when *both*
>>>> `interchange.cfg` and `catalog.cfg` had this enabled.
>>>>
>>>> Change this so it shows up when either the current catalog or the global config has this set.
>>>>
>>>> Since the old default was 'no', someone intended this information to be output, so this is the
>>>> correct behavior, considering that I only fixed the output of this message in the previous commit.
>>>>
>>>> lib/Vend/Dispatch.pm | 6 +++---
>>>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>>> ---
>>>> diff --git a/lib/Vend/Dispatch.pm b/lib/Vend/Dispatch.pm
>>>> index eeb463e..cea6640 100644
>>>> --- a/lib/Vend/Dispatch.pm
>>>> +++ b/lib/Vend/Dispatch.pm
>>>> @@ -690,7 +690,7 @@ Sorry, there was an error in processing this form action. Please
>>>> report the error or try again later.
>>>> EOF
>>>> $template .= "\n\nError: %s\n"
>>>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
>>>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
>>>> ;
>>>> $template = get_locale_message(500, $template, $err);
>>>> logError($err);
>>>> @@ -1680,7 +1680,7 @@ EOF
>>>> and not ($Vend::admin and ! $::Variable->{MV_TRACK_ADMIN});
>>>> # END TRACK
>>>>
>>>> - if($Vend::Cfg->{DisplayErrors} and $Global::DisplayErrors) {
>>>> + if($Vend::Cfg->{DisplayErrors} or $Global::DisplayErrors) {
>>>> $SIG{"__DIE__"} = sub {
>>>> my $msg = shift;
>>>> put_session() if $Vend::HaveSession;
>>>> @@ -1889,7 +1889,7 @@ Sorry, there was an error in processing this form action. Please
>>>> report the error or try again later.
>>>> EOF
>>>> $template .= "\n\nError: %s\n"
>>>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
>>>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
>>>> ;
>>>> $template = get_locale_message(500, $template, $err);
>>>> logError($err);
>>>
>>>
>>> --
>>> Jon Jensen
>>> End Point Corporation
>>> https://www.endpoint.com/
>>>
>>> _______________________________________________
>>> interchange-users mailing list
>>> interchange-users@icdevgroup.org
>>> http://www.icdevgroup.org/mailman/listinfo/interchange-users
>>
>> --
>> David Christensen
>> End Point Corporation
>> david@endpoint.com
>> 785-727-1171
>>
>>
>>
>
>
>
>> _______________________________________________
>> interchange-users mailing list
>> interchange-users@icdevgroup.org
>> http://www.icdevgroup.org/mailman/listinfo/interchange-users
>
>
> --
> People who would hamper free speech always assume that they're designing
> a world in which only their enemies will have to shut up.
> --Nicholas Christakis
>
> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users

--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171

The way you can do that without changing code is to just enable it in
the distribution interchange.cfg. That would be my preference.

Quoting David Christensen (david@endpoint.com):
> Sure; in any case, we should rectify this in one way or the other. Any strong feelings from anyone here to keep the original intent?
>
> Best,
>
> David
>
> > On Sep 11, 2017, at 2:48 AM, M Heins <mick@dlxmail.com> wrote:
> >
> > I can tell you that the original intent was because of the multi-customer
> > orientation of the original IC / Minivend. The messages that DisplayErrors
> > give can give specific server information that you wouldn't want a random
> > user to enable.
> >
> > I know that End Point tends to think anyone that is in control of a catalog
> > is a server admin, and that may even be mostly true these days. But it was
> > far from true in Minivend's early days.
> >
> > Quoting David Christensen (david@endpoint.com):
> >> Sure, glad to open a discussion here; I personally fail to see the usefulness of this division as documented, plus the code itself had been inconsistent when it came to how it applied the DisplayErrors criteria (some call sites checked either/or and some were both/and). Whatever the decision ends up being should be made consistent, plus there are probably additional things that can/should be made to respect this directive as well.
> >>
> >> Best,
> >>
> >> David
> >>
> >>> On Sep 8, 2017, at 6:00 PM, Jon Jensen <jon@endpoint.com> wrote:
> >>>
> >>> David,
> >>>
> >>> I don't mind the logic change personally, but the documentation shows clearly that the old behavior was intended.
> >>>
> >>> Unlike many identical pairs of global & catalog directives, this one was designed so that the global one is a universal switch to allow or deny the *ability* to use DisplayErrors in all catalogs:
> >>>
> >>> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_20.html
> >>> http://www.icdevgroup.org/interchange-doc-5.2.0/frames/icconfig_98.html
> >>> http://docs.icdevgroup.org/cgi-bin/online/confs/DisplayErrors.html
> >>>
> >>> So this probably needs either wider discussion and buy-in along with documentation updates, or to be reverted to match the docs.
> >>>
> >>> Jon
> >>>
> >>>
> >>>
> >>> On Fri, 8 Sep 2017, David Christensen wrote:
> >>>
> >>>> commit 096f9b97470c0106594573dda4114e5adf25e81c
> >>>> Author: David Christensen <david@endpoint.com>
> >>>> Date: Fri Sep 8 17:30:23 2017 -0500
> >>>>
> >>>> Correct logic of DisplayErrors
> >>>>
> >>>> Most directives (and other places with this specific directive) use 'or' logic to allow you to
> >>>> define directives in either `interchange.cfg` or `catalog.cfg` to enable a default-off behavior at
> >>>> either the Interchange server level or at the level of a specific catalog.
> >>>>
> >>>> As originally coded, the DisplayErrors directive would have only worked when *both*
> >>>> `interchange.cfg` and `catalog.cfg` had this enabled.
> >>>>
> >>>> Change this so it shows up when either the current catalog or the global config has this set.
> >>>>
> >>>> Since the old default was 'no', someone intended this information to be output, so this is the
> >>>> correct behavior, considering that I only fixed the output of this message in the previous commit.
> >>>>
> >>>> lib/Vend/Dispatch.pm | 6 +++---
> >>>> 1 files changed, 3 insertions(+), 3 deletions(-)
> >>>> ---
> >>>> diff --git a/lib/Vend/Dispatch.pm b/lib/Vend/Dispatch.pm
> >>>> index eeb463e..cea6640 100644
> >>>> --- a/lib/Vend/Dispatch.pm
> >>>> +++ b/lib/Vend/Dispatch.pm
> >>>> @@ -690,7 +690,7 @@ Sorry, there was an error in processing this form action. Please
> >>>> report the error or try again later.
> >>>> EOF
> >>>> $template .= "\n\nError: %s\n"
> >>>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
> >>>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
> >>>> ;
> >>>> $template = get_locale_message(500, $template, $err);
> >>>> logError($err);
> >>>> @@ -1680,7 +1680,7 @@ EOF
> >>>> and not ($Vend::admin and ! $::Variable->{MV_TRACK_ADMIN});
> >>>> # END TRACK
> >>>>
> >>>> - if($Vend::Cfg->{DisplayErrors} and $Global::DisplayErrors) {
> >>>> + if($Vend::Cfg->{DisplayErrors} or $Global::DisplayErrors) {
> >>>> $SIG{"__DIE__"} = sub {
> >>>> my $msg = shift;
> >>>> put_session() if $Vend::HaveSession;
> >>>> @@ -1889,7 +1889,7 @@ Sorry, there was an error in processing this form action. Please
> >>>> report the error or try again later.
> >>>> EOF
> >>>> $template .= "\n\nError: %s\n"
> >>>> - if $Global::DisplayErrors && $Vend::Cfg->{DisplayErrors}
> >>>> + if $Global::DisplayErrors || $Vend::Cfg->{DisplayErrors}
> >>>> ;
> >>>> $template = get_locale_message(500, $template, $err);
> >>>> logError($err);
> >>>
> >>>
> >>> --
> >>> Jon Jensen
> >>> End Point Corporation
> >>> https://www.endpoint.com/
> >>>
> >>> _______________________________________________
> >>> interchange-users mailing list
> >>> interchange-users@icdevgroup.org
> >>> http://www.icdevgroup.org/mailman/listinfo/interchange-users
> >>
> >> --
> >> David Christensen
> >> End Point Corporation
> >> david@endpoint.com
> >> 785-727-1171
> >>
> >>
> >>
> >
> >
> >
> >> _______________________________________________
> >> interchange-users mailing list
> >> interchange-users@icdevgroup.org
> >> http://www.icdevgroup.org/mailman/listinfo/interchange-users
> >
> >
> > --
> > People who would hamper free speech always assume that they're designing
> > a world in which only their enemies will have to shut up.
> > --Nicholas Christakis
> >
> > _______________________________________________
> > interchange-users mailing list
> > interchange-users@icdevgroup.org
> > http://www.icdevgroup.org/mailman/listinfo/interchange-users
>
> --
> David Christensen
> End Point Corporation
> david@endpoint.com
> 785-727-1171
>
>
>



> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users


--
Being against torture ought to be sort of a bipartisan thing.
-- Karl Lehenbauer

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users

> On Sep 11, 2017, at 9:02 AM, M Heins <mick@duxmail.com> wrote:
>
> The way you can do that without changing code is to just enable it in
> the distribution interchange.cfg. That would be my preference.

Well either way, the existing code contained contradictory logic (both && and || existed in the codebase), so that’s what I’m attempting to fix.

So it sounds like your preference is to change it to && but change the default to 1, yes?

Best,

Davod
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171

Hmm. Not what I see -- I see three && or "and" changing to an or function.
I can tell you that the intent was AND.


On Mon, Sep 11, 2017 at 4:40 PM, David Christensen <david@endpoint.com>
wrote:

>
> > On Sep 11, 2017, at 9:02 AM, M Heins <mick@duxmail.com> wrote:
> >
> > The way you can do that without changing code is to just enable it in
> > the distribution interchange.cfg. That would be my preference.
>
> Well either way, the existing code contained contradictory logic (both &&
> and || existed in the codebase), so that’s what I’m attempting to fix.
>
> So it sounds like your preference is to change it to && but change the
> default to 1, yes?
>
> Best,
>
> Davod
> --
> David Christensen
> End Point Corporation
> david@endpoint.com
> 785-727-1171
>
>
>
>
> _______________________________________________
> interchange-users mailing list
> interchange-users@icdevgroup.org
> http://www.icdevgroup.org/mailman/listinfo/interchange-users
>
>


--
Just because something is obviously happening doesn't mean something
obvious is happening. --Larry Wall

Hi Mike,

From HEAD^ (i.e., before my patch), we have the following in lib/Vend/Util.pm:

$Vend::Errors .= $msg . "\n"
if $Vend::Cfg->{DisplayErrors} || $Global::DisplayErrors;

So this should be changed to also be &&, yes? I’m happy to revert and fix this call site instead, since it sounds like there is an established reason and it wasn’t an oversight as I’d assumed. :-)

Best,

David
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171

> On Sep 11, 2017, at 12:27 PM, David Christensen <david@endpoint.com> wrote:
>
> Hi Mike,
>
> From HEAD^ (i.e., before my patch), we have the following in lib/Vend/Util.pm:
>
> $Vend::Errors .= $msg . "\n"
> if $Vend::Cfg->{DisplayErrors} || $Global::DisplayErrors;
>
> So this should be changed to also be &&, yes? I’m happy to revert and fix this call site instead, since it sounds like there is an established reason and it wasn’t an oversight as I’d assumed. :-)

Upon reviewing this, I’ve decided to leave things as-is; this instance occurs within `logError()` which I believe could be called in either a global context or a catalog-level context. I’m not convinced that turning this into an && won’t break this when called in a global context.

If you think I’m wrong about being wrong here :-), please feel free to push a correction for this instance. I did go ahead and push a reversion to the commit which changed this logic.

Best,

David
--
David Christensen
End Point Corporation
david@endpoint.com
785-727-1171

Quoting David Christensen (david@endpoint.com):
>
> > On Sep 11, 2017, at 12:27 PM, David Christensen <david@endpoint.com> wrote:
> >
> > Hi Mike,
> >
> > From HEAD^ (i.e., before my patch), we have the following in lib/Vend/Util.pm:
> >
> > $Vend::Errors .= $msg . "\n"
> > if $Vend::Cfg->{DisplayErrors} || $Global::DisplayErrors;
> >
> > So this should be changed to also be &&, yes? I???m happy to
> > revert and fix this call site instead, since it sounds like there
> > is an established reason and it wasn???t an oversight as I???d
> > assumed. :-)
>
> Upon reviewing this, I???ve decided to leave things as-is; this
> instance occurs within `logError()` which I believe could be called
> in either a global context or a catalog-level context. I???m not
> convinced that turning this into an && won???t break this when
> called in a global context.
>
> If you think I???m wrong about being wrong here :-), please feel
> free to push a correction for this instance. I did go ahead and push
> a reversion to the commit which changed this logic.

I think this makes sense. Though I am not at all against a new default
interchange.cfg file which has

DisplayErrors Yes

in it, expecially within the "ifdef DEBUG" area.

--
Mike Heins
phone +1.765.253.4194 <mike@heins.com>

"The secret of a good sermon is to have a good beginning and a good
ending, then having the two as close together as possible."
-- George Burns

_______________________________________________
interchange-users mailing list
interchange-users@icdevgroup.org
http://www.icdevgroup.org/mailman/listinfo/interchange-users