FWD » Core Development » User Interface

Add support for:

• FRAME-NAME attribute (this is actually a generic widget attribute but it will need to be implemented in the frame itself, possibly with conversion time changes needed)
• SCROLL frame option
• NO-ATTR-SPACE frame option

SCROLL and NO-ATTR-SPACE will be finished in #4164. The only thing needed for this task is FRAME-NAME.

I cannot find any useful documentation related to how FRAME-NAME would be used in a testcase. Searching through our testcases, I only see that utilized in &Scoped-define FRAME-NAME ... statements.

Any simple example I could see?

The frame-name is just a string, right? So you can do anything with it to see the value (e.g. message widget:frame-name. or my-char-var = widget:frame-name..

Of course, you need to check it in multiple scenarios (chui/gui, before/after realization, on the frame itself/on contained widgets, with the unnamed/default frame, with an explicitly named frame).

Greg Shah wrote:

Add support for:

• FRAME-NAME attribute (this is actually a generic widget attribute but it will need to be implemented in the frame itself, possibly with conversion time changes needed)

I'm researching, and there is already a keyword for FRAME-NAME which is kw_fr_name. However, there is no methods_attributes.rules support. My assumptions is that there will need to be getter/setter support added in FrameInterface.java and FrameWidget.java then conversion of the attribute to invoke. Then there's the actual frame name itself, but I have a hard time believing there isn't something already implemented to hold the frame name at this point... but I could be wrong.

My assumptions is that there will need to be getter/setter support added in FrameInterface.java and FrameWidget.java then conversion of the attribute to invoke.

No, as noted above: "this is actually a generic widget attribute". Look at the 4GL docs. This can be called on any widget. So it should not be in the frame, but in CommonWidget/GenericWidget.

Then there's the actual frame name itself, but I have a hard time believing there isn't something already implemented to hold the frame name at this point... but I could be wrong.

Search GenericFrame for frameName. You can obtain it by calling _getName(). All widgets know their frame. See GenericWidget.getFrame(). It may be null in some cases. This should be enough info to implement.

Code ready for review. Additional testcases being developed. Tested via ON MOUSE-SELECT-CLICK of various widgets on a large frame, as well as on a separate default frame.

Committing to: /home/rfb/secure/code/p2j_repo/p2j/active/4231b/                                                      
modified rules/convert/methods_attributes.rules
modified src/com/goldencode/p2j/ui/CommonWidget.java
modified src/com/goldencode/p2j/ui/GenericWidget.java
Committed revision 11446. 

1. assign b4-frname = i:frame-name. converts to b4Frname.assign()
2. display i:frame-name. converts to f0Frame.widgeti().getFrameName()

The first doesn't compile. The second is what I was expecting.

The b4Frname.assign() is missing the f0Frame.widgeti().getFrameName() argument.

You'll have to fix it. Most likely, there is a missing frame-id annotation which causes the attribute to bypass being emitted.

Code Review Task Branch 4231b Revision 11446

The only issue is the use of new String() in GenericWidget.getFrameName(). That is unnecessary and is not ever done in Java. The correct code is return frame != null ? frame._getName() : "";.

What have your testcases shown about the behavior? If the frame is not set for a widget (e.g. a CREATE <widget> which has no FRAME attribute ever set), then does it really return empty string? I would have guessed it to return unknown value. Anyway, please document the correct behavior so I can evaluate your work.

Greg Shah wrote:

The b4Frname.assign() is missing the f0Frame.widgeti().getFrameName() argument.

You'll have to fix it. Most likely, there is a missing frame-id annotation which causes the attribute to bypass being emitted.

Looking at more code emitted... would this be similar to why this code:

  b4col = frame-col(f0)
  b4row = frame-row(f0)

b4col.assign(0);
b4row.assign(0);

Those don't look correct.

Roger Borrello wrote:

Looking at more code emitted... would this be similar to why this code:
[...]
converts to:
[...]

Those don't look correct.

It looks like frame_scoping.rules has a check for this functions which reference the frame:

      <!-- process functions which reference the frame --> 
      <rule>
         evalLib("is_func", type)               and 
         this.getAnnotation("oldtype") != null  and
         (getNoteLong("oldtype") == prog.kw_fr_row  or
          getNoteLong("oldtype") == prog.kw_fr_col  or
          getNoteLong("oldtype") == prog.kw_fr_line or
          getNoteLong("oldtype") == prog.kw_fr_down)

         <rule>
            this.getFirstChild() != null and 
            this.getFirstChild().type == prog.wid_frame

            <!-- named frame as a parameter --> 
            <action on="true">
               execLib("add_node_ex", 
                       execLib("new_frame", this.getFirstChild().text, false), copy)
            </action>

            <!-- no valid named frame, but check if there is a parameter -->
            <rule on="false">this.getFirstChild() == null

               <!-- no parameter means this is an implicit reference to the default frame -->
               <action>execLib("add_node", copy)</action>

               <!-- invalid (non-existing) frame name case, all of these funcs always return 0
                    in this case; we will hard code it here because there is no frame reference
                    to emit -->
               <action on="false">copy.type = prog.num_literal</action>
               <action on="false">copy.text = "0"</action>

            </rule>
         </rule>
      </rule>

Do I need to look at these, and make a more in-depth analysis of whether or not there should be a frame reference?

We can't go on an 2 week effort to dig into this. It would be good to fix these as well. The issue is probably different.

In the function case (as opposed to your attribute case), the expression this.getFirstChild() != null and this.getFirstChild().type == prog.wid_frame fails. This is the reason for the issue. f0 should appear as a wid_frame in the tree. Fix that and these will work.

Greg Shah wrote:

The b4Frname.assign() is missing the f0Frame.widgeti().getFrameName() argument.

You'll have to fix it. Most likely, there is a missing frame-id annotation which causes the attribute to bypass being emitted.

In this case, I am referencing a widget that is not associated with any frame. I'll look at 4GL to see what happens.

Roger Borrello wrote:

Greg Shah wrote:

The b4Frname.assign() is missing the f0Frame.widgeti().getFrameName() argument.

You'll have to fix it. Most likely, there is a missing frame-id annotation which causes the attribute to bypass being emitted.

In this case, I am referencing a widget that is not associated with any frame. I'll look at 4GL to see what happens.

Could not find i in a frame. Try qualifying the reference with an IN FRAME phrase. (3566)
** z:\testcases\uast\frame_layout\frame_widget_attributes_helper.i Could not understand line 2. (196)

There should be some sort of error or warning during conversion, I would think, rather than letting this get to the compilation stage.

As someone wise once said: "The problem was a loose nut behind the wheel.". :)

