FWD » Core Development » Base Language

At this time the SESSION:STARTUP-PARAMETERS only needs to check to see if -rereadnolock is active.

What branch is this code in?

What is the status?

Greg Shah wrote:

What branch is this code in?

What is the status?

AFAIK there are no code changes yet.

Added conversion support and runtime stubs for SESSION:STARTUP-PARAMETERS and SESSION:SUPPRESS-WARNINGS to task branch 3284a revision 11171. Please review.

It seems that at this moment FWD can not and doesn't show warnings. The warnings as displayed in the native 4GL are in FWD presented as errors.

See the following example.

session:suppress-warnings = false.
default-window:column = ?.
WAIT-FOR WINDOW-CLOSE OF CURRENT-WINDOW.

The unknown assigned to COLUMN produces a warning message box in the native 4GL but in FWD an error message box is shown.

And indeed, when SUPPRESS-WARNINGS is set to true in the above code sample, no message box is displayed at all.

Thus in order to add support for SUPPRESS-WARNINGS the ability to show warning messages must be first implemented in FWD and eventually convert the existing error messages to warnings to some extent.

Do we want to investigate all the errors whether they should be displayed as warnings? Do we want to add this to the scope of this task?

Thus in order to add support for SUPPRESS-WARNINGS the ability to show warning messages must be first implemented in FWD

The ErrorManager has the following methods that are used for invoking some form of compatible error/warning processing. This is how we currently implement these methods, but it is not necessarily the 100% compatible approach. There are many inconsistencies that are probably due to our incremental approach to building the error processing. Over time we would add new method helpers, but we don't always go back and check all previously implemented error processing to see if how they handle all conditions. At some point we need to clean it all up.

It all seems quite haphazard to me, though I don't doubt that the 4GL error processing does have many special cases that will need to be handled.

Do we want to investigate all the errors whether they should be displayed as warnings? Do we want to add this to the scope of this task?

We don't have time to check it all. On the other hand, there are many paths through the error helpers that do not ever raise a condition (ERROR or STOP). I'm NOT talking about the ones that can be bypassed by NO-ERROR (this is silent mode) or by setIgnore(boolean). I'm saying that those entries with "Raised Condition" as "n/a" are possibly the ones that Progress would consider as warnings. It is worth checking (at least some of) those out to see if SUPPRESS-WARNINGS works to suppress the output (and if it forces recording of error data in that case). The call sites do need to be checked to see if they raise a condition after calling something like displayError() or displayAbend(). The ones that don't raise a condition are probably real warnings.

There is also a ErrorManager.isWarningMode() but that seems more like a kind of implicit NO-ERROR processing, which I think is different.

If you can find some simple patterns that allow us to know if something is a warning or not by how things are called in FWD, then it seems like we could add this processing by making changes in ErrorManager. We may have to remap the usage of those non-condition-raising helpers when they raise conditions in the caller, so that the usage is unambiguous.

Long term we will probably need to:

• rationalize the helpers to ensure all cases are considered in each form and that we minimize the number of methods to those actually needed
• push more of the processing back into the ErrorManager, even to the point of handling the message text so that internationalization of the messages can be centralized; to do this the caller will have to pass in some well defined constant (an enum?) that is unique to the specific error/warning and they would have to pass in any substitution variables instead of creating the text in the caller

I just don't think we have the time to do all of this now. See if you can find a fairly simple pattern that allows implementation. Report your results here and we can discuss.

As an aside, Progress has this knowledge base article that has a zip file which lists all the error messages and their spec (the error text with substitution placeholders):

https://knowledgebase.progress.com/articles/Article/000040758

Unfortunately, it doesn't tell whether something is an error or warning. It is not even clear which of these are compile-time, runtime or both. I think the way we have encoded things in FWD records the knowledge better.

Greg Shah wrote:

I'm saying that those entries with "Raised Condition" as "n/a" are possibly the ones that Progress would consider as warnings.

Although this sounds reasonable, it doesn't seem to be the case. I am finding that if a condition is not raised the displayed message can be shown either as a warning or an error. Also I am not finding any strong pattern between our static types (declares methods) and runtime states, and the warning/error behavior of the 4gl use cases.

An emerging pattern may be different areas of functionality. For example "error" messages displayed in the UI layer seem to be mostly displayed as warnings (with the SUPPRESS-WARNINGS sensitivity), other areas like runtime type checking, function invocation, persistence seem to be displayed as errors (without the SUPPRESS-WARNINGS sensitivity).

OK, then for now do the following:

• Create the helpers and infrastructure needed for easily implementing support for SUPPRESS-WARNINGS on a case by case basis.
• For those error cases that you have made a determination of what is or is not a "warning", implement this support.

You don't have to check all errors we currently support. As long as we can easily change the error processing code later to reflect future findings, it is OK.

The tip of 3285a implements SESSION:SUPPRESS-WARNINGS and limited support of SESSION:STARTUP-PARAMETERS. GUI regression tests passed, ChUI is in progress. Please review.

3285a passed ChUI regression tests.

Hynek Cihlar wrote:

The tip of 3285a implements SESSION:SUPPRESS-WARNINGS and limited support of SESSION:STARTUP-PARAMETERS. GUI regression tests passed, ChUI is in progress. Please review.

Code review 3285a/11175:

• Please note I've committed a new update 11176 to the branch with some minor cleanup.
• The new LegacyCLI class is missing a file header and license text.
• Please add a header entries to Persistence and CommonSession.
• Nice catch on the addRaisedCondition call in ErrorManager.displayError.
• Is access to the suppressed-warnings setting from RemoteErrorData meant to be read-only?

Code Review Task Branch 3285a Revision 11176

I'm fine with the code (aside from the items Eric mentioned).

I think Constantin will be merging 3260b to trunk shortly. You can rebase, add your final cleanups and then merge to trunk, unless there are conflicts that cause the regression testing to need to be redone.

Eric Faulhaber wrote:

• Is access to the suppressed-warnings setting from RemoteErrorData meant to be read-only?

That is correct.

The review points are addressed in 3285a revision 11177.

3285a was rebased and merged to trunk as revision 11174. The branch has been archived.

Please update the gaps\expressions.rules for both attributes. I think that both can be described as partial. Explain the status in a comment on the same line.

Do you have a branch in which to put this? If not, create a 3285b and put it there. Testing parsing and reports with Hotel GUI will be sufficient for regression tests.

Greg Shah wrote:

Please update the gaps\expressions.rules for both attributes. I think that both can be described as partial. Explain the status in a comment on the same line.

Do you have a branch in which to put this? If not, create a 3285b and put it there. Testing parsing and reports with Hotel GUI will be sufficient for regression tests.

I added the gap entries to 3285b, please review.

Code Review Task Branch 3285b Revision 11175

I'm fine with the changes. If you have tested them by running reporting on Hotel GUI, then you can merge to trunk.

3285a was merged to trunk as revision 11174.

Can I close this task?

Greg Shah wrote:

Can I close this task?

Yes, please.