Our current implementation assumes you are providing correct syntax.

Greg Shah wrote:

Code Review Task Branch 4231b Revision 11446

The only issue is the use of new String() in GenericWidget.getFrameName(). That is unnecessary and is not ever done in Java. The correct code is return frame != null ? frame._getName() : "";.

What have your testcases shown about the behavior? If the frame is not set for a widget (e.g. a CREATE <widget> which has no FRAME attribute ever set), then does it really return empty string? I would have guessed it to return unknown value. Anyway, please document the correct behavior so I can evaluate your work.

When you look for frame-name on a widget not yet on a frame, it shows "?" in 4GL.

f0: b4-col/b4-row= 0 / 0 aft-col/aft-row= 1 / 1 col/row= 1 / 1 h1: b4-frname/aft-frname= ? / ? x/y= 0 / 0 fx/fy= ? / ? c/r= 1 / 1 fc/fr= ? / ?

Legend:

   "{&fr}: b4-col/b4-row="b4-col "/" b4-row 
   "aft-col/aft-row="frame-col({&fr}) "/" frame-row({&fr}) 
   "col/row="{&colval} "/" {&rowval}
   "{&wid}:" 
   "b4-frname/aft-frname="b4-frname "/" {&wid}:frame-name
   "x/y=" {&wid}:x "/" {&wid}:y                   /* pix loc of wid left/top edge rel to left parent edge */
   "fx/fy=" {&wid}:frame-x "/" {&wid}:frame-y     /* pix loc of wid left/top edge rel to ul of frame */
   "c/r=" {&wid}:column "/" {&wid}:row            /* col/row pos of wid left/top edge rel to left/top (same as x/y) */
   "fc/fr=" {&wid}:frame-col "/" {&wid}:frame-row /* Dec col pos of wid left/top edge rel to ul of wid frame */

Right. So your existing code is not correct. I suspect that this is not the string "?" but it is just how the language statement prints unknown value. Check it.

Greg Shah wrote:

Right. So your existing code is not correct. I suspect that this is not the string "?" but it is just how the language statement prints unknown value. Check it.

There is a name() method in GenericWidget that returns the name of the widget as character. When I change getFrameName to return character, I get the desired unknown value:
f0: b4-col/b4-row= 1 / 1 aft-col/aft-row= 1 / 1 col/row= 1 / 1 h1: b4-frname/aft-frname= ? / ? x/y= 0 / 0 fx/fy= 0 / 0 c/r= 1 / 1 fc/fr= 1 / 1

There are other widget attributes that aren't matching up (frame-x, frame-y, frame-col, and frame-row), so I am looking those over.

The new code in getFrameName:

   public character getFrameName()
   {
      return frame != null ? new character(frame._getName()) : new character();
   }

We always must return BDT values from attribute getters. We never return String instances.

Initial attribute values being returned for frame-col and frame-row are incorrect. They are coming from the client, and I could use a hint how to debug that area of the client. Like where to set a breakpoint.

Roger Borrello wrote:

Initial attribute values being returned for frame-col and frame-row are incorrect. They are coming from the client, and I could use a hint how to debug that area of the client. Like where to set a breakpoint.

I see that I need to include suspend. Nevermind.

By updating getFrameRow and getFrameCol in ThinClient.java to return a 0, if the frame is not visible, I get matching results to the 4GL.

   public double getFrameRow(int frameId)
   {
      Frame frame = getFrame(frameId);
      double result = 0; // Changed from 1 to 0

      if (frame != null && frame.isVisible())
      {
         result = frame.location().y + 1;
      }

      return result;
   }

   {
      Frame frame = getFrame(frameId);
      double result = -1; // Changed from 0 to -1

      if (frame != null && frame.isVisible() && !frame.isRedirected())
      {
         result = ConfigHelper.getAssignedColumn(frame.config());
      }

      if (result == BaseConfig.INV_COORD)
      {
         result = 0;
      }

      return ++result;
   }

I am not particularly fond of the way in which they do not proceed in the same manner with returning an incremented value, versus starting with the same initialization. I want to change it, but Greg mentioned there are some complex computations that take place with respect to coordinates. So I thought I'd ask if these look feasible, or if it's ok to re-swizzle the code a little to make them more consistent to each other.

Roger Borrello wrote:

By updating getFrameRow and getFrameCol in ThinClient.java to return a 0, if the frame is not visible, I get matching results to the 4GL.

Code review, the changes are OK.

Getting back to FRAME-NAME, it looks like convert/frame_construction.rules retrieves the critical data using:

         <action>javaname = getNoteString("javaname")</action>
         <action>frameName = getNoteString("name")</action>

The frameName ends up being all lowercase, which is not the same as the 4GL outputs for FRAME-NAME. Shouldn't this be javaname for the frame, in order to preserve the case?

No. Nein. Nyet. Non. Nee.

Greg Shah wrote:

No. Nein. Nyet. Non. Nee.

Tried updating that, and it looks like it should be using name. However, the annotation is not correct.

OK, perhaps I need to clarify.

javaname is a converted Java name and will have been transformed to remove all the gorp that can be in a 4GL name but that cannot appear in Java. Never consider this to be a legacy name.

And you do need the legacy name, right?

So: save off the legacy name before it gets lowercased. Store than in an annotation and stuff it into the frame name.

Greg Shah wrote:

OK, perhaps I need to clarify.

javaname is a converted Java name and will have been transformed to remove all the gorp that can be in a 4GL name but that cannot appear in Java. Never consider this to be a legacy name.

And you do need the legacy name, right?

So: save off the legacy name before it gets lowercased. Store than in an annotation and stuff it into the frame name.

I understand about javaname. I could either add another annotation in the frame_alloc node, or make the name annotation the correct case. I'll have to see if that's used elsewhere.

About a year ago, Constantin made a specific update to annotations/frame_scoping.rules:

CA  20190327          Fixed dictionary for frame names (values must be lowercased).

      <!-- set name to the current frame scope -->
      <function name="name_scope">
         <parameter name="fname" type="java.lang.String" />
         <variable name="var"    type="java.util.Map" />
         <variable name="local"  type="java.util.Map" />

         <rule>fname != null and !fname.equals('')
            <action>var = execLib("get_named_frame", fname)</action>

            <rule>var != null
               <action>local = #(java.util.Map) 
                       lookupDictionaryObject(currScope, frameVar)</action>
               <action>evalLib("merge_scopes", local, var)</action>
            </rule>
            <rule>var == null
               <!-- no such frame -->
               <action>var = #(java.util.Map) 
                       lookupDictionaryObject(currScope, frameVar)</action>
               <action>var.put(frameName, fname)</action>

               <action>execLib("add_frame", fname, var)</action>
            </rule>
            <action>addDictionaryObject(currScope, frameVar, var)</action>
            <action>addDictionaryObject(currScope, frameName, fname.toLowerCase())</action>
         </rule>
      </function>

Unfortunately it is difficult to discern the backstory, because this has downstream effects in convert/frame_construction.rules where the name is pulled when frame_alloc is found.

I am still looking if I can add "legacy_name" annotation, but it's a challenge to work in the mapping methods of frame_scoping.rules.

Why mess around? Just store it before frame scoping occurs.

Greg Shah wrote:

Why mess around? Just store it before frame scoping occurs.

From what I know, it's pay-me-now, or pay-me-later, because frame_scoping is where the FRAME_ALLOC node is created, and that's where the frame is constructed in conversion. If I put the annotation in the frame definition, I'll have to find it during the construction phase. That could very well be the easier path.

Made changes in frame_scoping.rules to create a "parallel" legacy name that is handled in the same manner as the frame name, except the original case is maintained throughout. At the end, when the frame_alloc is created, a new annotation legacy_name is stored in the AST. This is used by frame_construction.rules to build the createFrame method.

Review needed, and other testcases related to shared frames and browses will need to be performed.

Committing to: /home/rfb/secure/code/p2j_repo/p2j/active/4231b/                                                                                          
modified rules/annotations/frame_scoping.rules
modified rules/convert/frame_construction.rules
Committed revision 11483.

1. Add the legacy_name as a 3rd parameter to GenericFrame.createFrame instead of replacing the 2nd, or
2. Create a setFrameName setter to add to the frame definition

FrameLocAttributesGet3Pbframe pbframeFrame = GenericFrame.createFrame(FrameLocAttributesGet3Pbframe.class, "pbFrame");

I'm not sure which is better. I do know the concept of the FRAME-NAME may have 2 definitions, one for the implementation internally, and one for the exposed 4GL interface. If the use of the mixed-case original 4GL name is OK to use throughout the implementation, I just need to complete this by storing it as "name" in the annotations, rather than creating a new "legacy_name" and letting all the other code handle it as-is. It would just be the original name, rather than a manufactured name.

I doubt that is a good idea. The original 4GL code allows specifying the name in any case when you refer to the frame. So it can be 100 different versions throughout a file. I suspect this is why Constantin lowercased name recently, so that all of the TRPL code would process using the same name.

And that is fine, except for when the legacy name is accessed as a character attribute or through a 4GL built-in function. Then we need to report the same thing as the 4GL.

Which brings up a point which I should have mentioned previously. Make sure you test these scenarios:

• DEFINE FRAME and different names used everywhere
• FORM and different names used everywhere
• no DEFINE FRAME or FORM and different names used everywhere

By "everywhere", I mean all the other frame phrase references:

DEFINE FRAME mY-fRaMe ...
DISPLAY ... WITH FRAME My-Frame.
DOWN 1 WITH FRAME MY-framE.

Capice?

Hint: I wonder if the 4GL returns (from FRAME-NAME()) the first frame name that is defined, the last one that is defined or some other ridiculous random feverdream.

Greg Shah wrote:

Hint: I wonder if the 4GL returns (from FRAME-NAME()) the first frame name that is defined, the last one that is defined or some other ridiculous random feverdream.

Thankfully it utilizes the first mention of the frame.

DISPLAY i ch WITH FRAME fRame1.
FORM i ch WITH FRAME FramE1 2 DOWN.

DO  i = 1 TO 2:
   ch = STRING(i).
   DISPLAY i ch WITH FRAME fRame1.
   i:PRIVATE-DATA IN FRAME frAme1 = STRING(i).
   ch:PRIVATE-DATA IN FRAME fraMe1 = STRING(i).
   DOWN WITH FRAME framE1.
END.
MESSAGE "ch:FRAME-NAME=" ch:FRAME-NAME.

Displays the first mention, regardless of what I move at the top (DISPLAY or FORM).

Is FRAME-NAME() ready for review?

Greg Shah wrote:

Is FRAME-NAME() ready for review?

1. Add the legacy_name as a 3rd parameter to GenericFrame.createFrame instead of replacing the 2nd, or
2. Create a setFrameName setter to add to the frame definition

Today, what is that 2nd parameter used for other than as the legacy name?

Greg Shah wrote:

Today, what is that 2nd parameter used for other than as the legacy name?

1. The frame's name (this.frameName) which is actually supporting FRAME-NAME attribute.
2. The frame's name (this.name) which is built by concatenating this.frameName+##+this.config.Class.getName().

I suppose I could keep the 2nd parameter as the original name, and in the GenericFrame.coreInitialize method, perform a lowercase of it before building up the 2nd frame name, and all would be as it was.

I suppose I could keep the 2nd parameter as the original name, and in the GenericFrame.coreInitialize method, perform a lowercase of it before building up the 2nd frame name, and all would be as it was.

I prefer that idea.

Constantin: Any objection?

Greg Shah wrote:

I suppose I could keep the 2nd parameter as the original name, and in the GenericFrame.coreInitialize method, perform a lowercase of it before building up the 2nd frame name, and all would be as it was.

I prefer that idea.

Constantin: Any objection?

No objection, this approach should work.

I have updated and my testcase passes.

Committing to: /home/rfb/secure/code/p2j_repo/p2j/active/4231b/                                                                                      
modified src/com/goldencode/p2j/ui/GenericFrame.java
Committed revision 11487.

However, there is a failure in this particular testcase (./uast/frame_layout/widget-hierarchy_fname.p) when it is executing @CAN-QUERY where it allows SCREEN-VALUE to pass, but in 4GL this shows as not a queryable attribute for FRAME widget. The testcase then attempt to read it, and ** SCREEN-VALUE is not a queryable attribute for FRAME widget. (4025) is displayed.

Will open a bug.

Code Review Task Branch 4231b Revisions 11483, 11486, 11487

The changes are good.

1. Please modify the GenericFrame javadoc so that it is clear what is the legacy 4GL name (case-preserved), what is the internal frame name (lowercased) and so forth. I think coreInitialize() needs edits in this regard as well as the name and frameName members.

2. In java_templates.tpl there is the construct_frame template. The javaname target is a misnomer. It now should be called legacy_name. Please change it there and you'll have to change it everywhere that template is used. This is just to make the code more clear, otherwise it will confuse those who follow you.

Greg Shah wrote:

Code Review Task Branch 4231b Revisions 11483, 11486, 11487

The changes are good.

1. Please modify the GenericFrame javadoc so that it is clear what is the legacy 4GL name (case-preserved), what is the internal frame name (lowercased) and so forth. I think coreInitialize() needs edits in this regard as well as the name and frameName members.

frameName is easy to find and document. But name is inherited from someplace else. Would that be cfg.Configuration? Should I just document GenericFrame or go beyond?

Greg Shah wrote:

2. In java_templates.tpl there is the construct_frame template. The javaname target is a misnomer. It now should be called legacy_name. Please change it there and you'll have to change it everywhere that template is used. This is just to make the code more clear, otherwise it will confuse those who follow you.

I only see it used in frame_construction.rules.

Should I just document GenericFrame or go beyond?

Just GenericFrame is fine.

Ready for test.

Committing to: /home/rfb/secure/code/p2j_repo/p2j/active/4231b/                                                                                      
modified rules/convert/frame_construction.rules
modified rules/convert/java_templates.tpl
modified src/com/goldencode/p2j/ui/GenericFrame.java
Committed revision 11488.   

Code Review Task Branch 4231b Revision 11488

The only issue is a cut/paste problem in frame_construction.rules which should not have history entry 132. It is just a "sub-entry" of the already existing history entry 016.