FWD » Core Development » User Interface

MENU, SUB-MENU and MENU-ITEM widget support; DEFINE MENU and DEFINE SUB-MENU stmts, widget options: MENU-ITEM, IN, SUB-MENU, TOGGLE-BOX, MENUBAR

• define menu/sub-menu is not properly support.
• to add menu for current window (like menubar) need to add next code:
  

  assign current-window:menubar = menu mbar:handle.
  

  
  but it not properly supported. At the moment I'm working on this problem. Added WindowWidget next methods: setMenuBar and getMenuBar. Also I've added properly conversion for it.

Next things I'll plan to create Classes Menu and SubMenu and interfaces for it. And then will work on conversion for Menu/SubMenu.

Please check in your 4GL testcases into a testcases/uast/menu/ directory so I can get an idea of the cases you have looked at.

Greg Shah wrote:

Please check in your 4GL testcases into a testcases/uast/menu/ directory so I can get an idea of the cases you have looked at.

for particular problem, I'm using next testcase:

define sub-menu topic
   menu-item numbr label "Number" 
   menu-item addr label "Address".

define sub-menu move
   menu-item forward label "Forward" 
   menu-item backward label "Back".

define sub-menu quitit
   menu-item quititem label "E&xit".

define menu mbar menubar
   sub-menu topic label "Topic" 
   sub-menu move label "Move" 
   sub-menu quitit label "E&xit".

on choose of menu-item numbr
   display "Number".

on choose of menu-item addr
   display "Address".

on choose of menu-item forward
   display "forward".

on choose of menu-item backward
   display "backward".

assign current-window:menubar = menu mbar:handle.

wait-for choose of menu-item quititem.

Make sure you add cases for TOGGLE-BOX in a menu-item definition and MENUBAR in the DEFINE MENU.

I think it also makes sense to include RULE, SKIP, DISABLED, ACCELERATOR and READ-ONLY in the tests. We will leave the color, font, LIKE and TITLE support for later.

Please post your current code for an early review. Your status report noted this:

2) added conversion for current-window object

We already have Vadim working on this feature for #2229 (which is about GUI support). And we already have some limited CURRENT-WINDOW support for ChUI implemented (see rules/convert/variable_references.rules and search for kw_cur_win.

I don't want to duplicate effort here. What do you need to add right now?

Greg Shah wrote:

I don't want to duplicate effort here. What do you need to add right now?

I need to properly conversion for next line:

assign current-window:menubar = menu mbar:handle.

I need to properly conversion for next line:
[...]
This code add menu to current window

Hopefully you only need to add the MENUBAR attribute support to methods_attributes.rules (and the proper methods to the window widget). I think the CURRENT-WINDOW referent should already resolve properly.

Greg Shah wrote:

I need to properly conversion for next line:
[...]
This code add menu to current window

Hopefully you only need to add the MENUBAR attribute support to methods_attributes.rules (and the proper methods to the window widget). I think the CURRENT-WINDOW referent should already resolve properly.

Yes, you are correct. I didn't do anything else for it.

Please upload your latest changes here.

Please check in all your testcases to testcases/uast/menu/.

I need to re-assign this task to someone else immediately. Post any notes here about the status or findings of the task.

I tried to work on conversion of current-window:menubar = menu mbar:handle.
I added the following rule to method-attributes.rules (added at line 1894):

              <rule>ftype == prog.kw_menu_bar
                  <action>hwrap = "Window"</action>
                  <action>methodText = "getMenubar"</action>
                  <rule>isAssign
                     <action>methodText = "setMenubar"</action>
                  </rule>
               </rule>

   /**
    * Get window MENUBAR.
    * 
    * @return  handle.
    */
   @LegacyAttribute(name = "MENUBAR")
   public handle getMenubar();

   /**
    * Set window MENUBAR.
    * 
    * @param  handle.
    */
   @LegacyAttribute(name = "MENUBAR", setter = true)
   public void setMenubar(handle menubar);

   @Override
   public handle getMenubar()
   {
      // TODO Auto-generated method stub
      return null;
   }

   @Override
   public void setMenubar(handle menubar)
   {
      // TODO Auto-generated method stub

   }

After that conversion became successful, but it converts source string to the following code currentWindow().unwrap().readOnlyError("menubar");. Could you advice me why it doesn't see the setter?

You need to add ftype == prog.kw_menu_bar || to the list in the read_only_attribute function in include/common-progress.rules. Then it will detect that attribute as a setter.

Thanks, it is little unexpected. Why the global read_only_attribute function is necessary?

The next conversion problem was appeared when I tried to convert define menu m.. Initially such source procedure could be successfully converted to empty Java class. I added MenuWidget class and started to add conversion rules and faced with errors. I do the following. Because of menu's have own namespace like frames (can be addressed using menu m:handle like frame f:handle), I tried to convert 2 statements:

def frame f.
def menu m.

block [BLOCK] @0:0
   statement [STATEMENT] @0:0
      def [DEFINE_FRAME] @1:1
         f [SYMBOL] @1:11
   statement [STATEMENT] @0:0
      def [DEFINE_MENU] @2:1
         menu [KW_MENU] @2:5
            m [SYMBOL] @2:10

<ast col="0" id="575525617666" line="0" text="statement" type="STATEMENT">
      <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
      <ast col="1" id="575525617667" line="1" text="def" type="DEFINE_FRAME">
        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        <ast col="11" id="575525617669" line="1" text="f" type="SYMBOL">
          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        </ast>
      </ast>
    </ast>
    <ast col="0" id="575525617671" line="0" text="statement" type="STATEMENT">
      <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
      <ast col="1" id="575525617672" line="2" text="def" type="DEFINE_MENU">
        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        <ast col="5" id="575525617675" line="2" text="menu" type="KW_MENU">
          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
          <ast col="10" id="575525617678" line="2" text="m" type="SYMBOL">
            <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
          </ast>
        </ast>
      </ast>
    </ast>

This problem looks like Lexer problem. It seems the Lexer extracts unwanted token KW_MENU from the source. Unfortunately conforming *.p.lexer file is empty. By the way, I found that before. Why it's empty?
I also tried to find something wrong in progress.g but I couldn't. Could you advice me something?

Why the global read_only_attribute function is necessary?

Progress doesn't detect assignments to read-only attributes at compile time. I don't know why. Instead, it is a runtime error. In order to duplicate this behavior, we convert assignments to read-only attributes into a special call that will only ever generate the error. Writable attributes must be listed in that function so that this read-only path is not taken.

We can see here an unwanted KW_MENU. Probably that is the real problem. It leads to wrong AST:

Not wrong, just sub-optimal. You would have to write different rules to handle the extra level in the tree. But it is simple enough to get rid of.

This problem looks like Lexer problem. It seems the Lexer extracts unwanted token KW_MENU from the source.

No, it is a parser thing. If the lexer did not generate the token, then the parser would not know this was a define menu statement. The token must be there in the token stream from the lexer.

But we don't have to include the token in the tree. Normally, when I rewrite the KW_DEFINE as a DEFINE_<type> token, I also drop the 2nd keyword from the tree. I didn't do that this time.

Just go into progress.g and make this change:

def_menu_stmt returns [int mtype = WID_MENU]                                   <---- here
   :
      (KW_MENU! | KW_SUB_MENU! { mtype = WID_SUB_MENU; } ) m:symbol            <---- here
      (
         options { generateAmbigWarnings = false; }
         :
           ui_stuff
         | (KW_MENU_BAR | KW_SUB_M_H | simple_title_string) 
         | (like_menu_clause | menu_element_descriptor)

           // TODO: are there other parts of menu_element_descriptor or
           //       like_menu_clause etc... which can trigger this deviant
           //       4GL behavior?
         | { LA(2) == KW_MENU_ITM }?
           DOT
      )*
      {
         sym.addMenu(#m.getText(), mtype);
      }      
   ;

and this one too:

define_stmt      
   :
      {
         int     mtype      = -1;   <----- add this here
         String  varname    = null;

and this from define_stmt:

              | mtype=def_menu_stmt           // also handles SUB-MENU          <----- here
                {
                   if (mtype == WID_MENU)                                       <----- here

The more I'm investigating menus, more I'm founding that they are very similar to frames.
1. Definition of menu consists of sub-menu's list as a frame definition consists of widgets list.
2. Menu like Frame has no parent. Menu can be one of 2 types: MENUBAR and POPUP MENU. Menubar can be owned by a Window. POPUP MENU can be owned by different widgets excluding some of them, like IMAGE, TEXT..
Menu has a property OWNER - intended for pointing to OWNER, FRAME contains config.windowId property, pointing to owning window.
3. Menuitems, sub-menus can be referenced with IN MENU or IN SUBMENU clauses like IN FRAME clause is used for frame widgets.

These peculiarities induce me to create similar conversion rules as for frames. These rules are complex. It is also difficult for me to gather frame conversion rules from different rules files and understand :overall conversion plan" if I can say so. Therefore I need your advice. Here are my questions.
1. Is comparison with frames correct and will it be right to create similar rules?
2. Is it correct to generate similar java code:
- 2 classes: GenericMenu and CommonMenu like GenericFrame and CommonFrame.
- for each procedure there are custom implementation of CommonMenu interface is generated.
- this custom impl contains static field configClass (class of Menu definition also generated here).
3. It probably needed separate MenuRegistry in a LogicalTerminal or also MenuItemRegistry. Is it right?
4. If I'm adding new rules file, do I need to register it somewhere or it will be automatically used during conversion because it will be resided in /rules subfolder?

Some questions about AST
1. What additional information should be added to AST in annotations? What are common useful annotations for similar tasks? I found the following: name, type, widdef
2. I modified fixups/post_parse_fixups.rules, adding menu type in the following way:

         <rule>
            (type == prog.define_button    or
             type == prog.define_browse    or
             type == prog.define_rectangle or
             type == prog.define_menu) and
            isNote('tempidx'))
            <action>
               addDictionaryLong("var_defs",
                                 string(getNoteLong("tempidx")),
                                 id)
            </action>
            <action>removeNote("tempidx")</action>
            <action>putNote("widdef", true)</action>
            <action>fprintf("post_parse_fixups.log",
                            "WID_DEF  %-40s: %-25s @ %05d : %05d\n",
                            file,
                            text,
                            line,
                            this.getColumn())
            </action>
         </rule>

  def frame f.
  def menu m.

WID_DEF  ./menu/menu.p           : def             @ 00001 : 00001
WID_DEF  ./menu/menu.p           : def             @ 00001 : 00001

What do you think?

1. Is comparison with frames correct and will it be right to create similar rules?

No, I don't think so.

Menus are significantly simpler than frames:

• few options
• very limited choices for contained items AND they are not full widgets
• only the togglebox option allows "editing" and even this is very simple
• few attributes and methods
• doesn't represent data (except for the togglebox)
• simple structure
• no redirected terminal usage
• no headers
• limited and implicit layout/size control
• no page-top/page-bottom features
• no aggregate phrase support
• no @ base-field support
• no need for a "menu-definition" like with frames (the frame definition is there to cleanly integrate with the converted business logic in getting/setting data into contained widgets

Menus do have some strangeness in their implementation since they are a widget that is not contained in a frame. But other than this fact, they are not close to frames at all.

2. Is it correct to generate similar java code:
- 2 classes: GenericMenu and CommonMenu like GenericFrame and CommonFrame.
- for each procedure there are custom implementation of CommonMenu interface is generated.
- this custom impl contains static field configClass (class of Menu definition also generated here).

Not really. It may be that a CommonMenu interface is probably needed if CommonWidget is not a good match. And then there would be a MenuWidget server-side class and a MenuConfig... just like with normal widgets.

Please treat this like a widget instead of a frame.

3. It probably needed separate MenuRegistry in a LogicalTerminal or also MenuItemRegistry. Is it right?

I don't see the need for it, unless you have found some reason.

4. If I'm adding new rules file, do I need to register it somewhere or it will be automatically used during conversion because it will be resided in /rules subfolder?

There is no special registration. If you add something to rules/annotations/ and insert that into annotations.xml, it will work. Likewise you can add something to rules/convert/ and add the reference in core_conversion.xml.

1. What additional information should be added to AST in annotations? What are common useful annotations for similar tasks? I found the following: name, type, widdef

I don't have any guidance here. We add annotations when we find some downstream decision that we must make based on something previously analyzed or decided at an earlier phase.

2. I modified fixups/post_parse_fixups.rules, adding menu type in the following way:

It seems reasonable.

First row corresponds to def frame f and the second row corresponds to def menu m.

No, this is not right. I think you are just seeing some kind of duplicate message for the first line of that program. Your DEFINE_MENU node doesn't ever get annotated with a tempidx during parsing, so it doesn't ever enter this block. Prove it to yourself by updating the printf code like this:

            <action>fprintf("post_parse_fixups.log",
                            "WID_DEF  %-40s: %-40s @ %05d : %05d\n",
                            file,
                            this.getSymbolicTokenType(),
                            line,
                            this.getColumn())
            </action>

The descriptive token type will be much more useful than the text, which will usually be DEF or def or define... You can leave this change since it will make the log more useful.

The bottom line here is that you need to make changes to how the parser's define_stmt implements menus. Pattern the changes after how buttons work. Buttons don't represent data but they act like widgets that respond to events, just like menus. Of course, buttons are contained inside frames, but that really doesn't matter for parsing purposes. Step through the parser to see how define button works, especially in how it is processed by the symbol resolver (see SymbolResolver.annotateVariableOptions()). Make sure that the definitions of menus and menuitems are properly linked with references to those same widgets, using the tempidx.

3. In spite of post_parse_fixups.log contains a row, related to def menu m the target AST does not contain any annotations, that should be added by modified conversion rule. No widdef annotation.

When there is a tempidx annotation in the DEFINE_MENU node, then you will have your widdef annotation.

I made for DEFINE MENU good AST similar to DEFINE BUTTON except the nested KW_MENU. I tried to find that place where you got rid of similar KW_BUTTON for DEFINE BUTTON but I didn't find that place. Could you remember where it is?

In note 17, I suggested the changes needed to get rid of the nested KW_MENU. The added ! in front of the KW_MENU and KW_SUB_MENU in def_menu_stmt will do it. But I put additional changes there because the calling code was relying upon those keywords, so in my suggested changes the rule had a new return value.

The def_button_stmt is the similar place where we drop the KW_BUTTON. But the menu approach must be a bit different since it can match 2 possible cases.

I also have difficulties to find a place where JAST is emitted.

frame_generator.xml does this for frames and all static widgets contained in frames.

If we wanted to create separate menu definition classes, we would put something new in place for that. It would NOT be complex like frame_generator.xml, but much simpler. This is probably best.

We could also emit this stuff into the business logic in ui_statements.rules, but I don't like that. The triggers and method/attribute usage would be in the business logic, but the static definitions are best kept elsewhere.

Code Review vig_upd20141105a.zip

The changes are a good start. Some thoughts:

1. Your progress.g changes are missing my suggestions from note 17. Since you are using the return value for varname, that will make it trickier to do this. Right now the calling code relies upon the presence of the KW_MENU or KW_SUB_MENU. When you drop those, you will need to return the WID_MENU or WID_SUBMENU values instead. The calling code can walk the returned tree (in the #m value) and read the text of the varname instead of returning this from def_menu_stmt.

2. I think the SymbolResolver will need more work. The lookups probably have to deal with the Variable wrapper.

Code Review vig_upd20141114a.zip

1. Lines 104 and 105 of annotations/variable_definitions.rules contain duplicated code.

2. You asked this:

Should I create similar definition class for Menu with menu items definition and initialization?

As I noted above:

If we wanted to create separate menu definition classes, we would put something new in place for that. It would NOT be complex like frame_generator.xml, but much simpler. This is probably best.

In other words: YES. I think this would be a sub-class of MenuDefinition. MenuDefinition would be an abstract class with a single void setup(menuWidget) method. The subclass would have:

• A public member for each SubMenuWidget from a corresponding DEFINE_SUB_MENU.
• A concrete setup() method that would:
  • The parameter passed in would be the MenuWidget that is being setup/configured.
  • Create MenuItemWidget instances and add them to the menu or sub-menu, based on the KW_MENU_ITM instances in the DEFINE_MENU or DEFINE_SUB_MENU. Since these aren't named, they don't need to be members of the MenuDefinition sub-class. However, they do need to be properly configured, so perhaps they need to be saved as local variables of the setup() method.
  • Create MenuRule and SkipEntity instances and add them to the corresponding sub-menu.
  • Add each sub-menu widget into the menu or sub-menu parent.
  • Configure the options in each of the above widget (and the menu itself)
  • All the "add to the menu or sub-menu" processing noted above should be done in the right order such that it would link together the menu items/sub-menus into the proper menu hierarchy.

In addition to a MenuDefinition sub-class, you would have the DEFINE_MENU statement emit a class member in the external procedure something like this:

MenuWidget name = MenuWidget.createMenu(<my_menu_def>.class)

This would be a static factory method that would instantiate the MenuWidget, instantiate the MenuDefinition sub-class and then call setup() to configure/link everything together. The MenuWidget instance would be returned from createMenu().

Any sub-menu widget references would be converted into menu.sub_menu_name member references. I don't see the need to have the getters/setters/widget accessors like with frames. Let's avoid that complexity.

When designing the methods for adding/configuring the menus/sub-menus/menu-items, please first design the approach for supporting CREATE MENU/CREATE SUB-MENU/CREATE MENU-ITEM. In such cases, I guess there is business logic to set attributes to configure PARENT link. I'm not sure how to setup the order of sub-menus and menu-items since there are no methods in the MENU widget to support "adds" or "removals". Anyway, it seems like whatever is done for dynamic cases can also be used in the non-dynamic MenuDefinition case.

3. In regard to your question:

What did you mean in the point 2 in previous note?

You use addWrappedWorker() in SymbolResolver.addMenu(). This probably means that you should use lookupWrappedType(lookupWrapped(menuDict, name)) in SymbolResolver.lookupMenu() instead of lookupWorkerExact().

As part of the proper "linking" and referencing that you will have to do to convert all the business logic references to menus and sub-menus, you will have to make sure that the refids link everything together during parse/post-parse-fixups. This will require some parser/symbol resolver improvements.

As part of your recent status report, you mention:

By the way I don't know how to implement InvocationHandler, that is required for proxy generation.

I haven't looked at the changes yet, but we don't need any proxy generation for menus. That is my point above when I said this:

I don't see the need to have the getters/setters/widget accessors like with frames. Let's avoid that complexity.

The proxy is only needed for frames because we have an interface that defines getters/setters/widget accessors. We don't need that for menus.

Code Review vig_upd20141117a.zip

1. MenuDefinition does NOT need to extend InvocationHandler and it needs no invoke() method. It is just meant as a simple base class for menu definitions, which for now may only need the setup(MenuWidget) method. This also means that the MenuWidget.createStaticMenu() will be much simpler (no proxy stuff).

2. Since we are now generating a separate class file, we should NOT be calling menu_generator.rules from core_conversion.xml. Rules used in core_conversion.xml are there for the conversion of the business logic. They are operating on a jast which is for the original .p.ast file. Note that the frame_generator.xml is a separate step in the conversion process, which occurs after annotations and before base structure. You will have to do something similar. See convert/ConversionDriver.java and the generateFrames() method. I suspect many of your problems are caused by trying to generate a separate menu definition jast from inside core conversion. By the way, as a separate step menu_generator.rules should be named menu_generator.xml.

Answers to your questions:

Some of rules I just copied from frame_generator.rules or frame_construction.rules. Some of them I changed a little. What way will be correct?

Yes, this is OK. Just try to avoid copying things that are not needed. My intention is for this menu def to be much simpler than frame defs.

Is it possible to inherit/override rules?

No.

2. Same question about java templates.

No.

Can I also modify existing templates as I did with gen_setUp template?

Yes, this is fine. As long as you fixup all the other places it is used, there is no problem with making changes.

First one is for source procedure and the second one is for Menu definition. But the problem is that menu.p.jast contains almost the same JAST as MenuDefinitionM.jast. It seems it is being mistakenly overridden during conversion, but I can't find where it happens. See menu.zip.

This is what I mentioned above. The step can't be part of core conversion.

I can't generate import com.goldencode.p2j.ui.*; for MenuWidget in the main class. I added createImport call, but it doesn't work. I can't find the reason yet. I suspect, that the problem relates to jroot (i.e. Aast is being added to wrong tree) and correlates with the point 3. Could you help me?

This is the same cause as mentioned above. Separate the steps and the problem will go away I think.

Greg, I'm trying to create separate rules as you wrote. I faced with the following problem. MenuDefinitionM class is not being created although corresponding rules are being executed. Why it can be offhand?

Is it just the .java code that is missing? Or are both the .jast and .java missing?

Only .java is missing.

Make sure you have the proper setup in the ConversionDriver. Here is the generateFrames():

      // enable new file tracking mode
      JavaPatternWorker.enableNewFileTracking();

      processTrees("Frame Generator",
                   "convert/frame_generator",
                   codenames,
                   false,
                   asts,
                   debug);

      // add generated JASTs to the overall list (used with brew later)
      JavaPatternWorker.disableNewFileTracking();
      frames = JavaPatternWorker.getPersistedFiles();

You must use enableNewFileTracking(), disableNewFileTracking() and getPersistedFiles() to obtain the list of .jast files that were created.

Then you need to add this array to the ExplicitFileList in ConversionDriver.convertSourceNamesToJavaAstNames(). Right now it only handles the "main" jasts created during core_conversion.xml and those created during frame generation.

Code Review vig_upd20141120a.zip

1. About frames versus menus:

In a frames it happens more complex with several tree passes. Firstly all frame widgets are marked with frame-id annotation in annotations/frame_scope.rules. Then all widgets are collected in a frame map in walk-rules in convert/frame_generator.xml. Real conversion is happen in post-rules: for each collected widget template variable_def_default is emitted.

Frames are MUCH more complicated than menus.

a) A frame definition can be defined in many different language statements (e.g. FORM, DEFINE FRAME, DISPLAY, ENABLE, UPDATE, SET...). These language statements usually (except for FORM and DEFINE FRAME) have other functional behavior that is separate from the frame definition itself. In other words, the frame definition is a kind of side-effect. For that reason, we often have to make copies of the AST nodes to a hidden location that can be used to separate the different conversion needs. This copying has to be done in an earlier conversion pass.

b) Frames are scoped resources. This means that we must find all references to that same frame and analyze their location to determine at which point the frame's scope opens. This is probably the most important reason that frames need multiple passes of processing.

c) The same options and widgets can be specified many times in a file, including with conflicting options/config/settings. The entire set must be processed and some precedence rules honored, before we can know which ones actually matter.

These are the most important reasons for frames having multiple passes. I don't think menus have any of this.

2. About these other questions:

Why my simple "method" doesn't work. graft method is called but JAST is not emitted?

Please explain what you mean by "JAST is not emitted". Is the entire .jast file missing for a given menu definition? Or are you talking about specific nodes in the .jast which are missing?

How do you think is there possible to use my method or I will be forced to use frame "method"

I think it is probably possible, however it could well be that some addition post-parse fixups or annotations processing will be needed to prepare the trees.

Do I need separate file for menu annotations?

What do you mean by "separate file for menu annotations"? Even for frames, we don't ever have a separate file just for annotations.

You also wrote earlier that I will need to set refid annotation to properly link ASTs. Do I correctly understand? What for it is used in frames?

For frames we gather up all widgets for a frame and mark them with a "frame-id" annotation (see annotations/frame_scoping.rules). This lets us link each widget reference back to its containing frame.

Menus are much simpler:

• Each DEFINE SUB-MENU creates a named sub-menu widget and its contained menu items or sub-menus.
• Each DEFINE MENU creates a named menu and there should be a menu definition class for this entire hierarchy. It lists the top-level contained menu-items and sub-menus.

This means that named sub-menus can be referenced in either a DEFINE MENU or in a "container" DEFINE SUB-MENU. This means there is some linking to do to figure out the container for each named sub-menu.

This can't be done fully during parsing, because AST nodes don't have their node IDs assigned yet. That is why we create the tempidx annotations to use to link things at parse time and then at post-parse-fixups we replace the tempidx with the real "refid".

3. ConversionDriver needs a history entry.

1. "JAST is not emitted". I expected that there will be separate field in MenuDefinitionM class for each menu-item. tw.graft call had to add corresponding AST subtree to MenuDefinitionM.jast. But there is no AST corresponding menu-item field, and therefore there is no field in a result Java class MenuDefinitionM

2. Separate file. For frames I meant frame_scoping.rules. If I shouldn't create separate annotations file. What file should I use to add annotations for menu (menu-id and some others may be).

Vadim Gindin wrote:

1. "JAST is not emitted". I expected that there will be separate field in MenuDefinitionM class for each menu-item. tw.graft call had to add corresponding AST subtree to MenuDefinitionM.jast. But there is no AST corresponding menu-item field, and therefore there is no field in a result Java class MenuDefinitionM

I see. I suspect the problem is due to where you call persistJavaFile(). This saves the JAST to a .jast file. It is called during gen_code which means that before you can ever add your menu items, the .jast has already been written.

In frame_generator.xml, gen_code is processed in post-rules, after everything has been gathered up. So it can work there using that "already gathered" approach.

2. Separate file. For frames I meant frame_scoping.rules. If I shouldn't create separate annotations file. What file should I use to add annotations for menu (menu-id and some others may be).

I see. I doubt you need something so "heavy" as that. More likely you can make some minor changes to the parser and to post-parse-fixups to link the sub-menus and containers together. That is probably all you need.

I'm trying to generate the field MenuItemWidget menu = new MenuItemWidget(false) and faced with the following problem. Only new MenuItemWidget() is generated, i.e. without parameter false. I used the following template:

   <!-- instantiate a final variable using a 1 param constructor -->
   <ast-root name="variable_def_final_1_param" terse="true" ast_class="com.goldencode.p2j.uast.JavaAst" >
      <ast id="0" type="ASSIGN" text="=">
         <ast id="0" type="REFERENCE_DEF" text="${wrapper}">
            <annotation datatype="java.lang.String" key="name" value="${javaname}" />
            <annotation datatype="java.lang.Boolean" key="final" value="${final}" />
         </ast>
         <ast id="0" type="CONSTRUCTOR" text="${wrapper}">
            <annotation datatype="java.lang.Boolean" key="peernode" value="true" />
            <ast id="0" type="${type}" text="${text}" />
         </ast>
      </ast>
   </ast-root>

My call looks like this:

                             
tw.graft("variable_def_final_1_param", null,
         getSectionAnchor(java.cs_instance_vars).id,
         "final", "false",
         "wrapper", "MenuItemWidget",
         "javaname", javaname,
         "type", "boolean",
         "text", "false")

The problem is in type value. I tried: "boolean", "java.lang.Boolean" and "KW_BOOLEAN". The result is the same: parameter value is not generated. JAST looks like this:

        <ast col="0" id="644245094418" line="0" text="=" type="ASSIGN">
          <ast col="0" id="644245094419" line="0" text="MenuItemWidget" type="REFERENCE_DEF">
            <annotation datatype="java.lang.String" key="name" value="mit"/>
            <annotation datatype="java.lang.Boolean" key="final" value="false"/>
          </ast>
          <ast col="0" id="644245094420" line="0" text="MenuItemWidget" type="CONSTRUCTOR">
            <ast col="0" id="644245094421" line="0" text="false" type=""/>
          </ast>
        </ast>

Take note at the type attribute in the line: <ast col="0" id="644245094421" line="0" text="false" type=""/>. It is empty for the values "boolean" and "java.lang.Boolean", and it becomes KW_BOOLEAN when "KW_BOOLEAN" was passed. But in all 3 cases result is the same: false is not set.

What am I doing wrong? What is the right value for the type attribute?

Sorry, I got it: BOOL_FALSE

Please consider emitting the most common settings into the SubMenuWidget/MenuItemWidget constructor. The unchanged 4GL name (accessed via the NAME attribute) and the label seem like very common settings that virtually every menu-item/sub-menu will have. By emitting the most common options as constructor parameters, the resulting menu definition will be much smaller/cleaner.

I have 2 questions. I have simple testcase containing 2 define menu statements and I'm expecting 2 menu definition files: MenuDefinitionMen1.jast and MenuDefinitionMen2.jast.

1. They are equal, i.e. MenuDefinitionMen1.jast contains definition of menu2. In other words it was overwritten. Although that resulting classes MenuDefinitionMen1.java and MenuDefinitionMen2.java are correct. Why and how to correct that?

2. Menu-items in spite of being processed in menu_generation.xml are converted to the main class too. Those labels are become a part of body method:

public void body()
{
    "Firs&t";
    "S&econd";
    "Third";
    "Firs&t";
    "S&econd";
    currentWindow().unwrap().readOnlyError("menubar");         
}

It seems that menu-items are treated as frame-widgets somewhere because of those labels. Where it could be?

1. They are equal, i.e. MenuDefinitionMen1.jast contains definition of menu2. In other words it was overwritten. Although that resulting classes MenuDefinitionMen1.java and MenuDefinitionMen2.java are correct. Why and how to correct that?

I'm not sure what is wrong here. It is best for you to debug into the JavaPatternWorker.createJavaFile() and JavaPatternWorker.persistJavaFile() to see where things go wrong. I suspect that it may have something to do with re-using the root AST node, since there can only be one of those at a time in the JavaPatternWorker, but I'm not sure. You're debugging session should prove the problem.

It seems that menu-items are treated as frame-widgets somewhere because of those labels. Where it could be?

Each KW_LABEL node has a STRING child, right? By default, all strings are output in conversion/literals.rules. Of course, anything output like that will emit into the business logic as you have found. Search on suppress and look at line 744 in that rules file. You'll have to add conditions there to avoid KW_SUB_MENU/KW_LABEL/STRING and KW_MENU_ITM/KW_LABEL/STRING cases.

Code Review vig_upd20141124a.zip

This is a good step forward.

1. In menu_generator.xml, you need more specific matching logic in line 74:

         <!-- processing of MENU-ITEMs -->
         <rule>type == prog.kw_menu_itm

The reason: in the parser, KW_MENU_ITM is matched in 3 different ways:

• as a menu item definition in a DEFINE MENU or DEFINE SUB-MENU statement
• as a widget type in CREATE WIDGET (e.g. CREATE MENU-ITEM my-item-handle.)
• as a qualifier in a widget reference (e.g. IF MENU-ITEM my-static-menu-item-name:CHECKED THEN MESSAGE "it's checked!".)

Your code will match all three, which is not what we would want. Search on KW_MENU_ITM in progress.g to see the different usages. Add more specific conditions to the rule. Most likely you should be checking the parent/grandparent types to ensure it is only matching the DEFINE MENU or DEFINE SUB-MENU statement cases.

2. I think you can remove the get_mnode function in menu_generator.xml, right?

3. Shouldn't the name parameter to the MenuItem constructor be the original Progress 4GL name? Right now you are passing the converted javaname. It is important that the NAME attribute for a widget to be the legacy 4GL name.

4. Your code merge of the WindowWidget has the history entries mixed up. Some text from Constantin's H024 appears after your H025 entry.

5. Why have you commented out the use of setDateOrder() in literals.rules?

6. It seems to me that the following is what is remaining for conversion support:

• other MENU-ITEM options/settings like READ-ONLY or TOGGLE-BOX
• sub-menu conversion:
  • DEFINE SUB-MENU
  • sub-menu linkage into menus
• SKIP
• RULE
• CREATE MENU
• CREATE SUB-MENU
• CREATE MENU-ITEM
• attributes and methods
• static options for the menu/sub-menu itself (in DEFINE MENU/DEFINE SUB-MENU statements)

Am I missing anything?

Greg Shah wrote:

Code Review vig_upd20141124a.zip

This is a good step forward.

1. In menu_generator.xml, you need more specific matching logic in line 74:

[...]

The reason: in the parser, KW_MENU_ITM is matched in 3 different ways:

• as a menu item definition in a DEFINE MENU or DEFINE SUB-MENU statement
• as a widget type in CREATE WIDGET (e.g. CREATE MENU-ITEM my-item-handle.)
• as a qualifier in a widget reference (e.g. IF MENU-ITEM my-static-menu-item-name:CHECKED THEN MESSAGE "it's checked!".)

Your code will match all three, which is not what we would want. Search on KW_MENU_ITM in progress.g to see the different usages. Add more specific conditions to the rule. Most likely you should be checking the parent/grandparent types to ensure it is only matching the DEFINE MENU or DEFINE SUB-MENU statement cases.

OK

2. I think you can remove the get_mnode function in menu_generator.xml, right?

Yes, You're right.

3. Shouldn't the name parameter to the MenuItem constructor be the original Progress 4GL name? Right now you are passing the converted javaname. It is important that the NAME attribute for a widget to be the legacy 4GL name.

Ok, I'll fix it.

4. Your code merge of the WindowWidget has the history entries mixed up. Some text from Constantin's H024 appears after your H025 entry.

5. Why have you commented out the use of setDateOrder() in literals.rules?

As you remember there was a problem with conversion related to this rule (reported by Ovidiu if I remember correctly). o I commented it just to be able to continue working on this task. Probably the original problem is already solved. I'll check it. Anyway I didn't plan to commit this change.

6. It seems to me that the following is what is remaining for conversion support:

• other MENU-ITEM options/settings like READ-ONLY or TOGGLE-BOX
• sub-menu conversion:
  • DEFINE SUB-MENU
  • sub-menu linkage into menus
• SKIP
• RULE
• CREATE MENU
• CREATE SUB-MENU
• CREATE MENU-ITEM
• attributes and methods
• static options for the menu/sub-menu itself (in DEFINE MENU/DEFINE SUB-MENU statements)

Am I missing anything?

No, you are correct. I only can say about MENUBAR option (from your last point) in DEFINE MENU, that defines is menu POPUP (owned by widget) or not (owned by window)

I need advices about SUB-MENUs.

1. SUBMENU and MENU have different sets of attributes and probably have different behavior relating to uniqueness rules (I'll check it). So I decided that SUBMENU will have separate widget class.

2. One SUBMENU can be owned by several MENU. It also defined in a separate statement DEFINE SUBMENU that is similar to DEFINE MENU statement. So I decided that SUBMENU also needs to have separate definition class and also specific setup method in it with SubMenuWidget parameter.

3. I will need to generate new field in MenuDefinition for each submenu used in this menu in a following manner (like menu field): MenuDefinitionSM mbar = SubMenuWidget.createStaticSubmenu(MenuDefinitionSM.class, "sm");

4. I'm only planning to use MenuDefinition abstract class in both cases.

Could you advice me if that is right?

1. SUBMENU and MENU have different sets of attributes and probably have different behavior relating to uniqueness rules (I'll check it). So I decided that SUBMENU will have separate widget class.

OK.

2. One SUBMENU can be owned by several MENU. It also defined in a separate statement DEFINE SUBMENU that is similar to DEFINE MENU statement. So I decided that SUBMENU also needs to have separate definition class and also specific setup method in it with SubMenuWidget parameter.

OK. I didn't realize that it could be used by multiple menu instances. That does suggest there should be a separate SubMenuDefinition.

3. I will need to generate new field in MenuDefinition for each submenu used in this menu in a following manner (like menu field): MenuDefinitionSM mbar = SubMenuWidget.createStaticSubmenu(MenuDefinitionSM.class, "sm");

I'm not sure about this.

Yes, each MenuDefinition will need a data member for each contained sub-menu. But I don't think that the createStaticSubmenu() can be used directly in the MenuDefinition.

When a sub-menu is shared, is it a single instance that is shared? I suspect so. If it is a single instance, then that instance must be instantiated in the business logic (the Java code that is the equivalent of the external procedure .p file) and then assigned to each of the MenuDefinition members that represent it.

4. I'm only planning to use MenuDefinition abstract class in both cases.

Yes, I think that is OK unless you find that the setup() method signature is not suitable for both.

Greg Shah wrote:
..

2. One SUBMENU can be owned by several MENU. It also defined in a separate statement DEFINE SUBMENU that is similar to DEFINE MENU statement. So I decided that SUBMENU also needs to have separate definition class and also specific setup method in it with SubMenuWidget parameter.

OK. I didn't realize that it could be used by multiple menu instances. That does suggest there should be a separate SubMenuDefinition.

3. I will need to generate new field in MenuDefinition for each submenu used in this menu in a following manner (like menu field): MenuDefinitionSM mbar = SubMenuWidget.createStaticSubmenu(MenuDefinitionSM.class, "sm");

I'm not sure about this.

Yes, each MenuDefinition will need a data member for each contained sub-menu. But I don't think that the createStaticSubmenu() can be used directly in the MenuDefinition.

When a sub-menu is shared, is it a single instance that is shared? I suspect so. If it is a single instance, then that instance must be instantiated in the business logic (the Java code that is the equivalent of the external procedure .p file) and then assigned to each of the MenuDefinition members that represent it.

Thank you. I need to check if a single instance is shared or a copy is created.

4. I'm only planning to use MenuDefinition abstract class in both cases.

Yes, I think that is OK unless you find that the setup() method signature is not suitable for both.

I would create generic variable in MenuDefinition<PARENT> or create a separate class SubMenuDefinition or just extend SubMenuWidget form MenuWidget. But I think the last variant is not good.

worked on remarks, added submenu conversion rules, tested submenu reusing case. I found that when submenu is used several times - each time a copy is created. So will it be correct to create a field in concrete MenuDefinition using createStaticSubMenu method?

Yes, that is correct.

I have some difficulty. I worked on sub-menu reference parsing and conversion. Have a look at the simple test-case:

define sub-menu sm
   menu-item Mi label "Forward".

define menu mbar menubar
  sub-menu sm label "SM Submenu".     <-- sm - is a "reference" 

I enclosed the word "reference" with quotas because sm is not really a reference it is a copy of defined sub-menu. That peculiarity is a source of my difficulty. I tried to add javaname annotation to sm and some others

At this moment an AST corresponding to sub-menu "reference" looks like this:

    <ast col="3" id="665719930906" line="5" text="sub-menu" type="KW_SUB_MENU">
       <annotation datatype="java.lang.Long" key="tempidx" value="269"/>
       <ast col="12" id="665719930908" line="5" text="sm" type="SYMBOL"/>
       <ast col="15" id="665719930911" line="5" text="label" type="KW_LABEL">
         <ast col="21" id="665719930913" line="5" text="&quot;SM Submenu&quot;" type="STRING"/>
       </ast>
    </ast>

I've just added sym.annotateVariableOptions(..) call to submenu_descriptor rule in progress.g to add tempidx annotation, that would be replaced with refid annotation and others in fixups/post_parse_fixup.xml. There is the place where tempidx is replaced with corresponding annotations depending on AST type: definition or reference.

So, I don't know what to do here. sm is a copy of sub-menu, defined earlier. It probably more definition than reference, but it is also differs from definition because all menu-items are defined in a real definition, although several sub-menu options could be set here.

Could you advice me something.

P.S. An AST without any annotations looks like this:

    <ast col="3" id="665719930906" line="5" text="sub-menu" type="KW_SUB_MENU">
       <ast col="12" id="665719930908" line="5" text="sm" type="SYMBOL"/>
       <ast col="15" id="665719930911" line="5" text="label" type="KW_LABEL">
         <ast col="21" id="665719930913" line="5" text="&quot;SM Submenu&quot;" type="STRING"/>
       </ast>
    </ast>

P.P.S. submenu_descriptor rule looks like this (without my changes):

submenu_descriptor
   :
      // symbol => this is a placeholder for menu namespace processing
      KW_SUB_MENU^ s:symbol (KW_DISABLED)? (label)?
   ;

I didn't fully understand specified comment. What does it mean? I expected that there are possible only on form of sub-menu reference: sub-menu sm .... How a menu namespace could be specified here?

Progress has a separate namespace for menus. That means that sm as a menu/sub-menu is different from sm as a variable. For this reason, we keep a separate namespace for menus in SymbolResolver.

When a new named, static menu, sub-menu or menu-item is DEFINEd in 4GL code, we must call one of SymbolResolver.addMenu() (for menus and sub-menus) or SymbolResolver.addMenuItem() for menu-items.

When a menu, sub-menu or menu-item is referenced in 4GL code (e.g. for sub-menus it is the submenu_descriptor rule), we need to call SymbolResolver.lookupMenu() or SymbolResolver.lookupMenuItem(). Right now it is only setup to tell the caller the token type of the resource. That actually is not very useful. More useful would be to provide a mechanism to annotate the original menu/sub-menu's tempidx into the menu/sub-menu reference so that they can be linked later in fixups/post_parse_fixup.xml.

You will have to make changes to the parser, the SymbolResolver and to fixups/post_parse_fixup.xml to get this to work.

I previously put in the comment // symbol => this is a placeholder for menu namespace processing because I had not yet implemented all this parse-time linkage processing although I did put in some basic dictionary support into the SymbolResolver. I is up to you to complete that work.

Greg Shah wrote:

Progress has a separate namespace for menus. That means that sm as a menu/sub-menu is different from sm as a variable. For this reason, we keep a separate namespace for menus in SymbolResolver.

When a new named, static menu, sub-menu or menu-item is DEFINEd in 4GL code, we must call one of SymbolResolver.addMenu() (for menus and sub-menus) or SymbolResolver.addMenuItem() for menu-items.

When a menu, sub-menu or menu-item is referenced in 4GL code (e.g. for sub-menus it is the submenu_descriptor rule), we need to call SymbolResolver.lookupMenu() or SymbolResolver.lookupMenuItem(). Right now it is only setup to tell the caller the token type of the resource. That actually is not very useful. More useful would be to provide a mechanism to annotate the original menu/sub-menu's tempidx into the menu/sub-menu reference so that they can be linked later in fixups/post_parse_fixup.xml.

You will have to make changes to the parser, the SymbolResolver and to fixups/post_parse_fixup.xml to get this to work.

I previously put in the comment // symbol => this is a placeholder for menu namespace processing because I had not yet implemented all this parse-time linkage processing although I did put in some basic dictionary support into the SymbolResolver. I is up to you to complete that work.

I understand and I already did it. I just asked about post_parse_fixup.xml step and how to interpret sm reference. Because it is not a reference and is not a definition of sub-menu. It defines of what annotations I should add: refid or widdef or some others. I also need to add javaname.

I just asked about post_parse_fixup.xml step and how to interpret sm reference. Because it is not a reference and is not a definition of sub-menu.

Anytime we need to be able to look up the original node that defined a resource, we leave behind a tempidx/refid link as an annotation. If you need to be able to get back to that node (for example, to lookup the javaname) then you will need a refid. Even if these references are copies, they still are close enough to references to use the same trick.

It defines of what annotations I should add: refid or widdef or some others. I also need to add javaname.

There is no standard for what you have to do. Generally, the javaname will be created on the node that defined the resource. You don't want to calculate the same name conversion in many places, because otherwise we can't properly override the name conversion with hints. To the extent that you need the classname of the sub-menu definition java file, you want to get that from the original definition too. So it seems like you need to treat this like a reference and make sure you have a refid.

Add the other annotations if you need them.

Code Review vig_upd20141128a.zip

The changes are pretty good.

1. As note 42 documents, KW_MENU_ITM is matched in 3 possible ways in the parser. That means that a simple type == prog.kw_menu_itm is not safe to use. variable_definitions.rules line 106 needs to be more specific.

2. About your questions:

Actually I don't fully understand what the difference between WID_SUB_MENU and KW_SUB_MENU or some other WID_* and KW_* tokens. I only found that WID_* tokens are used primarily in SymbolResolver and KW_* tokens are primarily used in conversion rules.

The KW_SUB_MENU is a token that has matched the text "sub-menu" or "SUB-MENU" or "SuB-meNu". It can only occur in certain locations in 4GL syntax, like DEFINE SUB-MENU. Anything that is a 4GL keyword will create/match a KW_* token.

A WID_SUB_MENU is a specific reference to a named sub-menu widget. It is created from a SYMBOL node in a construct that always looks like SUB-MENU my-sub-menu-name where the SUB-MENU text is a KW_SUB_MENU and the my-sub-menu-name text is a SYMBOL. The KW_SUB_MENU is made into the root node and the SYMBOL child should have its token type converted from SYMBOL to WID_SUB_MENU. So the WID_SUB_MENU node would never have the text "SUB-MENU", but its text would be a user-defined sub-menu name.

KW_* nodes are 4GL syntax keywords.

WID_* nodes are user-defined names that reference widget instances. These are most often used for widgets that aren't implicitly created by a DEFINE VARIABLE statement (buttons, frames, browses, rectangles, menus, sub-menus, menu-items...). These widget references are meant to be made more distinguishable any other SYMBOL instances by changing the token type to be a specific widget type.

In progress.g, your mistake is in submenu_descriptor where you should change #d.setType(WID_SUB_MENU); to #s.setType(WID_SUB_MENU); so that the SYMBOL node (which is the actual reference to the sub-menu) can be easily distinguished as a sub-menu reference. Using #d changes the root KW_SUB_MENU node which should not be changed since it is not the real sub-menu reference.

That rule is intended to replace tempidx annotation with refid annotation for widget references. This rule calls widget_reference function, that checks AST type and returns true only for WID_* tokens. That means that rule works only with WID_* tokens. That is why I had to replace KW_SUB_MENU with WID_SUB_MENU type in a grammar.

Since we are linking menu/sub-menu/menu-item references similar to how we are doing it for buttons, I think you would allow all 3 of those types to be processed by that section from post_parse_fixups.xml:

        <!-- identify all var references and convert tempidx to real using
              the dictionary, exclude widgets that can't be frame fields
              (and thus are not backed by a variable or are not cross-ref'd
              otherwise like button or browse) -->
         <rule>
            !(type == prog.wid_dialog   or  type == prog.wid_frame    or
              type == prog.wid_window)   and
            (evalLib("variables") or evalLib("widget_references"))      and
            type != prog.sys_handle                                     and
            isNote("tempidx")                                           and
            dictionaryContains("var_defs", string(getNoteLong("tempidx")))
            <action>
               putNote("refid",
                       lookupDictionaryLong("var_defs",
                                            string(getNoteLong("tempidx"))))
            </action>
            <action>removeNote("tempidx")</action>
            <action>fprintf("post_parse_fixups.log",
                            "VAR_REF  %-40s: %-25s @ %05d : %05d\n",
                            file,
                            this.getSymbolicTokenType(),
                            line,
                            this.getColumn())
            </action>
         </rule>

The idea is that we are adding these things into their own namespace (in SymbolResolver) and using the "DEFINE VARIABLE" approach to linking tempidx into a refid. By changing the SYMBOL to WID_SUB_MENU, this will still work.

I also don't fully understand what lvalue is. Why it is used in menu_reference and not used in submenu_descriptor. I suspect that I should used lvalue there and it would be better solution than the current. What do you think?

lvalue is a term often used in language specifications. It refers to the concept that resources that represent memory locations are assignable and the item being assigned is on the left side of the assignment operator. It is thus a left value or lvalue.

In most languages, the syntax is quite simple and there are only "variables" which can be used as lvalues.

In the 4GL, they a very large number of separate resources that can be used as an lvalue, each with their own namespace. In addition, they have other non-resource constructs that can appear on the left side of an assignment operator (see assign_type_syntax_stmt_list). In the end, the lvalue rule is used for matching references to all these different kind of resources even if it is just a reference in the middle of a sub-expression somewhere. It doesn't only get used on the left side of an assignment operator. It is used as a "general purpose resource reference".

As to your idea that we could use lvalue inside the submenu_descriptor, I think it is a good idea. It would allow us to re-use common code. BUT we must make sure that the code can only match a KW_SUB_MENU in that location (and NOT all the other things that can be matched by lvalue). So I recommend something like this:

submenu_descriptor   
   :
      (
           { LA(1) == KW_SUB_MENU }?
           lvalue
           (KW_DISABLED)?
           (label)?
         | // empty alternative
      )
   ;

I haven't tested this, so I'm not sure it will work.

By the way, I think that the menu_reference will probably not "link" properly since it has no tempidx annotation. Hopefully menu/sub-menu/menu-item references through lvalue will properly link.

I added SKIP and RULE support to menu conversion rules. It works. SkipEntity and correspondent class for RULE don't have setParent(menu) methods. So I had to add WidgetList functionality to menu and sub-menu definitions: I had to derive MenuDefinition and SubMenuDefinition classes from WidgetList and generate addWidget's block to generated classes as it goes in a frame definitions.

2 difficulties.¶

1. Frame definition contains initialization block, that looks like this.

{
  addWidget(..)
  addWidget(..)
  ..
}

2. WidgetList is expecting to contain GenericWidget subclasses. MenuDefinition or SubMenuDefinition subclasses can contain SkipEntity, RuleEntity, MenuItemWidget and SubMenuDefinition. The last one is not a GenericWidget and cannot be added to WidgetList. I had to add a SubMenuWidget self field to SubMenuDefinition class and add that field to WidgetList in case of sub-menu. It looks not good. What do you think?

1. Frame definition contains initialization block, that looks like this.
[...]
This block resides right in the inner class after setup method. I didn't find a way how to add this block and I had to add addWidget calls right in setup method.

This was done by adding a BLOCK node to the CS_STATIC_INITS section of the inner class in the JAST.

          <ast col="0" id="10209137262621" line="0" text="" type="CS_STATIC_INITS">
            <ast col="0" id="10209137262622" line="0" text="block" type="BLOCK">
              <ast col="0" id="10209137262662" line="0" text="addWidget" type="STATIC_METHOD_CALL">
                <ast col="0" id="10209137262663" line="0" text="expr7" type="STRING"/>
                <ast col="0" id="10209137262664" line="0" text="" type="STRING"/>
                <ast col="0" id="10209137262665" line="0" text="expr7" type="REFERENCE"/>
              </ast>
            </ast>
          </ast>

If I recall correctly, the reason it these addWidget() calls were not put in the setup() method was because they needed to exist much earlier in the initialization process.

Do we need the same thing for menus? If not, then leave the calls in setup().

2. WidgetList is expecting to contain GenericWidget subclasses. MenuDefinition or SubMenuDefinition subclasses can contain SkipEntity, RuleEntity, MenuItemWidget and SubMenuDefinition. The last one is not a GenericWidget and cannot be added to WidgetList. I had to add a SubMenuWidget self field to SubMenuDefinition class and add that field to WidgetList in case of sub-menu. It looks not good. What do you think?

Actually, I think it is fine. We do a similar thing with GenericFrame which implements both CommonFrame and the specific frame definition interface but which is NOT a sub-class of GenericWidget. GenericFrame has a frame member that is the FrameWidget and which is accessed via asWidget() when needed.

On the other hand, we don't have the same complexity of frames (we have no frame definition interface that is implemented differently for each frame), so I wonder if we can improve this.

Assignment assign current-window:menubar = menu m:handle. is converted to currentWindow().unwrapWindow().setMenubar();. I didn't find the target rule to change to add parameter to setMenubar method.

You get the parameter processing "for free". You don't have to do anything to make that work. The problem here is that you have to write rules to make the MENU menu, SUB-MENU sub-menu and MENU-ITEM menu-item references emit properly. See convert/widget_references.rules. When that is done, they will emit naturally into the right parameter location. You will need some additional annotations to be created to record the proper data member references and accessor methods. Those should be done during annotations processing.

Those should be done during annotations processing.

Actually, that is not right. You can probably leave behind the needed annotations during menu_generator.xml processing.

Code Review vig_upd20141202a.zip

This is great! You are very close now.

1. In WindowWidget, the menubar should not be stored as a handle but instead as a MenuWidget. The getMenuBar() method would just have return new handle(menubar); as the body. The setMenuBar() would just store the contained resource instance instead of the handle itself. In addition, the menubar instance should be stored in the WindowConfig instead of WindowWidget itself.

2. It seems heavyweight to treat rule as a widget, but I suspect that it is the easiest/best thing to do. It is something that can't be seen as a widget by the calling code right? It is never named and cannot be obtained via traversal. It has no configuration and no attributes/methods/events. For this reason, at least minimize the widget features used. It doesn't need its own LegacyResource. It probably does need a widget ID at least.

Greg, What are you writing about in the point 2 in previous note?

What are you writing about in the point 2 in previous note?

I am saying that RULE is not a real widget in 4GL terms. For this reason, RuleEntity and RuleConfig seem like too much of a solution. On the other hand, if the generated code and runtime code is simpler by treating it as a widget, then we can do so. You have to make a decision on that.

Greg, I got stuck a little. I'm trying to convert menu handle parameter to setMenubar call for the following statement assign current-window:menubar = menu m1:handle.. I.e. I'm trying to convert menu m1:handle to parameter. Here is an AST tree for the right part of that assingment:

<ast col="0" id="665719930953" line="0" text="expression" type="EXPRESSION">
            <ast col="40" id="665719930954" line="11" text=":" type="COLON">
              <ast col="33" id="665719930955" line="11" text="menu" type="KW_MENU">
                <ast col="38" id="665719930958" line="11" text="m1" type="WID_MENU"/>
              </ast>
              <ast col="41" id="665719930960" line="11" text="handle" type="ATTR_HANDLE">
                <annotation datatype="java.lang.Long" key="oldtype" value="1460"/>
              </ast>
            </ast>
          </ast>

        <!--
            Doesn't work... Why?
                type == prog.attr_handle          
                evalLib('methods_and_attributes')                 and
         -->

         <rule>
              this.prevSibling.firstChild.type == prog.wid_menu and
              this.prevSibling.type == prog.kw_menu             and
              this.prevSibling != null                          and
              parent.type == prog.colon 

               <action>
                  lastid = createPeerAst(java.constructor, "handle", closestPeerId)
               </action>
               <rule>this.prevSibling != null and this.prevSibling.firstChild != null
                  <action on="true">owner = this.prevSibling.firstChild.text</action>
                  <action on="false">owner = "temporary_value"</action>
               </rule>
               <action>
                  lastid = createJavaAst(java.reference, owner, lastid)
               </action>
          </rule>

The questions.
1. My attempt is intuitive and I'm not sure that the place and approach are correct. Aren't they?
2. AST node of type ATTR_HANDLE exists in source tree as you could see. I tried to write my rule for that node, but It'd failed. If I was adding the condition type == prog.attr_handle my actions was not being executed. As you can see the condition this.prevSibling != null and this.prevSibling.firstChild != null is also false ("temporary_variable" owner was used). Why?

Could you help me please. Here is 2 aspects. My rule does not use the common mechanism. It's about 2 aspects:
a) defining of "owner" i.e. defining a menu in my case.
b) working with handle attribute.
How to do that correctly and where?

Forgot to add another one question. As you could see the place where new parameter was appeared is also wrong:
it must appear in setMenubar() method, but appeared in currentWindow(). It seems closesPeerId has unexpected value (unexpected for me). How to fix it? Can I get closesPeerId ast siblings for that purpose?

I found widget_reference.rules and tried to add the following rules

As I mentioned in note 56 above, you just need to focus on getting the WID_MENU, WID_SUB_MENU and WID_MENU_ITM to emit naturally.

I see you tried to use evalLib('methods_and_attributes'). evalLib() and execLib() are our ways of invoking named functions. You cannot execute entire rule-sets using those methods. On the pattern-engine processing executes entire rule-sets.

But you don't need to do such a thing anyway. The methods_attributes.rules is already getting executed as part of core_conversion.xml. So you can let it properly process. All of the rule-sets listed in core_conversion.xml are executing sequentially for each file. Cooperatively, this results in building a complete JAST. But you must let each piece do its own part.

DO NOT worry about the COLON or ATTR_HANDLE processing.

Consider this code from widget_references.rules:

      <!-- convert anything that could be a widget reference -->
      <rule>
         evalLib("variables") or
         evalLib("fields")    or
         evalLib("widget_references")

         <!-- only process if this is a frame field -->
         <!-- widget handle is emitted via 'asWidget()' method  -->
         <rule>isNote("frame-id") 
            <action>methodType = java.method_call</action>

            <rule>type == prog.var_handle
               <action on="false">
                  owner = #(java.lang.String) execLib("get_framename", this)
               </action>
               <action on="false">
                  methodTxt = getNoteString("accessor")
               </action>
            </rule>

The evalLib("widget_references") will evaluate true for WID_MENU, WID_SUB_MENU and WID_MENU_ITM, right? Good, that is what we want.

The isNote("frame-id") will then evaluate false, right? With menus, we don't have a "frame-id" annotation and we probably don't need or want one. But we may need to do something else here to identify those WID_MENU, WID_SUB_MENU and WID_MENU_ITM cases that need to emit into the business logic. The "frame-id" is how we do it for frame widgets. Perhaps all WID_MENU, WID_SUB_MENU and WID_MENU_ITM nodes should always emit. You have to determine that. But we probably need a new section in widget_references.rules that handles menus. I doubt that we want the menus dropping through the current code because it is designed for frames and frame widgets.

Once past the "frame-id" test, most WID_* cases will execute methodTxt = getNoteString("accessor") which is the method call text that will return the widget from the frame. As I stated in note 56:

You will need some additional annotations to be created to record the proper data member references and accessor methods.

During menu_generator.xml, you MUST leave behind enough information in the WID_* nodes to allow them to emit the accessor call.

In addition, where the frame reference gets emitted today, you will have to do something similar to emit the menu instance so that the accessor will be called on the right object.

After conversion process I got the following code currentWindow(new handle(temporary_value)).unwrapWindow().setMenubar();, but I expected that one currentWindow().unwrapWindow().setMenubar(new handle(m1));

Your approach is incorrect. Instead please focus on my advice above and below. Some examples of mistakes you are making:

• You are processing at the COLON and ATTR_HANDLE nodes when you simply need to do your widget_references.rules processing at the WID_* node itself.
• Your code is misunderstanding the idea of "peerid".
• You have no annotations left behind during menu generation to allow these nodes to know the menu instance name (for WID_MENU) or menu instance name and accessor (for WID_SUB_MENU and WID_MENU_ITM).

Peer IDs are used to allow CHILD NODES to emit UNDERNEATH their parent's peer node. A peer node is a JAST node and a "peerid" annotation allows that node to be found. The most common way is through closestPeerId which searches up the AST parent hierarchy until an ancestor is found with a "peerid" annotation. That peerid is then used to get the JAST node that will be the parent for the newly created JAST node. createPeerAst() creates a new JAST node and saves its ID as the peerid annotation for the CURRENT NODE (copy). createJavaAst() should be used when this peering behavior is not appropriate either because the peerid is not wanted or because you are processing at a different node than where the peerid should be placed.

AST node of type ATTR_HANDLE exists in source tree as you could see. I tried to write my rule for that node, but It'd failed. If I was adding the condition type == prog.attr_handle my actions was not being executed. As you can see the condition this.prevSibling != null and this.prevSibling.firstChild != null is also false ("temporary_variable" owner was used).

Please avoid all this. Just focus on the WID_* nodes.

I hope this answers all your questions. If not, please ask what is still confusing.

Greg Shah wrote:

..
I see you tried to use evalLib('methods_and_attributes'). evalLib() and execLib() are our ways of invoking named functions. You cannot execute entire rule-sets using those methods. On the pattern-engine processing executes entire rule-sets.
..

I just wanted to call the function 'methods_and_attributes' in include/common-progress.rules not the entire rule-set. This function would pass my ATTR_HANDLE node. I tried to wrote my rule exactly for ATTR_HANDLE. Now I understand that it was wrong.
--

Thanks for good description! Now I understand that I need to emit annotations for each MENU-ITEM and SUB-MENU, containing the names of parent SUB-MENU (if exists) and parent MENU. I also need to check if WID_* nodes are emitted in all cases of usage: definitions and references.

I tried to add such annotation called container-id but didn't use it before. You advised me to add annotations right in menu_generator.xml. I added container-id before in fixups/post_parse_fixups.xml. Now I think that menu_generator.xml is better place because fixups/post_parse_fixups.xml is too early (there was no necessity in such annotations in menu_generator at until now). Right? Now I see that I'll probably need 2 separate annotations: "menu" and "sub-menu" instead of common annotation "container-id".

About "peerid" idea. Really good idea. In my task what would be the target nodes to add peerid annotations? How about JAST that corresponds to setup method? I used setUpLastId variable for that in menu_generator.xml. Now can I get rid of it and use "peerid" in menu_generator.xml instead?

Relating to widget_references.rules lets consider little Progress sample: menu-item mi in sub-menu sm. That could should be converted to that one sm.mi in Java class. So should I add peerid for a JAST node, that corresponds to sm, and use closestPeerId when emitting a JAST node for mi. Did I understand correctly?

Now I think that menu_generator.xml is better place because fixups/post_parse_fixups.xml is too early (there was no necessity in such annotations in menu_generator at until now). Right?

Yes, that is right. While you might be able to link the IDs up, the accessor and menu instance names can only be known in menu_generator.xml.

In my task what would be the target nodes to add peerid annotations?

For the WID_* you probably don't need peerid because they won't have any child nodes.

How about JAST that corresponds to setup method? I used setUpLastId variable for that in menu_generator.xml. Now can I get rid of it and use "peerid" in menu_generator.xml instead?

I don't think so. The setup() method doesn't have a counterpart in the Progress AST. So there is no place to put the peerid annotation to make it work. Even if there was a 4GL peer AST for the annotation, that AST would have to be the direct parent of all the options that are being emitted as method calls. Peerid helps where the natural structure of the tree needs to be the same (or very close) between both the 4GL and Java. Then peerid causes the natural tree structure to be duplicated.

lets consider little Progress sample: menu-item mi in sub-menu sm. That could should be converted to that one sm.mi in Java class. So should I add peerid for a JAST node, that corresponds to sm, and use closestPeerId when emitting a JAST node for mi. Did I understand correctly?

Close, but not quite right. Actually, the JAST needs to have a parent node that is type java.member for mi and a child node of type java.reference for sm:

         <ast id="0" type="MEMBER" text="mi">
            <ast id="0" type="REFERENCE" text="sm" />
         </ast>

When brew.xml emits this (a.k.a. it anti-parses this into Java source code), it operates depth-first, so the REFERENCE is emitted and then a . and finally the MEMBER.

This means that the mi could have the peerid. But this would only work if the IN SUB-MENU sm was the child node of the MENU-ITEM mi. I think it might be, so this case might work.

How about menu handle? I need that menu m1:handle will be converted to new handle(m1). Where it happens with frame? asWidget only returns FrameWidget and asWidgetHandle is not mention in rules at all.

This is a good question. The answer is that we don't actually create a new handle() constructor for the ATTR_HANDLE with oldtype == prog.kw_handle.

Consider this code:

def var h as handle.

def var i as int.
def frame f i.

i = 30.
display i with frame f.

def temp-table tt field num as int.
def buffer b for tt.

h = frame f:handle.
message "ROW = " + string(frame f:row).
message "FRAME NAME = " + frame f:name.

h = buffer b:handle.
message "BUF NAME = " + buffer b:name.

Here is how it converts (just the last lines that we care about):

            h.assign(fFrame.asWidget());
            message(concat(new character("ROW = "), valueOf(fFrame.getRow())));
            message(concat(new character("FRAME NAME = "), fFrame.name()));
            h.assign(b);
            message(concat(new character("BUF NAME = "), b.name()));

No constructors!

This works because when it is a handle type that is being assigned, the handle.assign(WrappedResource) is used. So we avoid wrapping here and just pass the instance itself.

And when we call a 4GL method or read and attribute we would have to unwrap the resource anyway, so we don't ever wrap it. The only time we actually unwrap a handle is when it is the handle that is being dereferenced, as in: h:name.

For attributes like MENUBAR that can be assigned from a handle, make sure you create multiple setter methods:

void setMenuBar(handle mb)
{
}

void setMenuBar(MenuWidget mw)
{
}

This way it can be used like this:

my-handle = menu my-menu:handle.
current-window:menubar = menu my-menu:handle.  /* this uses setMenuBar(MenuWidget mw) */
current-window:menubar = my-handle.            /* this uses setMenuBar(handle mb) */

Code Review vig_upd20141206a.zip

1. WindowWidget.setMenubar(handle) should check for null and unknown can take the same action as the 4GL would in that case. Then if it is not null or unknown, then it should get the contained MenuWidget and call setMenubar(MenuWidget) to have common code that does the assignment/setOwner() call.

2. Let's not create multiple methods that do the same thing. The 4GL does this and it is not great. For example, they have DISABLED as an option and well as SENSITIVE as an attribute. If DISABLED is the same as SENSITIVE=FALSE then let's convert it to setSensitive(false) and avoid the extra setDisabled() method.

3. Shouldn't mnemnonic, readOnly, toggleBox and accelerator be in the MenuConfig? Having them in the widget itself is not correct.

4. I'm not sure why frame_generator.xml has -java.reference in the litMap. I see that you have copied it. Can you say what purpose it serves? I also think it may be time to make the core logic in literals.rules into common code that can be accessed from menu_generator.xml, frame_generator.xml and literals.rules (in core_conversion.xml). Duplicating all that logic is not good.

5. The use of downPath() and then the de-referencing of children for all the option setters doesn't seem right. First, it is fighting the tree structure instead of letting it help you. It is much better to match directly on the node being converted and emit that into the right location using createPeerAst(). And then handle the children separately on their own and let them emit naturally into closestPeerId. The resulting code will be MUCH easier to write, read and maintain. The frame_generator.xml is a very bad design to copy, which is why I was wanting to avoid using it as a "template". Of course in such an approach, all that code would not be inside the type prog.kw_menu_itm condition. But it might be inside an parent != null && parent.type prog.kw_menu_itm and (parent.parent.type prog.define_menu or parent.parent.type prog.define_sub_menu).

6. In widget_references.rules, this looks wrong: <!-- variables -->/l (the /l at the end is junk).

7. Your question:

Although I added relativePath("KW_MENU_ITM/KW_DCOLOR/EXPRESSION/NUM_LITERAL") to literals.rules the values are still presented in the body method. Why it could be?

I'm not exactly sure, but I can say that in frame_generator.xml, the literals that are emitted for frame and widget setters/options are then hidden so they don't emit into the business logic. But I would really prefer an annotation that bypasses processing by literals.rules instead of calling setHidden(true) on the literal node.

Greg, I have some difficulties with dynamic menus.
1. I missed one attribute in MenuItemWidget called "SUBTYPE". It has character type and can contain one of following constants: "NORMAL", "SKIP", "RULE". This attribute is used mostly in dynamic scenarios. It can mean that we don't have to use SkipEntity and RuleEntity and MenuWidget and SubMenuWidget can contain only MenuItemWidget@s. I.e. when the source procedure contain RULE or SKIP I need to create @MenuItemWidget and set SUBTYPE attribute. There are possible two ways:
a) correct implementation rules to support only MenuItemWidget as a children of MenuItemWidget or SubMenuWidget.
b) In dynamic cases treat the constructs, creating MENU-ITEMs with non-normal SUBTYPEs as creation of SkipEntity or RuleEntity
What way will be preferrable?

2. If I remember correctly the following constructs CREATE ... ASSIGN .. was not fully supported by conversion rules. Should I check this variant too?

• TITLE attribute of the Menu
• LIKE clauses

The question about shared menus. The problem is in try to generate menu definition second time when DEFINE SHARED MENU is met. I didn't find any special processing of that case in frame_generator.xml. I.e. I didn't find any check if current node corresponds to DEFINE SHARED FRAME statement and related frame definition is already generated. So I can conclude that it works only by name, i.e. when frame definition is generated its name is saved in a special map. At the next try to generate frame definition for the frame with the same name we can check this map and bypass generation for that node. I doubt that it really works by name. Don't you remember how shared frame definition generation really works? I.e. how it bypasses generation for the shared frame?

I am working my way through the pending updates and questions.

While I am doing that, I wanted to answer your question about shared menus.

Any NEW SHARED menu is one that needs to be converted into a menu definition. Any menu that is just SHARED but has no NEW is one that is created elsewhere than then "imported" into the current program.

Please convert these two programs to get the idea:

shared-var-creator.p

define new shared variable my-var as int init 14.

run shared-var-importer.p.

shared-var-importer.p:

define shared var my-var as int.

message my-var.

You will see the references to the SharedVariableManager in the resulting Java code. The creator program instantiates the variable, initializes it and stores a reference to it (by its legacy 4GL name) in the SharedVariableManager. The importer just obtains that reference using the same name and then can use it as a local member of the class. But the importer does not instantiate a new var.

I think menus will work the same way.

So I can conclude that it works only by name, i.e. when frame definition is generated its name is saved in a special map. At the next try to generate frame definition for the frame with the same name we can check this map and bypass generation for that node. I doubt that it really works by name. Don't you remember how shared frame definition generation really works? I.e. how it bypasses generation for the shared frame?

Search for prog.kw_shared in annotations/frame_scoping.rules. Also look there for how the frameShr var is being used to create "shared" and "imported" annotations. Those are used downstream in convert/frame_construction.rules@.

If I recall correctly, imported shared frames are still generated as separate frames. I think it was done because the frame definition can be slightly different between the two locations. Write some simple shared frame testcases (new shared and just shared) to see exactly how this works.

Then you must determine if we have a similar problem with menus. Of course, we would prefer to deal with menus like variables but we must match the 4GL's behavior.

1. I missed one attribute in MenuItemWidget called "SUBTYPE". It has character type and can contain one of following constants: "NORMAL", "SKIP", "RULE". This attribute is used mostly in dynamic scenarios. It can mean that we don't have to use SkipEntity and RuleEntity and MenuWidget and SubMenuWidget can contain only MenuItemWidget@s. I.e. when the source procedure contain RULE or SKIP I need to create @MenuItemWidget and set SUBTYPE attribute. There are possible two ways:
a) correct implementation rules to support only MenuItemWidget as a children of MenuItemWidget or SubMenuWidget.
b) In dynamic cases treat the constructs, creating MENU-ITEMs with non-normal SUBTYPEs as creation of SkipEntity or RuleEntity
What way will be preferrable?

Interesting. I see that the SUBTYPE attribute is writable, which means that something that starts as one type can be changed later. That means that option A above is the only proper way to do this. That means you eliminate the SkipEntity and RuleEntity. And you implement these based on the MenuItemWidget SUBTYPE attribute.

In menu definitions, the widgets are defined statically, so it seems you can pass a SUBTYPE enum value to the MenuItemWidget constructor to initialize the correct subtype. By default, the subtype should be NORMAL so you can possibly avoid passing that enum value in most of the time.

2. If I remember correctly the following constructs CREATE ... ASSIGN .. was not fully supported by conversion rules. Should I check this variant too?

I think this is supported. It definitely works for frames. Please do check this too.

But I have one difficulty here: literals was removed but semicolons are stayed. Could you help me?

I suspect that there is some hidden node that exists which is not suppressed. For example, there may be an EXPRESSION node which then generates something in the JAST that causes the semicolon to emit because it thinks something is there.

WINDOW attribute exist for all three widgets but I think it's a common attribute for all widgets and should be added in GenericWidget for example. I didn't added it yet. What do you think?

Yes, it should be present in CommonWindow and implemented in GenericWidget since it is a valid attribute for all widgets in the 4GL (except SHADOW-WINDOW which we don't support yet).

Unfortunately I can't extract litMap with them. It didn't work I can't find why. So I had to add new parameter "litMap" to these functions.

This was using a bad practice of accessing variables from the containing scope. When you moved the function to a different rule-set, it changed the scope of the function making the litMap unavailable. Passing it as a parameter is the correct approach.

The code review is next.

Code Review vig_upd20141213a.zip

I generally like the changes.

1. Removing the on="false" from line 511 of literals.rules is incorrect. I think that code is required.

2. I don't think menu_construction.rules needs the litMap stuff. It is called during core conversion, where the literals.rules is co-executing directly.

3. I'm not sure if making MENUBAR and POPUP-ONLY synonymous is correct, especially since POPUP-ONLY probably generates an error in ChUI and MENUBAR probably does not. Did you test that?

4. Did you test if it was safe to make SENSITIVE and DISABLED synonymous? A test would be to create a DISABLED menu item and then see if setting SENSITIVE to true causes the item to be usable.

5. Why is setDateOrder() added to SessionUtils?

6. The ServerImpl methods should not have the LegacyAttribute annotations. In addition, they should call handle.readOnlyError() instead of throwing their own RuntimeException.

I've tested shared menus. As far as I can understand there are one instance of menu is used for created shared menu and for shared menu. I changed labels in external procedure for shared menu created in main procedure. It was changed. But I couldn't create one popup menu and assign it for 2 buttons. I got the error: "**OWNER/PARENT exists on the BUTTON btn1. Unable to set attribute POPUP-MENU. (4074)".

I changed labels in external procedure for shared menu created in main procedure. It was changed.

Does this mean that when it is used in the called procedure that the labels are different than when it is used in the calling procedure?

But I couldn't create one popup menu and assign it for 2 buttons. I got the error: "**OWNER/PARENT exists on the BUTTON btn1. Unable to set attribute POPUP-MENU. (4074)".

This is understandable. I assume it works if you don't assign it to something in the calling procedure (at least until after it is done being used in the called procedure)?

Greg Shah wrote:

I changed labels in external procedure for shared menu created in main procedure. It was changed.

Does this mean that when it is used in the called procedure that the labels are different than when it is used in the calling procedure?

I only mean the following. A menu is defined in calling procedure with several labeled menu-items. In called procedure labels are the same - all is the same, but I can change them in called procedure and I able to see the new labels in shown menu.

From the other point of view I can make the following conclusions. In CHUI we have only one WINDOW, therefore we can have only one MENUBAR. In my tests this menubar is shared between 2 procedures and I can change it in both of them. It confirms that both procedures are working with one object. Another test is about POPUP menu. Such shared menu is assigned as POPUP-MENU for some button in calling procedure. When I'm trying to assign this shared menu in called procedure to another button I'm getting an error that menu already has a PARENT/OWNER. It also confirms that both procedures are working with one instance of menu.

But I couldn't create one popup menu and assign it for 2 buttons. I got the error: "**OWNER/PARENT exists on the BUTTON btn1. Unable to set attribute POPUP-MENU. (4074)".

This is understandable. I assume it works if you don't assign it to something in the calling procedure (at least until after it is done being used in the called procedure)?

Yes, in this case I will be able to assign it in called procedure.

Another one thing. As I could understand only MENU can be shared. MENU-ITEMs or SUB-MENUs - cannot be shared. Moreover, when I'm trying to call some MENU-ITEM from shared MENU in called procedure - it dont' work:
menu-item mi in menu m will raise an error that mi is not found. I only can get a MENU-ITEM or SUB-MENU of a shared MENU using CHILD/SIBLING attributes. It looks strange.

1. Such objects are emitted as fields of menu (or sub-menu) definition class (MenuDefinitionM) without access modifier (i.e. default). The procedure class (References in my example@) is generated in a package different from MenuDefinitionM package. In my simple case the References is generated in the package com.goldencode.testcases.menu and MenuDefinitionM is generated in com.goldencode.testcases.ui.menu. It means that our default fields will be unaccessible in a main procedure for references. What to do? May be I should emit public access modifier?
2. If I want to assign some MENU m attribute, for example title, I'm emitting m.getSelf().setTitle(..) at this moment. But now I need to emit reference to menu-item of our menu: menu-item mi in menu m for example. I'll probably need to emit m.mi instead of m.getSelf().mi because mi is a field of MenuDefinition not of MenuWidget. The difficulty is how to distinguish these 2 cases.

I changed labels in external procedure for shared menu created in main procedure. It was changed.

Does this mean that when it is used in the called procedure that the labels are different than when it is used in the calling procedure?

I only mean the following. A menu is defined in calling procedure with several labeled menu-items. In called procedure labels are the same - all is the same, but I can change them in called procedure and I able to see the new labels in shown menu.

Are the changes made in the called procedure also visible in the calling procedure? Can the menu-items/sub-menus be different in each procedure for the same shared menu?

Another one thing. As I could understand only MENU can be shared. MENU-ITEMs or SUB-MENUs - cannot be shared. Moreover, when I'm trying to call some MENU-ITEM from shared MENU in called procedure - it dont' work:
menu-item mi in menu m will raise an error that mi is not found. I only can get a MENU-ITEM or SUB-MENU of a shared MENU using CHILD/SIBLING attributes. It looks strange.

Do all the menu-items and sub-menus defined in the calling procedure still appear and work? If so, I assume they execute triggers in the calling procedure even while the menus are being processed in the called procedure?

Vadim Gindin wrote:

2 questions about MENU-ITEM references or SUB-MENU references.

1. Such objects are emitted as fields of menu (or sub-menu) definition class (MenuDefinitionM) without access modifier (i.e. default). The procedure class (References in my example@) is generated in a package different from MenuDefinitionM package. In my simple case the References is generated in the package com.goldencode.testcases.menu and MenuDefinitionM is generated in com.goldencode.testcases.ui.menu. It means that our default fields will be unaccessible in a main procedure for references. What to do? May be I should emit public access modifier?

Yes, these must be public, which can be done using an annotation in the JAST.

1. If I want to assign some MENU m attribute, for example title, I'm emitting m.getSelf().setTitle(..) at this moment. But now I need to emit reference to menu-item of our menu: menu-item mi in menu m for example. I'll probably need to emit m.mi instead of m.getSelf().mi because mi is a field of MenuDefinition not of MenuWidget. The difficulty is how to distinguish these 2 cases.

Please provide some example 4GL menu reference code that must convert in 2 different ways so that I can get a better idea about the problem.

Greg Shah wrote:
..

Are the changes made in the called procedure also visible in the calling procedure? Can the menu-items/sub-menus be different in each procedure for the same shared menu?

Yes for the first question. No for the second question. I created dynamic menu-item in called procedure and it became visible in calling procedure.
..

Do all the menu-items and sub-menus defined in the calling procedure still appear and work? If so, I assume they execute triggers in the calling procedure even while the menus are being processed in the called procedure?

Yes for the first question. Triggers are work. I didn't understand the last question. What do you mean?

Do all the menu-items and sub-menus defined in the calling procedure still appear and work? If so, I assume they execute triggers in the calling procedure even while the menus are being processed in the called procedure?

Yes for the first question. Triggers are work. I didn't understand the last question. What do you mean?

To make the menu do something useful, one must setup triggers, right?

The place where the menu-items are defined seems like the obvious place where the triggers would also be defined. This is the calling procedure, right?

Since menu items cannot be accessed by name from the called procedure, it seems best to have the triggers in the calling proc.

If you use the menu in the called procedure, does any user selection of a menu item execute the associated trigger in the calling procedure?

Yes, it does.

OK, then it seems shared menus are in fact like shared variables. There is really just 1 instance and "importing" should do the job.

Is there any way to reference an "imported" shared menu in a DEFINE SUB-MENU that is in the called procedure? I guess that would not work, but I want to check to be sure.

I have some difficulty working with references. I've added the following rule to widget_references.rules:

      <rule>type == prog.wid_menu and parent.type == prog.kw_menu
        <rule>parent.parent == null or parent.parent.type != prog.kw_in   
           <action on="true">
               lastid = tw.graft("method_call", null, closestPeerId,
                                 "methodtxt", "getSelf",
                                 "javaname", text).getId()
            </action>
            <action on="false">printfln("TEXT: %s", text)</action>
            <action on="false">printfln("ID: %d", closestPeerId)</action>
            <action> on="false">
               lastid = createJavaAst(java.reference, text, closestPeerId).getId()
            </action>
         </rule>
      </rule>

    <ast col="0" id="751619276865" line="0" text="assignment" type="ASSIGNMENT">
      <ast col="30" id="751619276866" line="10" text="=" type="ASSIGN">
        <annotation datatype="java.lang.Long" key="peerid" value="755914244133"/>
        <ast col="13" id="751619276869" line="10" text=":" type="COLON">
          <ast col="1" id="751619276870" line="10" text="menu-item" type="KW_MENU_ITM">
            <ast col="11" id="751619276873" line="10" text="mi" type="WID_MENU_ITM"/>
          </ast>
          <ast col="14" id="751619276875" line="10" text="label" type="ATTR_CHAR">
            <annotation datatype="java.lang.Long" key="oldtype" value="649"/>
          </ast>
          <ast col="20" id="751619276877" line="10" text="in" type="KW_IN">
            <ast col="23" id="751619276879" line="10" text="menu" type="KW_MENU">
              <ast col="28" id="751619276882" line="10" text="m" type="WID_MENU"/>
            </ast>
          </ast>
        </ast>
        <ast col="0" id="751619276885" line="0" text="expression" type="EXPRESSION">
          <ast col="32" id="751619276886" line="10" text="&quot;MMMM&quot;" type="STRING">
            <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
            <annotation datatype="java.lang.Long" key="peerid" value="755914244135"/>
          </ast>
        </ast>
      </ast>
    </ast>

Returning to the added rule. It is intended for AST of type WID_MENU. As you can see that AST has attribute text="m" (variable name of menu). That is confirmed by printed value in the log. But there is some other value "list0" that is transfered to followed createJavaAst call as a value of attribute text. Moreover, target menu reference is not appear in generated class. If I'll swap conditions with each other: on="false" with on="true" then m.getSelf() will appear in generated class in a correct place. I don't understand why simplest java.reference AST is not added, why the text attribute has a strange value in createJavaAst call. Could you help me?

1. what is the expected converted form for the menu-item mi:label in menu m = "MMMM". code? Is it something like m.menuMi().setLabel("MMMM")?
2. are you sure you are on the correct 4GL AST? You can debug the this.id value to identify it.
3. what is the intent of the parent.parent == null test? Please post the 4GL code which is expected to be treated by it, as it doesn't seem correct.
4. using the text attribute to identify the java var is not correct. You need to identify the menu definition referenced by the WID_MENU AST, and resolve the java name from it. The text attribute always contains the legacy 4GL name, which is not necessarily the java converted name. I think you need some annotations at the KW_MENU_ITM, with the ID of the parent menu, the name of the accessor emitted to access the widget in the converted code (i.e. menuMi), etc. See how a ch:name in frame f1 construct gets converted, menu items should follow a similar approach.

You have a syntax problem with your code <action> on="false">. There is an extra > there.

Also, you can remove the parent.parent null test since this code is only executed when there is a type wid_menu which can never have a null grandparent.

Oh..
Sorry.

Constantin, thank you too. I'll process your remarks.

I'm working with non-standard type of menu-item reference menu-item mi in sub-menu sm. Peculiarity of this reference type is the fact, that the target menu is not specified and is defined implicitly. The sub-menu sm can be a child of several menus and the first menu is used in order of DEFINE MENU statements appearance. Here is an example, illustrating this case.

define sub-menu sm
  menu-item mi label "Source mi label".

define menu m1
  sub-menu sm label "Sm in m1".

define menu m2
  sub-menu sm label "Sm in m2".

menu-item mi:label in sub-menu sm = "New mi label".

The reference menu-item mi:label in sub-menu sm must be converted to the following code sm.mi.setLabel("New mi label"); and this code must appear in menu definition class for m1 - MenuDefinitionM1 only. Not in the procedure class, because there is no sm field and not in both menu definition classes, because of the described behavior.

I'm adding created menu conversion rules from widget_references.rules to menu_generator.xml, but I'm not sure if it is a right way. May be there is a simpler variant: generate full reference in a procedure class:
m1.sm.mi.setLabel("New mi label");. What do you think?

Constantin, about IN ... clauses. I've converted simple test for frames with the trigger

on enter of i in frame f
  message string(frame f:handle).

         <ast col="0" id="614180323381" line="0" text="" type="WIDGET_LIST">
            <ast col="13" id="614180323382" line="5" text="i" type="WID_FILL_IN">
              <annotation datatype="java.lang.Long" key="oldtype" value="2388"/>
              <annotation datatype="java.lang.Long" key="refid" value="614180323331"/>
              <annotation datatype="java.lang.String" key="javaname" value="i"/>
              <annotation datatype="java.lang.String" key="accessor" value="widgeti"/>
              <annotation datatype="java.lang.Long" key="frame-id" value="614180323411"/>
              <annotation datatype="java.lang.Long" key="peerid" value="618475290679"/>
              <ast col="15" id="614180323385" line="5" text="in" type="KW_IN">
                <ast col="18" id="614180323388" line="5" text="frame" type="KW_FRAME">
                  <ast col="24" id="614180323391" line="5" text="f" type="WID_FRAME"/>
                </ast>
              </ast>
            </ast>
          </ast>

Vadim Gindin wrote:

... Is it because every widget is already annotated with "frame-id"?

Yes.

You should find a way to disambiguate the IN SUB-MENU clause and annotate the MENU-ITEM with the sub-menu-id somewhere in the annotations phase. Later, in the conversion phase, it will just read the AST referenced by the sub-menu-id annotation and get the java name (or other info).

The reference menu-item mi:label in sub-menu sm must be converted to the following code sm.mi.setLabel("New mi label"); and this code must appear in menu definition class for m1 - MenuDefinitionM1 only. Not in the procedure class, because there is no sm field and not in both menu definition classes, because of the described behavior.

I'm adding created menu conversion rules from widget_references.rules to menu_generator.xml, but I'm not sure if it is a right way. May be there is a simpler variant: generate full reference in a procedure class:
m1.sm.mi.setLabel("New mi label");. What do you think?

I think your approach is correct: emit it into the first menu definition and not in the business logic (procedure class).

If there is any duplication of code between widget_references.rules and menu_generator.xml, then please do make functions that can be called from both places.

I think that procedure class is better place because concrete reference could be a part of some logic in that procedure: conditions, cycles and so on. Don't you mind? I've finished the case "in sub-menu sm" using this variant. There are possible the other variant "menu-item mi in menu m1". And mi can appear in different subtrees and levels of m1's tree. I need to test it and implement

OK, let's go with your plan.

Just for documenting purposes.

One SUB-MENU can appear in one MENU tree only once. Therefore when we have a reference menu-item mi in sub-menu sm it means that MENU-ITEM can be a immediate child of this SUB-MENU or a descendant of some level. We need to convert such reference to something like this: m.sm.sm1.sm2[.smN].mi, where m is an implicitly defined MENU reference, the first met MENU containing SUB-MENU sm, and sm1, sm2, .. smN - several interim SUB-MENUs. That is why we can use simple algorithm to build full reference on that menu: Just going up the tree, starting from menu-item and collect all interim SUB-MENUS.

The situation with references of type menu-item mi in menu m is more complex, because MENU-ITEM can appear in different levels of MENU tree. And we will need some sort of depth-first search (DFS) to find concrete MENU-ITEM. Here are couple of tests to illustrate the difference from DFS used in TRPL and the DFS used by Progress.

Here is the sample tree in order of definition in a source procedure.
m -> sm1 -> sm2 -> mi
m -> sm -> mi
m -> mi

Another sample tree:
m -> sm1 -> sm2 -> mi
m -> sm -> mi

These and other tests allow to formulate the search algorithm as follows:
1. Lets assume that cur is the current node in a MENU tree. Initial value is a DEFINE_MENU node.
2. First we are searching the given MENU-ITEM in all immediate child MENU-ITEMs.
3. If we've found the match than return success.
4. If we've not found the match we are going into the all SUB-MENUs and for each of them we are repeating steps from the step 2.
5. If we've searched all tree and didn't find a match - just return null

P.S. Some off-topic question. Lets assume that we've found some path sm1.sm2.sm3.mi. Can I emit only one JAST node with the text sm1.sm2.sm3? It is simpler and It works but it gives incorrect JAST sub-tree. What do you think?

This doesn't seem right. Wouldn't the 4GL do this "lookup" at runtime instead of at compile time? In other words, can you move these things around at runtime such that what we can detect at conversion is no longer correct?

It seems to me that we could write a runtime method that can do this lookup for us, returning back the correct menu-item, no matter how deeply nested it is. Even if the menus cannot be changed at runtime and are truly static, I would rather have this lookup method approach because it will be much cleaner to use and read in the business logic.

The reference menu-item mi in menu m would probably be converted to m.findMenuItem("mi"). Is that what you mean? Do your proposal relates to menu-item mi in sub-menu sm too?

I.e. adding new method findMenuItem to the MenuDefinition class.

I.e. adding new method findMenuItem to the MenuDefinition class.

Almost. I was thinking that m is really an instance of MenuWidget right, not a MenuDefinition. Once the menu has been instantiated, the MenuDefinition no longer represents the state of the widget. The definition is just for the setup. At runtime, all functionality should be in the widget classes themselves.

I will respond with a code review later, but for now I am responding to your parser ambiguity issue.

In the current commit I commented out the lines:

I know that those lines are needed since as you have noticed, the 4GL has a quirk in how it processes DOT in menus. I don't know why, but we have actually seen this in customer code multiple times.

In fact, I just checked in a parser that extends this code to additional "lookahead" choices.

           // TODO: we know that at least the MENU-ITEM and RULE parts of menu_element_descriptor
           //       can trigger this quirk (we've added SUB-MENU and SKIP here to get ahead of
           //       the game (hopefully that is not wrong), but are there other possible
           //       following like_menu_clause etc... which can also be a trigger for this
           //       deviant 4GL behavior?
         | { LA(2) == KW_MENU_ITM || LA(2) == KW_SUB_MENU ||
             LA(2) == KW_RULE     || LA(2) == KW_SKIP }?
           DOT

Please write testcases to check if KW_SUB_MENU and KW_SKIP also trigger this behavior. If they do, then we can remove the TODO. Let's get this area implemented properly.

If we uncomment back those 2 lines, I noted earlier, than we will get another error while trying to convert the same procedure (see several_dots_error.log). This error happens because when the parser meets menu-item mi:label in menu m = "New mi lbl". it expects that it will be the next element in DEFINE_MENU elements list and when it meets : it throws an error.

Interesting. Check if the 4GL responds differently based on whitespace. Is that extra blank line significant?

If we can't detect this based on whitespace, then the correct fix is to look at LT(3) (only when LA(2) is KW_MENU_ITM or possibly KW_SUB_MENU). If that token's text matches an existing menu-item (or possibly an existing sub-menu), then we would NOT allow a match on the DOT inside the loop.

1. copy all definitions classes (for menus or submenus) for all tree (sub-menus of all levels and root menu or submenu) and make neccessary renames. Only one rename is necessary: copied root menu/sub-menu file and it's class name.
   - non trivial rules and tricky renames, but possible.
   + as a result we will have static definitions classes and we'll be able to use references to copied elements as usual, because we will have all static fields.
2. Do it dynamically
   + probalby simplier implementation in Java
   - unclear how to process references to copied menu elements.

Could you advice me something?

1. This ambiguity also appears for SUB-MENU
2. RULE and SKIP cannot be used as separate statement.
3. Whitespace does not affect behavior.

I would propose the following rule:

         | { (LA(2) == KW_MENU_ITM && LA(4) != COLON) ||
             (LA(2) == KW_SUB_MENU && LA(4) != COLON) ||
             LA(2) == KW_RULE     || LA(2) == KW_SKIP }?
           DOT

What do you think?

About LIKE clause. I didn't find conversion implementation of LIKE in other statements for example DEFINE VARIABLE, DEFINE FIELD or others. Is it implemented or my case is the first?

Yes, it is implemented.

The way it works is that at parse time (in progress.g and SymbolResolver) we rewrite the tree to copy/match the configuration of the target variable, temp-table or temp-table field.

Please see like_clause in progress.g and SymbolResolver.addLocalVariableLike() to see the logic.

By doing this at parse time, we don't need different conversion rules downstream. That is why you haven't found any conversion rules for LIKE.

Talking about LIKE conversion. I see 2 variants:

1. copy all definitions classes (for menus or submenus) for all tree (sub-menus of all levels and root menu or submenu) and make neccessary renames. Only one rename is necessary: copied root menu/sub-menu file and it's class name.
   - non trivial rules and tricky renames, but possible.
   + as a result we will have static definitions classes and we'll be able to use references to copied elements as usual, because we will have all static fields.
2. Do it dynamically
   + probalby simplier implementation in Java
   - unclear how to process references to copied menu elements.

Avoid all of this. Focus on getting the menu properly configured during parse time and then let it convert using the rules you already have.

Vadim Gindin wrote:

About ambiguity with DOTs. I've made additional testing and found the following.

1. This ambiguity also appears for SUB-MENU
2. RULE and SKIP cannot be used as separate statement.
3. Whitespace does not affect behavior.

I would propose the following rule:
[...]

What do you think?

I don't like hard-coding this to COLON because it misses many other possible constructs. We can't put in special logic for each one.

Instead, why not do a lookup in the symbol resolver to see if the LT(3).getText() is an already existing menu item or sub-menu? By doing that we know for sure that we don't have to "eat" the DOT.

Code Review vig_upd20141225a.zip

This looks pretty good.

1. Does referencing convert/methods_attributes.rules from menu_generator.xml do something useful? In core_conversion.xml, we have multiple <rule-set> nodes nested inside another <rule-set> node. In that mode, all the nested <rule-set> nodes operate on the same tree in sequence, allowing them to cooperatively generate a single output tree that matches the structure and features of the input tree. I'm surprised that referencing this <rule-set> in a non-nested way would have a positive affect.

2. MenuItemWidget.setSubType(character value) should handle the case where the value is unknown.

3. As far as I know, you don't need to extend the frame registry support for shared menus. Shared frames are much more complicated and have multiple instances that shared some but not all state. Menus are different. As we discussed earlier, there is only 1 instance that is really shared in many places. That is very different from frames. So: you don't need the LogicalTerminal change and you don't need to call releaseSharedWidget(). If you know about a reason that I am wrong, please let me know.

4. Please update the javadoc in SharedVariableManager to note that GLOBAL is not a valid option for shared menus. Mention that it will be silently converted to scoped support.

5. Your question:

Added several annotations for references (in_wid_menu, in_wid_menu_id, in_wid_menu, in_wid_sub_menu_id) and first met path first menu-path. By the way first-menu-path is now emitted as one AST with dot separated text. For example for the path m.sm, where m - menu name and sm - sub-menu name following ast node will be emitted: <ast col="0" id="755914244153" line="0" text="m.sm" type="REFERENCE"/>
This is probably incorrect. What do you think?

You're right, it is not the best but sometimes it is just so much simpler that we take some "shortcuts". I think it is OK for now.

Greg Shah wrote:

Vadim Gindin wrote:

About ambiguity with DOTs. I've made additional testing and found the following.

1. This ambiguity also appears for SUB-MENU
2. RULE and SKIP cannot be used as separate statement.
3. Whitespace does not affect behavior.

I would propose the following rule:
[...]

What do you think?

I don't like hard-coding this to COLON because it misses many other possible constructs. We can't put in special logic for each one.

Instead, why not do a lookup in the symbol resolver to see if the LT(3).getText() is an already existing menu item or sub-menu? By doing that we know for sure that we don't have to "eat" the DOT.

The difficulty here is the fact, that already existing menu-item or sub-menu can be a part of DEFINE MENU/SUB-MENU statements. Therefore we can't use this criteria to distinguish menu-item/sub-menu references from using them in DEFINE statements.

The difficulty here is the fact, that already existing menu-item or sub-menu can be a part of DEFINE MENU/SUB-MENU statements. Therefore we can't use this criteria to distinguish menu-item/sub-menu references from using them in DEFINE statements.

OK, then leave it with the COLON for now. Please put in a TODO comment to explain that the exclusion code needs to be improved from just using COLON.

The question about SHARED menus implementation. How about MenuWidget.createStaticSharedMenu? Is its implementation correct and do we need to use TransactionManager that is commented out at this moment? If no - when to call mgr.untrackSharedConfig? See details in commented block.

Vadim Gindin wrote:

The question about SHARED menus implementation. How about MenuWidget.createStaticSharedMenu? Is its implementation correct and do we need to use TransactionManager that is commented out at this moment? If no - when to call mgr.untrackSharedConfig? See details in commented block.

I'm not sure if we need to implement this tracking for shared menus. The use cases for frames are much more complicated than for menus. For example, since it is a single shared instance, the original creation point for that instance probably determines the lifetime in the 4GL for the shared menu. With frames, we have multiple different instances that share the same widget ID and the original frame instance is somewhat "disconnected" from the other instances. I think there are use cases where the original shared frame instance can exit scope while the imported instances still exist. This is probably why we are doing this special tracking by widget ID. I suspect this may not be needed for menus. If I am right, then you can eliminate all of that code.

If I am wrong, then you WILL have to use the TransactionManager code that is commented out in order to get the cleanup to occur when the containing external procedure exits.

Constantin: do you have any thoughts on this?

Greg, I don't understand about LIKE. Lets look at simple statement define menu m like m1. As a result, Do I need to have AST subtree for m1 identical AST subtree for m?

I thought so until now, but I just didn't find the place where the copying is happen.

Vadim Gindin wrote:

Greg, I don't understand about LIKE. Lets look at simple statement define menu m like m1. As a result, Do I need to have AST subtree for m1 identical AST subtree for m?

Mostly yes. You need to modify m so that it would have the same nodes and annotations as if it was explicitly coded the same way that m1 is coded, except with a different name.

As I mentioned, we do this copying in the parser and SymbolResolver so that the downstream processing won't care whether something was defined LIKE or explicitly defined. The resulting AST must be the same.

Please ask questions about the processing in the like_clause in progress.g and SymbolResolver.addLocalVariableLike() which is not clear.

Thank you for quick answer. I've seen this method earlier and decided to write separate method addMenuLike for menus, that will be simpler. But this method copies only AST of DEFINE_MENU. Target copied AST doesn't contain any children from source AST. Does the copying happen in other place? Another variant, that at parsing moment the source subtree is also empty. What do you think?

Vadim Gindin wrote:

Thank you for quick answer. I've seen this method earlier and decided to write separate method addMenuLike for menus, that will be simpler. But this method copies only AST of DEFINE_MENU. Target copied AST doesn't contain any children from source AST. Does the copying happen in other place? Another variant, that at parsing moment the source subtree is also empty. What do you think?

For define variable, we don't need to duplicate all the child nodes. Even for regular (non-LIKE usage) we convert those nodes to annotations and then hide the child nodes. So when we do a LIKE, we only need to copy annotations and we don't have to create duplicate child nodes.

For menus, you will probably need to duplicate the child nodes. We don't have an example today. I think you can probably use duplicateFresh() and graft() to make the copies.

Now I understand. Thank you!

My method is the following

  public void addMenuLike(String name, Aast ast)
   {
      int     ltype   = ast.getType();
      int     type    = ltype;
      String  oldname = ast.getText();

      type = lookupMenu(oldname);
      Variable var   = new Variable(name, type, null);
      Aast     fopts = null;

      // we must pattern the variable off another variable
      Variable like = (Variable) lookupWrapped(menuDict, oldname);
      fopts = like.getOptions(0);

      if (fopts != null)
      {
         System.out.println(fopts.dumpTree());
         var.copyDefaultOptions(fopts);
         System.out.println(var.getOptions(0).dumpTree());
      }

      if (menuDict.getSymbolAtScope(name, 0) == null)
      {
         addWrappedWorker(menuDict, false, name, (TokenDataWrapper) var);
      }
   }

The call copyDefaultOptions copies full subtree as documented. I've compared printed output before and after the call. They are identical and contain children. But resulting *.p.ast contains unchanged subtree:

    <ast col="0" id="214748364833" line="0" text="statement" type="STATEMENT">
      <ast col="1" id="214748364834" line="7" text="define" type="DEFINE_MENU">
        <annotation datatype="java.lang.String" key="name" value="m1"/>
        <annotation datatype="java.lang.Long" key="type" value="401"/>
        <annotation datatype="java.lang.Boolean" key="widdef" value="true"/>
        <annotation datatype="java.lang.String" key="javaname" value="m1"/>
        <ast col="13" id="214748364837" line="7" text="m1" type="SYMBOL"/>
        <ast col="16" id="214748364840" line="7" text="like" type="KW_LIKE">
          <ast col="21" id="214748364842" line="7" text="m" type="SYMBOL"/>
        </ast>
      </ast>
    </ast>

Why it could be?

The copyDefaultOptions() only saves a reference (inside the Variable instance) to the copied subtree. It does not graft the copied tree into the source tree. Please see Variable.setOptions() and how that method uses overrideIfNotPresent().

How to hide/remove unnecessary AST nodes? I need to replace DEFINE MENU m1 LIKE m with DEFINE MENU m ..... I've tried to call getParent from DEFINE_MENU ast node but got null. I just wanted to get DEFINE_MENU ast parent, remove its child and graft copied subtree to it by call: parent.graft(..). It would be not very good to transfer DEFINE_MENU parent as an additional parameter. Isn't it?

I don't really understand what you are doing. If getParent() returns null, then that subtree hasn't yet been grafted into a larger tree. That isn't possible for the normal AST subtrees created by the parser, since they are created already attached into the larger tree. Please provide some more detail to explain.

I'm calling sym.addMenuLike(#m.getText(), like_name, ##); in the end of def_menu_stmt rule in progress.g. Inside the addMenuLike I'm trying to call ast.getParent() where ast - is the node parameter, corresponding to ##. It seems I'm all adrift.

The parser will be connecting the ## node to the rest of the tree AFTER the def_menu_stmt is done processing. So anything you do before then can only work with the subtree that is being created.

You should not need to remove and re-graft it. Just add children and annotations to the ## node to make it match the LIKE subtree. The LIKE subtree will already exist previously in the overall tree, so this should be fine. But you'll have to find that other tree in order to copy it. Look carefully at the Variable.overrideIfNotPresent() to see how we only differentially copy things needing to be overridden. At least the define variable approach can have some partial definition where only some of the LIKE sub-tree is copied.

Greg Shah wrote:

I'm not sure if we need to implement this tracking for shared menus. ... If I am right, then you can eliminate all of that code.

Greg, you are correct. The shared menus don't have the same complexity as the shared frames. A shared menu will always use the master definition, even if you specify different labels at the shared definition. So the track/untrackSharedConfig calls are not needed and can be removed - these APIs were added only because of the shared frame complexities.

I'm trying to find out a solution for extra semicolon emission (see note #71). I still can't find a place where it is processed. It really happens for NUMERICAL literals. You wrote that it could happen because of extra AST node (probably hidden). Probably it is. Lets consider following example:

define menu m title "Menu_title" fgcolor 3.

<ast type="KW_TITLE">
  <ast type="STRING"/>
</ast>

<ast type="KW_FGCOLOR">
  <ast type="EXPRESSION">
    <ast type="NUM_LITERAL" />
  </ast>
</ast>

I found some interesting place in expressions.rules line 1234:

         <!-- Bypass emission of the handle referenced into TABLE-HANDLE FOR and
                  formatting of OLD/NEW VALUE in ASSIGN TRIGGERS -->
         <rule>relativePath("DEFINE_PARAMETER/KW_TAB_HAND/EXPRESSION")       or
               relativePath("KW_ON/KW_OLD/KW_FORMAT/EXPRESSION")             or
         ... 

You'll have to upload the entire update for me to get a better sense of your approach. We should not have to put lots of exception rules in all over the place in order to get this to work.

If that whole KW_FGCOLOR subtree is hidden AFTER it is used in menu_generator.xml, it really should not be causing any problems and we should not need the downstream exceptions.

Code Review vig_upd20150112a.zip

Generally, the changes are quite good. I see the following cleanup items. It seems pretty close to something we should put through testing.

0. Is there anything left to implement? I see a mention above about testing trigger support. Everything else seems like it may be complete.

1. In regard to the expressions.rules change, I can accept the approach but you should create your own dedicated rule for menu element suppression. Right now you are sharing it with the TABLE_HANDLE code. That is confusing and may lead to problems editing that code in the future. Please split your code into its own section and give it a useful comment.

2. Please do the same thing for the literals.rules suppression. It should be in its own section with its own comment instead of mixing it into the triggers section. In fact, you are not suppressing the character literal at all, but it is a new suppression of integer literals, so the comment is especially confusing.

3. The methods_attributes.rules is improperly merged. Some entries have been duplicated. Please bzr diff it to see the problems.

4. Can the following be removed from the progress.g TODO in def_menu_stmt: "we know that at least the MENU-ITEM and RULE parts of menu_element_descriptor can trigger this quirk (we've added SUB-MENU and SKIP here to get ahead of the game (hopefully that is not wrong), but"? I think you have proven that both sub-menu and skip CAN be used there right? If so, then those parts of the TODO should not be there.

5. On line 10114 of progress.g, I think we should drop the DOT because it is really just trash. Unless you have a need for it, please change it like this: DOT! which will drop the token from the tree after matching it.

6. In progress.g, I like that you used lvalue in submenu_descriptor. That is a big improvement. Please remove the commented code AND remove the comment about being a "placeholder". The namespace processing is now there thanks to your changes, so it is no longer a placeholder.

7. In progress.g the method #itm.getText() is called 3 times in the menu_item_phrase exit action. Please cache it in a local var (String txt = #itm.getText();) and then reuse it. It is a minor optimization but there is no reason not to take it.

8. Making AnnotatedAst.putAnnotationImpl() into a public access method is not safe. The reason is that only certain data types are supported by the AST persister plugins. So we only allow very specific types through there.

9. You have a dead rule (about WAIT-FOR) at the bottom of menu_scoping.rules.

The only triggers references are left. What do you think about approach I used for that?

I'm OK with what I saw in the code.

Please post some examples here (both the 4GL and then the converted Java for the triggers). Each trigger should have both a registerTrigger() in the code and a TriggerBlock inner class with the trigger code itself. Please paste the examples into sections so it is easy to review.

After I fixed some parsing problem I faced with the following difficulty during conversion of the following trigger:

on choose of menu-item mi in sub-menu sm
   message "mi in sm".

            <ast col="14" id="309237645369" line="8" text="menu-item" type="KW_MENU_ITM">
              <ast col="24" id="309237645372" line="8" text="mi" type="WID_MENU_ITM"/>
              <ast col="27" id="309237645375" line="8" text="in" type="KW_IN">
                <ast col="30" id="309237645377" line="8" text="sub-menu" type="KW_SUB_MENU">
                  <ast col="39" id="309237645380" line="8" text="sm" type="WID_SUB_MENU">
                    <annotation datatype="java.lang.String" key="first-menu-path" value="m"/>
                  </ast>
                </ast>
              </ast>
            </ast>

         <ast col="1" id="317827580082" line="26" text="menu-item" type="KW_MENU_ITM">
            <ast col="11" id="317827580085" line="26" text="mi" type="WID_MENU_ITM">
              <annotation datatype="java.lang.String" key="in_wid_sub_menu" value="sm"/>
              <annotation datatype="java.lang.Long" key="in_wid_sub_menu_id" value="317827580094"/>
            </ast>
          </ast>
          <ast col="14" id="317827580087" line="26" text="label" type="ATTR_CHAR">
            <annotation datatype="java.lang.Long" key="oldtype" value="654"/>
          </ast>
          <ast col="20" id="317827580089" line="26" text="in" type="KW_IN">
            <ast col="23" id="317827580091" line="26" text="sub-menu" type="KW_SUB_MENU">
              <ast col="32" id="317827580094" line="26" text="sm" type="WID_SUB_MENU">
                <annotation datatype="java.lang.String" key="first-menu-path" value="m2"/>
              </ast>
            </ast>
          </ast>

The difference in these ASTs is in a place of KW_IN AST node: for trigger KW_IN is a child of KW_MENU_ITEM and for the assignment KW_IN is a neighbor of KW_MENU_ITEM.

The question.
Should I fix generated AST for these 2 cases to unified view or I should process both views of ASTs to add my annotations correctly?

I suppose I should make the common AST subtree for both cases. How can I make it and where? May be it is also a parser bug?

It is a good question. We already have quite a bit of logic that handles these cases differently. Right now they cannot be unified. Please just put extra logic in to handle the annotations for both cases.

So: NO, do not try to make these into the same AST structure.

I think the triggers code looks fine. Is there anything else to do for the conversion?

Code Review vig_upd20150113a.zip

The only problem I see is that you still have a duplicate section for kw_full_h_c in methods_attributes.rules.

Otherwise it seems ready to test.

Vadim Gindin wrote:

Please, recall me, should I change a year of a Copyright block in each modified file to 2015?

Yes, you need to update the copyright date, to from e.g. 2012-2014 to 2012-2015, or from e.g. 2013 to 2013-2015.

Code Review vig_upd20150114c.zip

This is really good. Please put it into both conversion and runtime regression testing.

The one minor change I would still like to see, is for name conversion to occur in only 1 place (probably during menu_generation.rules@. Then that name should be saved in the original definition node as an annotation and when references are emitted "downstream" for a reference in places like widget_references.rules, then those places should lookup the converted name from the original definition node and use that. This allows us to always share the same name (which may be important if we have to disambiguate the name for some reason or if in the future we add hints to allow name customization). It also makes the downstream code less duplicative and less error prone, since we don't have to remember to re-convert the name everywhere.

DON'T make this change before putting the code into testing. Make this change with your next work which is about getting the runtime working for ChUI.

I've ran conv-regression task. It executed, but failed at the end. Here is the end of the log:

...
jar:
      [jar] Building jar: /home/vig/testing/majic/build/lib/majic.jar
      [jar] Building jar: /home/vig/testing/majic/build/lib/majic_ui.jar

convert-all:

save-generated:

prepare:
     [copy] Copying 119 files to /home/vig/testing/majic/src
    [input] Save the generated sources (y/n)? (y, n)
     [exec] ** Adding the generated sources to 'generated/20150114b/generated_source_backup_20150114b_10707.zip'...
     [exec] ** Adding the MAJIC/P2J updates to 'generated/20150114b/generated_source_backup_20150114b_10707.zip'...
     [exec] ** Sources saved to 'generated/20150114b'.

conversion-test:
    [input] Enter the stable folder name (without 'generated/' prefix):
    [input] Enter the candidate folder name (without 'generated/' prefix):
     [exec] The folder 'generated/some_folder' do not exist!
     [exec] Result: 2

BUILD FAILED
/opt/secure/clients/timco/testing/build_rt.xml:369: Non-source compare error encountered.

The script asked me to specify stable and candidate folders. I've specified new names ("some_folder, candidate_folder"). But it was expecting the names of existing folders. I don't know what names I had to specify there. Could you help me?

As noted in the timco.html:

Build and Save a Comparison Version

Before running any conversion regression test, you will need the save off a comparison version of the converted MAJIC project. It is important to have the latest version of the P2J trunk built at the time you run this, so that your saved comparison version will be comparable to a version converted using any changes to P2J that are added later.

To convert MAJIC for the first time (and save off this build as the comparison version):

run_regression.sh build save-generated -Dsave.candidate=y

Multiple targets are passed to ant in the above command.

The conversion and compilation of the result will take a bit less than 1 hour for the MAJIC project. This will run using whatever version of MAJIC and P2J is already there.

The saved off version will be stored in ~/testing/generated/ under a directory that is named with a sortable date (the date it was saved) plus an alphabetic character to index the builds done in that day, so the first saved build of the day will be ~/testing/generated/YYYYMMDDa/ and the second saved build will be ~/testing/generated/YYYYMMDDb.

I was using an eclipse plugin to merge files last time and didn't accustom to this plugin probably. The reason of the error was in improperly merged frame_generator.xml. I'm going to run conv-regression again.

Can It be useful for somebody?

Very cool! I like it.

The only thing we need to do here is to eliminate the dependency on ~/work/ as most systems won't have that path.

Yes, please do check in vig_upd20150114d.zip and distribute it.

vig_upd20150114d.zip is committed to bzr with rev №10709

Should I commit (without work folder as you wrote) the script from the note #152? If yes - can I commit it right to p2j folder?

No, that is not something that is specific to the p2j project. It is a tool that we would add to the standard workstation setup.

Clean it up and then send it out to the team in an email. I'll include it into the zip files for the standard workstation.

I trying to decide how to transfer information about main menu (menubar=true) to the client. ScreenBuffer is not really useful because it linked to a frame and we don't know what frame is the main (common). Should I create some separate class for that or ScreenBuffer is preferable?

Also I'm a bit confused about drawing implementation. Should I use screen().append(..) string-like methods to draw a menu as it work in ButtonImpl for example?

ScreenBuffer is not really useful because it linked to a frame and we don't know what frame is the main (common). Should I create some separate class for that or ScreenBuffer is preferable?

ScreenBuffer is for editing data in widgets that are contained in frames. Menus are outside of any given frame and have no editing widgets, although I guess there may be a little state for things like toggle box menu items. The ScreenBuffer should not be used.

The majority of the functionality can be pushed from the server to the client as a configuration. You do need to create a similar function to pushFrameDefinition(). I expect pushMenuDefinition() will be a good choice. Then anytime the configuration of the menus change, make the config class edits and then just call pushMenuDefinition() again. It should replace the config on the client with the newer config.

Make a list of the state that can be changed on the client-side and describe it here. We will come up with an approach to transfer that back to the server.

Also I'm a bit confused about drawing implementation. Should I use screen().append(..) string-like methods to draw a menu as it work in ButtonImpl for example?

Generally, yes. However, you will need to implement the top-level menu class as a kind of container. It won't be as complicated as a frame.

Constantin: thoughts?

Constantin: thoughts?

1. I suspect that showing the sub-menu (when a menu is activated in the menubar) will need to bring the menubar and the activated sub-menu on top in the window (by changing its z-order). The menubar needs to act as a container to keep the menus (their labels). The sub-menu's body (containing the menu-items) I think can be parented at the menubar, too; it's just a matter of making the appropriate sub-menus visible.
2. when drawing, yes, you need to use screen().append(..) to draw text and screen().at(...) to position the cursor to the appropriate location. Note that in ChUI, the coordinates received by screen().at(...) are all relative to the screen - so use screenLocation to determine the widget's origin (relative to screen). Check the ChuiOutputManager for other ChUI-specific drawing APIs (like box and vert/horiz line drawing).

Code Review vig_upd20150117a.zip

As a general approach, I'm not entirely convinced it is the best idea to add menu to ScreenDefinition. However, for now I think you should continue working the solution in that way. Please do not "force fit" the menus into the ScreenDefinition, which is a class that is very specific to frames today. It may be better to create a new mechanism for sending menus to the client. If you find yourself making the code do "unnatural things", then it is time to design something menu-specific.

In addition, make sure there is a need for a menu-specific registry in LogicalTerminal. What will it be used for? And if you do need a menu registry, then you must also de-register menus from it.

In the future, when posting something that isn't large, please post it in-line here. Don't attach a file for such things because it makes it harder to review.

com.goldencode.p2j.net.SilentUnwindException: Connection ended abnormally
    at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1142)
    at com.goldencode.p2j.net.Queue.transact(Queue.java:585)
    at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:223)
    at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:163)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1424)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:97)
    at com.sun.proxy.$Proxy4.standardEntry(Unknown Source)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:276)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:99)
    at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:201)
    at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:398)
    at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:1)
    at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:267)
Caused by: com.goldencode.p2j.net.ApplicationRequestedStop: Queue stop is requested by application
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:304)
    at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:257)
    at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1128)
    ... 12 more

and the server log:

[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded auth-mode 4, retries 0
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded auth hook as <com.goldencode.p2j.security.GuestAccess>
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded anonymous ID <null>
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded auth-mode object
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded group <everybody>
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded group <admins>
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded 2 groups, 0 objects ignored
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded process <standard>
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded 1 processes, 0 objects ignored
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded user <bogus>
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded user <admin>
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded 2 users, 0 objects ignored
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} No holidays defined in the P2J directory.
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} Loaded resource plugin for <system> as 0
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded 1 resource plugins, 0 failed
[01/23/2015 20:07:23 YEKT] (SecurityManager:WARNING) {main} No custom server extension defined in P2J directory
[01/23/2015 20:07:23 YEKT] (SecurityManager:WARNING) {main} No custom client extension defined in P2J directory
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main} ACLs for resource type <system>:
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINER) {main}   origin /security/acl/system/000100
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}    instance name <admin> (exact)
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       rights {true}
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       subjects admins 
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINER) {main}   origin /security/acl/system/000200
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}    instance name <change> (exact)
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       rights {true}
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       subjects admins 
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINER) {main}   origin /security/acl/system/000300
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}    instance name <logon> (exact)
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       rights {true}
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       subjects *all_others* 
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINER) {main}   origin /security/acl/system/000400
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}    instance name <shutdown> (exact)
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       rights {true}
[01/23/2015 20:07:23 YEKT] (SecurityManager:FINE) {main}       subjects *all_others* 
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {main} Loaded 4 ACLs for resource type <system>
[01/23/2015 20:07:23 YEKT] (SecurityManager:WARNING) {main} Security audit log is disabled
[01/23/2015 20:07:23 YEKT] (SecurityManager:INFO) {00000000:00000001:standard} No exported entry points defined in the P2J directory
[01/23/2015 20:07:23 YEKT] (com.goldencode.p2j.main.StandardServer:WARNING) Unable to initialize runtime conversion pool; if dynamic database features are not in use, this warning can be ignored safely (otherwise restart server with FINE logging for additional information)
[01/23/2015 20:07:23 YEKT] (Scheduler:WARNING) {00000000:00000004:standard} No scheduler configured.
[01/23/2015 20:07:23 YEKT] (com.goldencode.p2j.persist.DatabaseManager:WARNING) No databases are configured
[01/23/2015 20:07:23 YEKT] (SessionManager.listen():WARNING) {00000000:00000001:standard} INSECURE sockets in use!
[01/23/2015 20:07:23 YEKT] (SessionManager.listen():INFO) {00000000:00000001:standard} Server ready
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Insecure socket connection.
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Getting AUTHTYPE
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Got AUTHTYPE: 0 userName: null
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINE) {00000000:00000005:standard} hook class is com.goldencode.p2j.security.GuestAccess
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sending AUTHMODEREQ: 4 (action = 1)
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sent AUTHMODEREQ
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sending AUTHCUSTOM
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sent AUTHCUSTOM
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Getting AUTHDATA
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Getting AUTHDATA authSize=9
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Got AUTHDATA
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Received user ID <bogus>
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000001:00000005:bogus} Search open: resId 0, instance logon, mode 0, handle 1
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000001:00000005:bogus} Search next: handle 1, rights {true}
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000001:00000005:bogus} Search done: handle 1, decision true, cache false
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sending AUTHRESULT0 (action = 0)
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sent AUTHRESULT
[01/23/2015 20:07:30 YEKT] (SecurityManager:FINER) {00000000:00000005:standard} Sending LOCALUSERID: bogus
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Insecure socket connection.
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Getting AUTHTYPE
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Got AUTHTYPE: 0 userName: null
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINE) {00000000:00000009:standard} hook class is com.goldencode.p2j.security.GuestAccess
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sending AUTHMODEREQ: 4 (action = 1)
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sent AUTHMODEREQ
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sending AUTHCUSTOM
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sent AUTHCUSTOM
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Getting AUTHDATA
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Getting AUTHDATA authSize=9
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Got AUTHDATA
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Received user ID <bogus>
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000002:00000009:bogus} Search open: resId 0, instance logon, mode 0, handle 2
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000002:00000009:bogus} Search next: handle 2, rights {true}
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000002:00000009:bogus} Search done: handle 2, decision true, cache false
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sending AUTHRESULT0 (action = 0)
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sent AUTHRESULT
[01/23/2015 20:12:34 YEKT] (SecurityManager:FINER) {00000000:00000009:standard} Sending LOCALUSERID: bogus

I can't tell anything useful from the logs. You'll have to set breakpoints in the SilentUnwindException constructor to see where the problem occurs. It should be on the server side I guess, because I don't think we use SilentUnwindException on the client.

I did it and nevertheless the constructor of SilentUnwindException is called from client-side with the error com.goldencode.p2j.net.ApplicationRequestedStop: Queue stop is requested by application

Hopefully you are continuing to debug. I have no idea what is causing the problem.

Code Review vig_upd20150128a.zip

Overall, this is MUCH better than the previous approach. Well done.

1. Menus need to be deregistered from the LogicalTerminal registry.

2. I don't understand the need for MenuElementConfig interface. These values are already directly available in the parent class hierarchy, without any need for getters/setters. In fact, we are trying to eliminate all getters/setters from our config hierarchy. What is the functional reason to add more for menus?

3. You should not leave an empty JPRM column in the file header for files that never had any JPRM numbers (e.g. all new or recent files like MenuItemWidget, SubMenuWidget, MenuWidget).

4. The copyright date for MenuConfig and WidgetConfigDef should be 2014-2015 since both files had development work done in both years.

5. There are places were you are using explicit imports which should be converted to wildcard imports (see MenuConfig, Menu, MenuChuiImpl, MenuItemChuiImpl, MenuWidget).

6. ThinClient, ClientExports are each missing a history entry.

I'd like to see this next set of functionality stabilized and put into testing. What is left to do?

I'm working on showing POPUP-MENU for widgets. I'd like to add this too.

About MenuElementConfig. There is some confusion for me with parentId property (different fields for different classes somehow). I'll try to handle with this property and get rid of that interface.

I need an advice. Assume we have a button with popup-menu assigned. We must press ESC-U for CHUI or F4 for WINDOWS to call this menu when the button has focus. After hot-key is pressed the client-side must display this menu. Does it mean, that I should push menu description to the client-side right in that moment (during processing of a hot-key)? Another variant is to push menu description along with pushing a frame, containing our button. What way is better? I think the first one.

You asked earlier about state list that can be changed from the client side. I also thought earlier that this is the only TOGGLE-BOX property of menu items. But what about VISIBLE property? In described case this property got new value. It also make sense for showing sub-menus in a menu of any type: MENUBAR or POPUP. I don't see other state at this moment.

I also want to add, that I faced with conversion bug for frames during conversion of chui_menu_2_purpose.p. Extra frames were generated and FrameDefinition was generated incorrectly: expr (and also extra expressions) were generated instead of buttons. I've simplified the testcase and the bug is gone. I'll have to return to this bug later.

Assume we have a button with popup-menu assigned. We must press ESC-U for CHUI or F4 for WINDOWS to call this menu when the button has focus. After hot-key is pressed the client-side must display this menu.

Can you create a trigger on this event and use no-apply? Something like this:

ON ESC-U OF my-menu
DO:
   RETURN NO-APPLY.
END.

If so, does it cancel the menu popup before it is ever displayed?

In all likelihood, the default popup behavior can be driven on the client side without any trip to the server as part of default processing for the frame or widget that has a popup menu associated. The only question is where in the client side processing to add such support? It may have ot be after the triggers are executed OR it could be that triggers don't work in this case.

Regardless, I think the popup menu definition should already be pushed whenever it is defined in the business logic, just like a frame. Any changes to the menus would naturally cause a push again. At the time editing is occurring, the menu should already be there.

Does it mean, that I should push menu description to the client-side right in that moment (during processing of a hot-key)?

I don't think so. At that moment we are on the client side already and we should not need to "up-call" to the server to get a menu def.

Another variant is to push menu description along with pushing a frame, containing our button.

I would prefer if the push of menus stays separate. You already have the menu creation pushing the defs down to the client. Keep the same approach for popup menus.

But what about VISIBLE property? In described case this property got new value. It also make sense for showing sub-menus in a menu of any type: MENUBAR or POPUP.

Are you saying that the client can implicitly change the value of this attribute which can can be seen in the business logic by 4GL code? If so, then this state will have to be sync'd. Though I think we already do that for other widgets today, so why would menus be any different?

I've simplified the testcase and the bug is gone. I'll have to return to this bug later.

OK.

Greg Shah wrote:

Can you create a trigger on this event and use no-apply? Something like this:

[...]

If so, does it cancel the menu popup before it is ever displayed?

I've added this trigger and it really cancels menu appearing. The only moment that the trigger was added for the button, not for the menu.

OK. Keep this in mind when you are working on getting the event processing to work properly.

Code Review vig_upd20150130a.zip

I think it is moving in the right direction.

1. I think your drawing issues are related to how you are directly calling menu.draw() in ThinClient. I wish I could find Constantin's email or redmine post about this topic.

Constantin: do you remember where that is posted?

2. Imports in MenuItemConfig have expanded from wildcards.

3. In MenuWidget, the LegacyAttribute annotation should not be there. Those annotations should be placed in the interface, not the implementation.

4. In methods_attributes.rules, please use the new load_descriptors approach for defining new attributes.

1. I've added POPUP-ONLY to load_descriptiors function along with other menu related attributes, created MenuInterface and used it there, added this interface to HandleCommon, but still got mbar.getSelf().readOnlyError("popup-only");.
2. I didn't find probably Constantin's email or redmine post about drawing. I tried to enclose menu.draw() call with eventDrawingBracket but the error still happen.

but still got mbar.getSelf().readOnlyError("popup-only");.

You have to add the attribute into the writable attributes list in common-progress.rules.

I can't find Constantin's note/post either which is why I asked him for the location. He is working on some bugs this morning, I'm sure he'll get to this at some point. In the meantime, consider that the menu visibility should be toggled based on certain events and then a repaint cycle should be triggered, instead of directly calling draw().

Greg Shah wrote:

1. I think your drawing issues are related to how you are directly calling menu.draw() in ThinClient. I wish I could find Constantin's email or redmine post about this topic.

Constantin: do you remember where that is posted?

1. in ChUI, all drawing is relative to the screen's top-left corner (0, 0) - so all cursor positioning, etc, needs to be done in non-relative units. in GUI, this is different, we have locations relative to the parent; see note 647 in #2252 for more about this
2. drawing is done directly (by invoking the draw API) only in special circumstances. In normal cases, you need to raise a repaint event and let the drawing infrastructure do its work. This sometime means to raise the repaint in a TC.eventDrawingBracket. But your code in TC.processProgressEvent doesn't look like it's in the right place:
   

         if (action == Keyboard.KA_DEFAULT_POP_UP && (tr == null || !tr.executed) && src != null)
         {
            int menuId = src.config().popupMenuId;
            if (menuId != -1)
            {
               Widget<?> menu = tk.getRegistry().getComponent(menuId);
               if (menu != null)
               {
                  menu.draw();
               }
            }
         }
   

   
   a. I think it should be in either the source's or the menu's processEvent implementation.
   b. you need to call menu.repaint() to raise the repaint event, not menu.draw().

Try calling menu.repaint() first and see if this fixes your drawing problem. If so, check if it can be moved to processEvent

I did as you advised. I used menu.setVisible(true) that calls repaint() method, that in turn call EventManager.postEvent(new PaintEvent(this, ur));. But draw wasn't called at all. By the way. I don't know how to initialize coordinates for new widgets. Is it normal if coordinates have uninitialized values (Integer.MIN_VALUE) at the moment of posting event?

I'm debugging further..

I've also added a flag displayed, that will mean dropped down for popup menu or sub-menu. This flag is also a state, that can be changed on the client-side. But I'm not sure if it needed on the server-side.

   public void draw()
   {
      drawPanel();
      super.draw();
      this.firstFocus();
      EventManager.postEvent(new FocusEvent(this.focus(), EventType.FOCUS_GAINED));
   }

1. this.firstFocus(); this statement set the first menu-item as a current;
2. EventManager.postEvent(new FocusEvent(this.focus(), EventType.FOCUS_GAINED)); this statement posts FocusEvent.

It don't works. The menu-item is not highlighted. Could you advice something?

How to properly test static menus deletion?

Vadim Gindin wrote:

Will you also insist on using of PaintEvent? If yes could you help me to understand how it should work?

1. the OutputManager is notified that code which needs drawing is to be executed
2. TC.eventBracket is called which executes some custom code, followed by processing (or discarding) all the collected events. When a PaintEvent is encountered, it collects "invalidation rectangles" - this are screen areas which require painting.
3. when OutputManager is notified that the code has finished, it will go through the entire widget tree (starting from the window) and will draw only the widgets which intersect the invalidation rectangles. This avoids redrawing the entire screen. Also, only the screen area resulted from intersecting the widgets boundary and the invalidation rectangles will be flushed to the screen.

About the focus issue: there is the AbstractWidget.requestFocus API - this should be enough to focus the widget. But this API should not be called from the draw method - here you are AFTER the event processing has finished, so it's too late for this event to be picked up and processed. You need to call this either in the widget's processEvent or within the Runnable code passed as parameter to the TC.eventDrawingBracket API (so the event is added to the TC.currentEventList).

1. if TC's specialized APIs are used, the Runnable code allows custom code related to your action
2. widget's processEvent API - raise and/or process events; if the widget's appearance is different when i.e. focus is gained, then this allows you to set some flag.
3. widget drawing: this does the drawing; no more events should be raised here, UNLESS via a TC.independentEventDrawingBracket - so they are processed independently.
4. main conclusion: event processing and drawing need to be separated.

How to properly test static menus deletion?

Call delete object <handle-var-referencing-the-menu>, i.e.:

def var h as handle.
h = menu mbr:handle.
delete object h.

About the event processing for menus: looks like these are similar to the combo-box dropdown and some other cases, especially when a EDITING block is used. I'm working on making a generic way for treating these system events, but I'm not finished yet.

Code Review vig_upd20150210a.zip

This is very close.

1. There is a build directory included in your update zip file.

2. The attributes/methods added need to be included in an interface. It is the interface name that is specified in methods_attributes.rules, not the widget name, so GenericWidget is invalid. There is a special case for CommonWidget which we just unwrap as Widget. That means that your GenericWidget (widget class) would be Widget (emits as unwrapWidget() on a handle) instead of CommonWidget (the real interface name).

3. MenuItemChuiImpl is missing javadoc for processKeyEvent() and processKeyListeners().

4. I don't want to add references to org.apache.commons.collections.CollectionUtils to the project. Please remove them from MenuContainerWidget.

5. For label processing in menu_generator.xml, is the use of removeQuotes() correct? I wonder if we need proper progressToJavaString() usage to convert any possible escaped chars and so forth.

6. AbstractContainer needs a history entry.

I'm debugging difficult bug:

def button btn1 LABEL "Button".

def FRAME frame1 btn1.

define menu mbar
  menu-item numbr label "Number" 
  menu-item addr label "Address".

on choose of menu-item numbr
   display "Number".

on choose of menu-item addr
   display "Address".

assign btn1:POPUP-MENU = MENU mbar:HANDLE.

enable all with FRAME frame1.

wait-for close of current-window.

This procedure defines frame frame1, containing only one button btn1. Wrong frame definition class is generated for that procedure:

public interface FrameFrame1
extends CommonFrame
{
   public static final Class configClass = FrameFrame1Def.class;

   public void setExpr3(character parm);

   public void setExpr3(String parm);

   public void setExpr3(BaseDataType parm);

   public FillInWidget widgetExpr3();

   public static class FrameFrame1Def
   extends WidgetList
   {
      FillInWidget expr3 = new FillInWidget();

      public void setup(CommonFrame frame)
      {
         frame.setDown(1);
         expr3.setDataType("character");
         expr3.setFormat("x(7)");
      }

      {
         addWidget("expr3", "", expr3);
      }
   }
}

We can see FillInWidget here instead of Button. But if we remove the latest trigger in the source procedure, the frame definition will be generated correctly.

I'm debugging frame generation, but I'm getting stuck. Could you propose me possible linkage between trigger and incorrect frame generation?

In the 4GL I suspect that your DISPLAY statements in the triggers are targeting the the default "unnamed" frame. Somehow we are connecting that with FRAME frame1 in the trigger.

Is this code supposed to work in 4GL ChUI? It doesn't do much that is useful there when I tried it. What are those DISPLAY statements supposed to do?

Greg Shah wrote:

In the 4GL I suspect that your DISPLAY statements in the triggers are targeting the the default "unnamed" frame. Somehow we are connecting that with FRAME frame1 in the trigger.

I've tested of what frames is used in triggers and found that new "unnamed" frame is created for each trigger. For our procedure there will be 3 frames: frame1 and 2 unnamed frames for triggers.
..
Wait a minute. It seems that it is a frames naming (class naming) problem. Here is the main procedure class:

public class Frame
{
   FrameFrame1 frame1Frame = GenericFrame.createFrame(FrameFrame1.class, "frame1");

   MenuDefinitionMbar mbar = MenuWidget.createStaticMenu(MenuDefinitionMbar.class, "mbar");

   FrameFrame0 frame0 = null;

   FrameFrame1 frame1 = null;

...

As you can see the generated interface for 2nd unnamed frame is FrameFrame1 - the same as the main named frame! It means that interface was just overridden! By the way where triggers processing really happen?

Is this code supposed to work in 4GL ChUI? It doesn't do much that is useful there when I tried it. What are those DISPLAY statements supposed to do?

I didn't pursue concrete goal when I was writing this procedure. I just faced with this bug accidentally. But this procedure works for CHUI.

Wait a minute. It seems that it is a frames naming (class naming) problem.

This makes sense. Please put some protection logic in for this naming conflict.

I didn't pursue concrete goal when I was writing this procedure. I just faced with this bug accidentally. But this procedure works for CHUI.

How do you get the menu to display? When I run it, the button appears but I can't make it do anything.

By the way where triggers processing really happen?

Any operation that blocks for user input will be first invoked on the server side. Before we transfer control to the client, we prepare an EventList to describe the combinations of events and widgets that match triggers and validation expressions.

On the client side, during event processing, we check the current event and the event's widget source to see if there is a match to an existing trigger. Look in ThinClient for calls to invokeTriggers(). If there is a match, then the server-side is called back to invoke the trigger on the server. See LogicalTerminal.trigger().

Validation expression processing is similar, but it gets tested using ThinClient.validateWidget() and the server-side call is LogicalTerminal.validate().

Greg Shah wrote:

Wait a minute. It seems that it is a frames naming (class naming) problem.

This makes sense. Please put some protection logic in for this naming conflict.

Don't you remember where that conversion logic?

I didn't pursue concrete goal when I was writing this procedure. I just faced with this bug accidentally. But this procedure works for CHUI.

How do you get the menu to display? When I run it, the button appears but I can't make it do anything.

There are popup-menu associated with button. To open it up you need to press ESC-U.

By the way where triggers processing really happen?

Any operation that blocks for user input will be first invoked on the server side. Before we transfer control to the client, we prepare an EventList to describe the combinations of events and widgets that match triggers and validation expressions.

On the client side, during event processing, we check the current event and the event's widget source to see if there is a match to an existing trigger. Look in ThinClient for calls to invokeTriggers(). If there is a match, then the server-side is called back to invoke the trigger on the server. See LogicalTerminal.trigger().

Validation expression processing is similar, but it gets tested using ThinClient.validateWidget() and the server-side call is LogicalTerminal.validate().

I meant conversion logic, that generates unnamed frames for triggers. Could you remember at first sight where it could be?

Don't you remember where that conversion logic?

See the process_frame function in annotations/frame_scoping.rules.

I meant conversion logic, that generates unnamed frames for triggers. Could you remember at first sight where it could be?

I think this would also be in annotations/frame_scoping.rules. However, I doubt there is any specific logic in there for triggers. As far as I know, there is nothing special about them for frame definition purposes.

Code Review vig_upd20150219a.zip

I ma OK with the changes. Please get this update conversion and runtime regression tested.

Current update has passed conversion testing. I ran regression testing twice. Only one test has failed during both runs: tc_code_employees_20. Could you help me to interpet its results. Here is the failing screen from the report:

02/19/2015                  EMPLOYEE MASTER (5 of 5)                    17:20:04
┌──────────────────────────────────────────────────────────────────────────────┐
│           Employee: 1029    ********** JR., **** E.                          │
│              Skill:                                                          │
│          Crew Code:                                                          │
│              Shift:   0     Days-Off:                                        │
└──────────────────────────────────────────────────────────────────────────────┘
 Start      Start End        End        Crew Crew       Skil     Shift      Days
 Date       Time  Date       Time  Dept Code Eff Date   Code Sft Eff Date   Off
 ────────── ───── ────────── ───── ──── ──── ────────── ──── ─── ────────── ────
>07/02/1990 07:00 07/02/1990 15:30 7000 D40  01/01/1990 7300  11 01/01/1990 67
 11/01/1995 07:00 11/01/1995 15:30 7000 40D  01/01/1990 7300  11 01/01/1990 67
 09/25/1999 07:00 09/25/1999 15:30 1500 070  01/01/1990 1540  11 01/01/1990 67
 05/18/2002 07:00 05/18/2002 15:30 1100 050  01/01/1990 1130  11 01/01/1990 67
 10/18/2003 07:00 10/18/2003 15:30 7000 070  01/01/1990 1540  11 01/01/1990 67
 12/20/2003 07:00 12/20/2003 15:30 7000 070  01/01/1990 7500  11 01/01/1990 67
 06/12/2004 07:00 06/12/2004 15:30 7000 060  01/01/1990 7500  11 01/01/1990 67
 11/01/2004 00:00 11/01/2004 00:00      62E  01/01/1990 1130  11 01/01/1990 67
 11/02/2004 00:00 11/02/2004 00:00      62E  01/01/1990 1130  11 01/01/1990 67
(N)ext (P)rev (U)pdate (A)dd (C)opy (F)ind (D)el (O)utput (T)ext (X)Crew (H)elp
(E)xtend (1)Up (2)Down (3)Pg Back (4) Pg Frwd (5)Beg (6)End (R)eturn: F        

I've manually ran TIMCO from the server and opened that screen:

02/20/2015                  EMPLOYEE MASTER (5 of 5)                    03:16:31
┌──────────────────────────────────────────────────────────────────────────────┐
│           Employee: 1029    *** JR., ********* E.                            │
│              Skill: 1130    IT SOFTWARE DEVEL                                │
│          Crew Code: 62E     *** *** *** *** - *******                        │
│              Shift:  11     Days-Off: 67                                     │
└──────────────────────────────────────────────────────────────────────────────┘
 Start      Start End        End        Crew Crew       Skil     Shift      Days
 Date       Time  Date       Time  Dept Code Eff Date   Code Sft Eff Date   Off
 ────────── ───── ────────── ───── ──── ──── ────────── ──── ─── ────────── ────
>07/02/1990 07:00 07/02/1990 15:30 7000 D40  08/04/2007 7300  11 01/01/1990 67
 11/01/1995 07:00 11/01/1995 15:30 7000 40D  08/04/2007 7300  11 01/01/1990 67
 09/25/1999 07:00 09/25/1999 15:30 1500 070  08/04/2007 1540  11 01/01/1990 67
 05/18/2002 07:00 05/18/2002 15:30 1100 050  08/04/2007 1130  11 01/01/1990 67
 10/18/2003 07:00 10/18/2003 15:30 7000 070  08/04/2007 1540  11 01/01/1990 67
 12/20/2003 07:00 12/20/2003 15:30 7000 070  08/04/2007 7500  11 01/01/1990 67
 06/12/2004 07:00 06/12/2004 15:30 7000 060  08/04/2007 7500  11 01/01/1990 67
 11/01/2004 00:00 11/01/2004 00:00      62E  08/04/2007 1130  11 01/01/1990 67
 11/02/2004 00:00 11/02/2004 00:00      62E  08/04/2007 1130  11 01/01/1990 67
(N)ext (P)rev (U)pdate (A)dd (C)opy (F)ind (D)el (O)utput (T)ext (X)Crew (H)elp
(E)xtend (1)Up (2)Down (3)Pg Back (4) Pg Frwd (5)Beg (6)End (R)eturn: F

As you can see the point [3, 22] corresponds to line Skill: . At the failing screen it is empty because of some unknown reason and the same screen in manual run this line contains: Skill: 1130 IT SOFTWARE DEVEL.

I suspect that this error is not related with my update. Don't you know why it happens?

LE: GES redacted customer personnel info.

I've ran regression testing 3rd time. And the test tc_codes_employees_20 has passed. So, we can assume that update has passed regression testing. Can I commit it?

I suspect that this error is not related with my update. Don't you know why it happens?

We don't know for sure. I suspect that there is a flaw in the terminal software that drops characters from time to time. These sort of errors seem to happen more frequently when the system has a heavy load, so the code may be somehow sensitive to that condition.

Someday we will have someone dig into this. The problem is that it cannot be easily recreated, so debugging it will likely involve instrumenting the terminal source code to output log entries when unexpected conditions occur and interpreting the results that are matched up with failing test runs.

So, we can assume that update has passed regression testing. Can I commit it?

Actually, don't you have to merge up to the latest bzr revision?

OK, you can commit vig_upd20150220a.zip.

Current update has passed full testing and committed to rev #10766.

In your next update, please resolve these:

  [javadoc] /home/ges/projects/p2j/src/com/goldencode/p2j/ui/MenuWidget.java:139: warning - Tag @link: can't find findMenuItem(String, MenuContainerWidget) in com.goldencode.p2j.ui.MenuWidget
  [javadoc] /home/ges/projects/p2j/src/com/goldencode/p2j/ui/SubMenuWidget.java:100: warning - Tag @link: can't find findMenuItem(String, MenuContainerWidget) in com.goldencode.p2j.ui.MenuWidget

Please post a list of the remaining features needing to be implemented, enhanced and/or bugs needing fixing. You can exclude GUI support from the list.

At this point are ChUI menus fully functional?

Is there any error handling, event or other missing functionality? I remember some todos in the ThinClient.destroyMenu() method.

Please add the following attributes next:

MENU: FIRST-CHILD, LAST-CHILD
SUB-MENU: FIRST-CHILD, LAST-CHILD, NEXT-SIBLING, PREV-SIBLING
MENU-ITEM (CHECKED, READ-ONLY and SUBTYPE are done, right?): ACCELERATOR, TOGGLE-BOX, NEXT-SIBLING, PREV-SIBLING

Make sure that these events are supported:

MENU: MENU-DROP
SUB-MENU: MENU-DROP
MENU-ITEM: CHOOSE, VALUE-CHANGED

Have you tested colors, toggle-boxes, accelerators, read-only, rule, skip?

You're right. Some of noted points are not implemented. See below

Greg Shah wrote:

At this point are ChUI menus fully functional?

No. I can't finish client-side part because of event-processing. I recall, that Constantin is working on common event mechanism for menus and comboboxes.

Is there any error handling, event or other missing functionality? I remember some todos in the ThinClient.destroyMenu() method.

Probably. It is hard to say without working events in GUI and good testing. About destroyMenu I'm not sure of how to implement it.

Please add the following attributes next:

MENU: FIRST-CHILD, LAST-CHILD
SUB-MENU: FIRST-CHILD, LAST-CHILD, NEXT-SIBLING, PREV-SIBLING

MENU-ITEM (CHECKED, READ-ONLY and SUBTYPE are done, right?): ACCELERATOR, TOGGLE-BOX, NEXT-SIBLING, PREV-SIBLING

READ-ONLY, SUBTYPE, ACCELERATOR, TOGGLE-BOX are implemented.

Make sure that these events are supported:

MENU: MENU-DROP
SUB-MENU: MENU-DROP
MENU-ITEM: CHOOSE, VALUE-CHANGED

Waiting for common event-processing mechanism.

Have you tested colors, toggle-boxes, accelerators, read-only, rule, skip?

No, I stopped at event-processing. I'm doing what is possible without events at this moment.

At this point are ChUI menus fully functional?

No. I can't finish client-side part because of event-processing. I recall, that Constantin is working on common event mechanism for menus and comboboxes.

Constantin's changes are all checked in as of bzr rev 10760. Anything that is not working properly at this point is something for you to work on.

About destroyMenu I'm not sure of how to implement it.

If it has no purpose, get rid of it.

Please get the ChUI menus fully functional as your top priority. I don't want to add GUI in, before the ChUI cases are all operational.

I'm stuck... I can't make menu events working. I don't fully understand how events queue works. Here are the following difficulties.

1. When we have a frame all its fields and other components are stayed "freezed" untill we call ENABLE command. Menus works differently: MENUBAR (menu for window) must be enabled right after it became displayed.
2. Lets look at the following procedure:

def menu m menubar
  menu-item mi label "&Mikki Mouse" 
  menu-item mi1 label "T&iffany".

on choose of menu-item mi
  message "AAA".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item mi.

I'm running described procedure and got the following. After menu is displayed - procedure is assumed as completed and paused before end (ThinClient.pauseBeforeEnd is called) and waiting for pressing any button to close its window. TransactionManager.popScope pops the last scope from the stack and calls pauseBeforeEnd from it.

It looks like I didn't run events cycle and didn't find a way to run it..

I'm recalling that MENUBAR drawing is placed to WindowChuiImpl.setVisible. I've tried to enclose this drawing in ThinClient.eventBracked call to push a new EventQueue to queues stack in EventManager but it didn't help because it is only a drawing processing.

Could you advice me something?

I found that during processing of ThinClient.waitFor the flag immedExit is set to true. It probably a reason.. It don't look obvious: immedExit is set as a result of:

               case FocusManager.FOCUS_UNDEF:
                  // undefined, explicit
                  immedExit = !focusManager.focusUndefExplicit(startWidget,
                                                  list,
                                                  cop,
                                                  !pendingExplicit);

Mistake..
Here is the place in ThinClient.waitFor where immedExit is set to true:

               case FocusManager.FOCUS_UNDEF:
                  // undefined, implicit
                  immedExit = !focusManager.focusUndefImplicit(list, false);
                  break;

Yes, that was the reason.. Sorry for trouble.

When I'm pressing '->' in MENUBAR first menu-item (sub-menu) is blended when loosing focus. There are several calls. One of them draws sub-menu correctly but the next one completely blend it from MENUBAR.

It sounds like there is a z-order problem and/or a problem with drawing in non-visible (covered) areas of a widget.

Code Review vig_upd20150306a.zip

This is a good step forward.

1. It seems like there should be shared code between MenuItemChuiImpl and SubMenuChuiImpl for methods like processKeyEvent(), calcPosition() and drawTitle(). What is an approach where that code can be in a common class?

2. MenuChuiImpl.processKeyEvent() is missing javadoc.

3. In multiple places of your update, you use for( instead of for (. MenuChuiImpl.processKeyEvent() is one example.

4. Menu.java ends with this: }}.

If you are stuck, please post specific questions or post more details of the problem so that we can help.

1. I have simple MENUBAR with sub-menu and menu. It is drawn succesfully. At starting moment sub-menu has focus. When I'm pressing "->" I expect that menu-item will gain focus and sub-menu will loose it, but during several redrawings sub-menu first loose focus and after some drawing disappear at all. Menu-item do not gain a focus.

In other words, there are several calls of SubMenuChuiImpl.draw method after I press "->". After one of them sub-menu loose focus and after some other of them - disappear. Why It happen?

2. I'm trying to draw sub-menu body (i.e. sub-menu items). But it doesn't appear at screen. I tried several variants: manually set correct coordinates, manually set color to Color.NORMAL. Nothing is drawn.

It looks like I don't understand something conceptual.

P.S. About z-order. I didn't find something that could be a reason there. More over there are no frames at all, one window. I think z-order mechanism isn't used in my case.

Here is the testcase:

def sub-menu sm
  menu-item mi1 label "Hekkle Berry F&in".

def menu m menubar
  sub-menu sm label "Subm&enu".
  menu-item mi label "&Mikki Mouse".

on choose of menu-item mi
  message "AAA".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item mi.

I've analyzed DoubleBufferedTerminal. There is the following conception. There are 2 screen buffers: real screen buffer and cached previous value. The goal is to update only minimal area of the screen where changes was really happen. These changes are defined as a difference between actual screen buffer and cached previous buffer. I suspected that my code leads to incorrect situation in this buffers. I've tried other places of MENUBAR drawing: draw, repaint methods in WindowChuiImpl but without success. I didn't find the reason, why it happen.

Some thoughts:

1. Menus are different from our normal containers like Frames. With menus, the contained widgets can draw outside of the container's rectangle. For this reason, you may be having clipping problems and/or the drawing algorithms may naturally be assuming that contained widgets are drawing within the contained area.

2. Even without frames being involved, you certainly have multiple widgets which need to draw in overlapping coordinates, so there is definitely a z-order requirement here. For example, you have a menu-item that must draw on top of a sub-menu that must draw on top of the menu-bar. When the sub-menu overdraws the menu-item, that sounds like a z-order issue.

3. Make sure that the visible flag is managed carefully. With frames and other "normal" widgets, the visibility is controlled by high level functions like when a VIEW is called for a frame. With menus and their contents, this is different. The contents must be made visible in response to certain events and then must be hidden again when dismissed. You must find the right places to this to occur. A mismatch could certainly cause things to not draw. I would also worry that the standard widget visibility processing may need modifications to be compatible.

4. You need to make sure that the menu-items and sub-menus are properly integrated with the focus manager such that focus can be properly changed in response to events.

For some ideas on all of the above, you might want to look at how the drop-down draws and handles events with combo-box. This is a temporary "sub-widget" control that displays when needed, displays outside of the controlling widget's dimensions, integrates with focus processing and interacts with the user, including event processing and dynamic drawing.

Greg Shah wrote:

Some thoughts:

1. Menus are different from our normal containers like Frames. With menus, the contained widgets can draw outside of the container's rectangle. For this reason, you may be having clipping problems and/or the drawing algorithms may naturally be assuming that contained widgets are drawing within the contained area.

2. Even without frames being involved, you certainly have multiple widgets which need to draw in overlapping coordinates, so there is definitely a z-order requirement here. For example, you have a menu-item that must draw on top of a sub-menu that must draw on top of the menu-bar. When the sub-menu overdraws the menu-item, that sounds like a z-order issue.

I don't fully understand this statement. MENUBAR draws it's items (menu-items and sub-menus) within its area. Dropped down sub-menu body does not overlap other menu parts: other menu-items or sub-menus of different levels. Containers (MENU and SUB-MENU) draws its children (SUB-MENU and MENU-ITEM). But none of these components could overlap. Am I wrong?

3. Make sure that the visible flag is managed carefully. With frames and other "normal" widgets, the visibility is controlled by high level functions like when a VIEW is called for a frame. With menus and their contents, this is different. The contents must be made visible in response to certain events and then must be hidden again when dismissed. You must find the right places to this to occur. A mismatch could certainly cause things to not draw. I would also worry that the standard widget visibility processing may need modifications to be compatible.

4. You need to make sure that the menu-items and sub-menus are properly integrated with the focus manager such that focus can be properly changed in response to events.

For some ideas on all of the above, you might want to look at how the drop-down draws and handles events with combo-box. This is a temporary "sub-widget" control that displays when needed, displays outside of the controlling widget's dimensions, integrates with focus processing and interacts with the user, including event processing and dynamic drawing.

Do you mean ComboBox here?

Does an "event processing model" documented somewhere? I mean in some web gui frameworks authors create a separate topic in documentation called "Request lifecycle" or something like that.

Thank you for thoughts.

I don't fully understand this statement. MENUBAR draws it's items (menu-items and sub-menus) within its area. Dropped down sub-menu body does not overlap other menu parts: other menu-items or sub-menus of different levels. Containers (MENU and SUB-MENU) draws its children (SUB-MENU and MENU-ITEM). But none of these components could overlap. Am I wrong?

I don't know. I haven't used the 4GL menus, but in every menu system I've ever used, there is often at least a partial overlap between sub-menus and their containing menus or menu-items and their containers (they often appear connected and the connection point is drawn differently than when the sub-menu is not displayed). For example, in Firefox, open View -> Toolbars. The Toolbars sub-menu slightly overlaps with the View sub-menu. If that isn't possible in the 4GL, then that is great. However, I would wonder about overlapping in cases where the right side of the screen is too close to the right side of the container for a contained sub-menu to be drawn.

Do you mean ComboBox here?

Yes. ComboBox has a special drop-down sub-widget that exhibits similar behavior to menus. It takes special processing to make that sub-widget work.

Does an "event processing model" documented somewhere? I mean in some web gui frameworks authors create a separate topic in documentation called "Request lifecycle" or something like that.

I wish we had such a thing. The best way to understand the event loop is to set a breakpoint in ThinClient.processEventsWorker() and follow the flow of the event processing. This is the top-level portion of the loop. It is very important that you step into ThinClient.processProgressEvent() and also into Window.processEvent(). The idea is to duplicate the 4GL event processing here. There can sometimes be some widget-specific or event specific behavior before triggers, then there are triggers (user-defined code blocks) executed, the event can also be treated as a high level event (F1 can also be GO) which can also execute triggers, if the triggers RETURN NO-APPLY then the default processing is generally bypassed, but otherwise the default processing for the widget is executed. Intermixed with this we have the low-level P2J events for drawing, synching, focus management and so forth.

The next buffled difficulty. Here is my menu:

Mikki Mouse Submenu  Rabbit 

Menu-item Mikki Mouse has focus right after start (title is highlighted). I press '->' and see that only first letter of Submenu is highlighted. Submenu is sub-menu. I've added to drawTitle following lines:

      Dimension titleDim = new Dimension(width(), 1);
      Rectangle titleRect = new Rectangle(this.location(), titleDim);
      EventManager.postEvent(new PaintEvent(this, titleRect));

this.location() returns the following point: [x=12, y=0], titleDim is [height=1, width=8]. Calculated titleRect is the following: [top=0, left=12, right=19, bottom=0]. These rectangle, as I think, has to conform to sub-menu title Submenu. So I expect that Submenu will be highlighted after repaint, but as a result only first letter is highlighted.

Sorry for stupid question, but where is my mistake?
Does the calculated rectangle correct?

P.S. Combobox has artifical component DropDown that conforms to expanded values list of Combobox component. My dropdown list fully differs from DropDown:
1. DropDown is scrollable, but menu is not, when sub-menu contains large number of children that more than screen lines number - the error is shown: Menu to large to display on the screen. (5902).
2. DropDown contains strings, but menu contains menu-items (components)
3. DropDown is hard linked with ComboBox

I think it will be not suitable to use DropDown and ComboBox classes for menu goals as a common mechanism. Probably implementation sub-menu window as a separate class similar to DropDown is make sense. I just suspected that it is possible without separate component. What do you think?

Separate question about events. Lets look at Button.processEvent() method:

   /**
    * Process event and dispatch it to specific method.
    * 
    * @param   event
    *          Event to process.
    */
   @Override
   public void processEvent(Event event)
   {
      super.processEvent(event);

      if (event instanceof KeyInput)
      {
         KeyInput keyEvent = (KeyInput) event;

         if ((!keyEvent.isConsumed()) &&
             keyEvent.keyCode() == Key.VK_ENTER && isEnabled())
         {
            EventManager.postEvent(new ActionEvent(this, getCommand()));
            keyEvent.consume();
         }

         return;
      }

      if (event instanceof ActionEvent)
      {
         for (ActionListener listener : listeners)
         {
            listener.onAction(((ActionEvent) event));
         }
      }
   }

What is the necessity of wrapping KeyInput event in ActionEvent here, posting new event to queue, to extract that ActionEvent from queue later but process it in the same place: the method where ActionEvent was created?

I just wanted to understand on that example where is the right place to wrap low-level events (such as KeyInput) and where is the right place to process high-level events. In described example that is the one place for both.

The first question in previous note is solved: wrong implementation of menu.width()

I think it will be not suitable to use DropDown and ComboBox classes for menu goals as a common mechanism.

I agree. The idea was for you to see how that works so that you can understand the techniques needed to have an independent drawing/events processing environment.

Probably implementation sub-menu window as a separate class similar to DropDown is make sense. I just suspected that it is possible without separate component. What do you think?

I think your current class hierarchy makes sense. And the 4GL does treat these as real widgets, so that is a good fit for us to treat them that way.

Don't try to rework your class hierarchy. Just use the ideas in the drop-down processing to help you design the menus.

The bottom and right borders of sub-menu is not drawn. I've debugged it and found that it because ScreenBitmap activated rectangles do not contain target sub-menu body rectangle. Will be correct to add sub-menu body rectangle to activated rectangles list using PaintEvent created after draw method finished?

Vadim Gindin wrote:

The bottom and right borders of sub-menu is not drawn. I've debugged it and found that it because ScreenBitmap activated rectangles do not contain target sub-menu body rectangle. Will be correct to add sub-menu body rectangle to activated rectangles list using PaintEvent created after draw method finished?

No, it will not work. These rectangles need to be collected during the event processing, not during drawing - at that time, it's too late to raise and process the PaintEvent's.

What is the necessity of wrapping KeyInput event in ActionEvent here, posting new event to queue, to extract that ActionEvent from queue later but process it in the same place: the method where ActionEvent was created?

If I recall correctly, this is because in some cases two events are raised in 4GL: one for the actual key press and, if that is not consumed, one for the widget's action (like CHOOSE). And each event can have its own trigger: that's why the ActionEvent is raised, so that the event processing will pick it up and interpret it.

Vadim Gindin wrote:

The bottom and right borders of sub-menu is not drawn. I've debugged it and found that it because ScreenBitmap activated rectangles do not contain target sub-menu body rectangle. Will be correct to add sub-menu body rectangle to activated rectangles list using PaintEvent created after draw method finished?

PS: check if for the sub-menu a PaintEvent is generated and if so, if the rectangle matches the sub-menu's dimension.

Thanks for help!

The next 2 questions.
1. Every opened sub-menu has its own focus. Lets assume we have MENUBAR with 2 opened sub-menus. In this situation focused widgets will be:
a. sub-menu in menubar
b. contained sub-menu
c. menu-item in contained sub-menu.
AbstractContainer has a field current that contains a widget having focus, so I think such situation, when a several widgets can have focus simultaneously, is supported. Am I right?

2. I'm trying to implement focusing next menu-item in opened sub-menu. I'm calling ThinClient.nextFocus(widget) in SubMenu.processKeyEvent where widget is the first menu-item in this sub-menu. This call really changes AbstractContainer.current field but later it is changed to previous value in the following place: FocusManager.focusChange(), line 1262, in the call setFocusOn(widget). I don't understand why this call is there? and what am I doing wrong, while trying to set focus?

AbstractContainer has a field current that contains a widget having focus, so I think such situation, when a several widgets can have focus simultaneously, is supported. Am I right?

If I understand your question correctly, I believe the answer is yes. The idea is that multiple levels of container can each track the child within them that has the focus. SO: the Window knows which of its contained Frames is focused and the focused Frame knows which widget is in focus. I think this is the same thing as you are doing, with the menu and sub-menus being containers.

2. I'm trying to implement focusing next menu-item in opened sub-menu. I'm calling ThinClient.nextFocus(widget) in SubMenu.processKeyEvent where widget is the first menu-item in this sub-menu. This call really changes AbstractContainer.current field but later it is changed to previous value in the following place: FocusManager.focusChange(), line 1262, in the call setFocusOn(widget). I don't understand why this call is there? and what am I doing wrong, while trying to set focus?

This one is tricky to explain because I don't have the complete answer myself. I can say this:

• The focus does get set back to the original widget at that line BUT focus will shift to the target widget if the ENTRY trigger doesn't exist on the target widget OR if the trigger does exist but does NOT use RETURN NO-APPLY. But it is the ENTRY event processing that causes the "final" focus change to occur.
• I believe this was done to get the environment/state to look correct for the ENTRY event/trigger processing. I don't have more specifics on this, but I remember it was done on purpose and at the time we didn't have a better way to resolve the issues.

I faced with NPE:

java.lang.NullPointerException
    at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:14419)
    at com.goldencode.p2j.ui.client.FocusManager.focusChange(FocusManager.java:1271)
    at com.goldencode.p2j.ui.chui.ThinClient.nextFocus(ThinClient.java:16356)
    at com.goldencode.p2j.ui.client.chui.MenuItemChuiImpl.processKeyEvent(MenuItemChuiImpl.java:140)
    at com.goldencode.p2j.ui.client.widget.TitledWindow.processKeyEvent(TitledWindow.java:297)
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.processEvent(AbstractWidget.java:1006)
    at com.goldencode.p2j.ui.client.widget.TitledWindow.processEvent(TitledWindow.java:266)
    at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:13910)

Lets look at FocusManager.focusChange():

      Widget inFocus = UiUtils.getCurrentFocus();

      ..

      // create and process the ENTRY event now. the ENTRY event will be 
      // responsible for setting focus to the required widget, if it accepts
      // it
      KeyInput entry = new KeyInput(inFocus,
                                    EventType.KEY_PRESSED,
                                    Keyboard.SE_ENTRY,
                                    true);
      tc.processProgressEvent(entry);

Here inFocus is null. I've digged into UiUtils.getCurrentFocus(); and found the following.

There are following hierarchy: Window -> BorderedPanel -> Menu -> SubMenu -> Menu-item and all of these nodes are AbstractContainer's except Menu-item. AbstractContainer has field current - current focused widget.
UiUtils.getCurrentFocus() returns null because AbstractContainer.current becomes null at some moment. It happens in AbstractContainer.focusWorker line 1076:

          ..

          for (Widget<O> widget : iter)
          {
             if (widget == current)
             {
1076:           current = null;
                return;
             }

             if (checkWidget(widget, direction))
                return;
          }

More concrete: container is Window and current was BorderedPanel and is set to null. There only one child in the Window.

I don't understand why it is needed: reset current to null especially for Window. As a result: Window.focusWorker(true) returns null when it had current=BorderedPanel.

I don't understand why it is needed: reset current to null especially for Window. As a result: Window.focusWorker(true) returns null when it had current=BorderedPanel.

I don't know the answer exactly. In think this only can happen for Window because all other widgets have parents. But I think this would only happen if some much deeper focus change processing failed when it should not have failed. Shouldn't the SubMenu (in Window -> BorderedPanel -> Menu -> SubMenu) have changed the focus and then terminated its AbstractContainer.focusWorker()? It would only have asked its parent() to next|prevFocus() if none of its children could be focused. I think you have a problem there.

Also, I do wonder if the menus should be considered part of the content pane of the window or if they should be treated separately like the message and status areas?

Constantin: do you have any thoughts on these items?

Please post the test you used to get the NPE.

Here it is:

def sub-menu sm
  menu-item mi1 label "Hekkle Berry F&in".
  menu-item mi2 label "Chuk I Gek". 

def menu m menubar
  menu-item mi label "&Mikki Mouse".
  menu-item mmi label "Rabbi&t" ACCELERATOR "PAGE-DOWN".
  sub-menu sm label "Subm&enu".

on choose of menu-item mi
  message "AAA".

on choose of menu-item mmi
  message "BBB".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item mi.

The error happens when I'm pressing down error having focus on the last menu-item in dropped-down sub-menu.

Greg Shah wrote:

I don't understand why it is needed: reset current to null especially for Window. As a result: Window.focusWorker(true) returns null when it had current=BorderedPanel.

I don't know the answer exactly. In think this only can happen for Window because all other widgets have parents. But I think this would only happen if some much deeper focus change processing failed when it should not have failed. Shouldn't the SubMenu (in Window -> BorderedPanel -> Menu -> SubMenu) have changed the focus and then terminated its AbstractContainer.focusWorker()? It would only have asked its parent() to next|prevFocus() if none of its children could be focused. I think you have a problem there.

Yes, that is probably a real reason, but I faced with this error several times - not only in my current situation. I thought that it looks strange. Ok, lets consider, if we can't change focus - just leave it as it was, why should we reset it? That is what I asked about.

I'm going to override Container.nextFocus and Container.prevFocus at least for SUB-MENU to make it simpler, because there are no necessity to call parents focused item - there are simple cyclic algorithm.

What do you think?

Also, I do wonder if the menus should be considered part of the content pane of the window or if they should be treated separately like the message and status areas?

At this moment menus are contained in the content pane.

Greg Shah wrote:

Also, I do wonder if the menus should be considered part of the content pane of the window or if they should be treated separately like the message and status areas?

Greg, I think you are onto something here... when a window has a menu, its client-area (where frames are drawn) is shifted on row down on the physical screen. See this:

def sub-menu sm
  menu-item mi1 label "Hekkle Berry F&in".

def menu m menubar
  sub-menu sm label "Subm&enu".
  menu-item mi label "&Mikki Mouse".

on choose of menu-item mi message "AAA".

def var ch as char.
form ch with frame f1 row 1 col 1.
display ch with frame f1.
pause.
assign current-window:menubar = menu m:handle.
pause.
wait-for choose of menu-item mi.
pause.

Vadim: at some point, you need to check what happens if the sub-menu is drawn over an existing frame (like in my test above).

I'll come back with some thoughts related to the focus management.

I thought that it looks strange. Ok, lets consider, if we can't change focus - just leave it as it was, why should we reset it? That is what I asked about.

I don't know why that code is there, but it is dangerous to modify it when we don't know the implications.

And I don't think menu focus processing should ever get to that code anyway.

I'm going to override Container.nextFocus and Container.prevFocus at least for SUB-MENU to make it simpler, because there are no necessity to call parents focused item - there are simple cyclic algorithm.

I don't understand why this is necessary. Perhaps I am overlooking or mistaking something. I see this code in AbstractContainer.focusWorker() (as checked in to bzr right now):

   private void focusWorker(boolean direction)
   {
      Iterable<Widget<O>> iter = (direction) ?
                                     Iterables.directFrom(widgets, current) :
                                     Iterables.reverseFrom(widgets, current);

      if (iter == null)
         throw new IllegalStateException("Focused widget does not belong to container");

      for (Widget<O> widget : iter)
      {
         if (checkWidget(widget, direction))
            return;
      }

      if (parent() != null)
      {
         if (direction)
             parent().nextFocus();
         else
             parent().prevFocus();
         return;
      }

The way I read this, the SubMenu (which is a subclass of AbstractContainer) should be the this when this method is executing. This code should already be able to shift the focus without the parent() being involved:

      for (Widget<O> widget : iter)
      {
         if (checkWidget(widget, direction))
            return;
      }

Why is checkWidget() returning false for all contained items?

It looks like the Iterables.direct|reverseFrom() code doesn't implement the "cyclic" nature of looping back around to the beginning of the elements (or vice versa). So that is why you have to add some special code.

I think we do something similar in Frame too, but I wonder what containers don't need this feature?

I'm fine with your proposed approach unless we determine that our core AbstractContainer algorithm really needs this feature too.

I have z-order difficulty and need your advice. I have 2 nested sub-menus:

Mikki Mouse Rabbit Submenu                   
                   ┌──────────────────┐      
                   │ Hekkle Berry Fin │      
                   │ Europa ->        │      
                   │ Chuk I Gek       │      
                   └──────────────────┘      

Mikki Mouse Rabbit Submenu               
                   ┌               ───┐  
                   │               in │  
                   │                  │  
                   │                  │  
                   └─               ──┘                                         

Submenu  Mikki Mouseubmenu  
┌┌─────────────┐───┐        
││ Karl Chapek │in │        
│ Hekkle Berry Fi  │        
│ Chuk I Gek       │        
└──────────────────┘        

As you can see the nested sub-menu is displayed ("Karl Chapek" menu-item) but the order of items in menu and sub-menu is corrupted: "Submenu@" has became first element in menu (was third) and its sub-menu "Europa" has became also first element in "Submenu" body (was second). See earlier screens to compare.

It happened because of z-order implementation in AbstractContainer: z-order is implemented as the order of elements in widgets list. So when I added parent().moveToZpos(this, 0) in SubMenuChuiImpl.showBody I really changed its places in widgets list and therefore affected the common drawing.

I don't know how to fix this issue. Is it possible to process z-order and drawing as separate widgets list somehow?

I've attached my current changes. Z-order change is in SubMenuChuiImpl.showBody method. Here is my testcase:

def sub-menu insm
  menu-item mi0 label "Karl Chapek".

def sub-menu sm
  menu-item mi1 label "Hekkle Berry F&in".
  sub-menu insm label "Europa".
  menu-item mi2 label "Chuk I Gek".

def menu m menubar
  menu-item mi label "&Mikki Mouse".
  menu-item mmi label "Rabbi&t" ACCELERATOR "PAGE-DOWN".
  sub-menu sm label "Subm&enu".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item mi.

Vadim, about your update:
SubMenuChuiImpl
- draw: why are you raising a PaintEvent there? When draw() is called, is too late to capture and process these events, as the current event processing loop has already finished
- calcLocation: I think your z-order problem is related to this; each time location() is called, you recompute it based on the widget's place in the parent's widget list. As the z-order changes, the location changes... which is not correct. You need to layout the menu once, and save the computed location in the AbstractWidget.origin field.

Same with dimension: I think this is set once during initial layout, and if the menu's text changes to something else, the width will not get readjusted. Please test and confirm this.

Constantin Asofiei wrote:

- draw: why are you raising a PaintEvent there? When draw() is called, is too late to capture and process these events, as the current event processing loop has already finished

I've removed this. Just missed earlier.

- calcLocation: I think your z-order problem is related to this; each time location() is called, you recompute it based on the widget's place in the parent's widget list. As the z-order changes, the location changes... which is not correct. You need to layout the menu once, and save the computed location in the AbstractWidget.origin field.

I'm working on that - looking for appropriate location. It really helps: the order is not changed now, but adding of parent().moveToZpos(this, 0); stopped help. I got the following screen when trying to display nested sub-menu:

Mikki Mouse Rabbit Submenu               
                   ┌               ───┐  
                   │               in │  
                   │                  │  
                   │                  │  
                   └─               ──┘        

Same with dimension: I think this is set once during initial layout, and if the menu's text changes to something else, the width will not get readjusted. Please test and confirm this.

You're rigth. Sub-menu width is not recalculated when its menu-item width is being changed. New title is shrinked to sub-menu width:

Submenu Mikki Mouse
┌──────────────────┐
│ Hekkle Berry Fin │
│ Uk-co ->         │
│  12345678901234567
└──────────────────┘

Submenu Mikki Mouse

                    8901234567890123456789

Submenu Mikki Mouse
┌──────────────────┐
│ Hekkle Berry Fin │
│ Uk-co ->         │
│  123456789012345678901234567890123456789
└──────────────────┘

Vadim Gindin wrote:

So my problem with z-order is stayed unsolved. Could you advice me something?

Please use the widget browser to check the runtime values for the menu (location, dimension, visibility, etc). It will help you walk the menu tree more easily.

Otherwise, debug the invalidation rectangles collected by ScreenBitmap and see which PaintEvent gets ignored and which get collected; AbstractContainer.draw is the main entry point of drawing, so debug this and check how the drawing gets performed.

When something like you posted happens, usually there is a problem in the invalidation rectangle collected in ScreenBitmap (wrong coordinates, cleared to early) or the drawing coordinates are wrong.

I've tried to add JVM option "-Dwidgetbrowser", but WidgetBrowser wasn't show. I suspect that WidgetBrowserAspect is not configured. Where it should be configured?

Vadim Gindin wrote:

I've tried to add JVM option "-Dwidgetbrowser", but WidgetBrowser wasn't show. I suspect that WidgetBrowserAspect is not configured. Where it should be configured?

The correct option is -Dwidgetbrowser=yes.

This variant also don't work. WidgetBrowserAspect.onWidgetDrawn is not called at all. I didn't find something about WidgetBrowser in docs.

Vadim Gindin wrote:

This variant also don't work. WidgetBrowserAspect.onWidgetDrawn is not called at all. I didn't find something about WidgetBrowser in docs.

In ChUI the widgets are not highlighted - just inspect the table with the widget's config state (dimension, location, etc).

Constantin, thank you for help!

Yesterday I fixed nested sub-menu drawing. Now It is shown, but also with an error:

Mikki Mouse Rabbit Submenu                     
                   ┌──────────────────┐        
                   │ Hekkle Berry Fin │        
                   │ Europa ->        │        
                   │  Chuk I Gek───┐  │        
                   └─│ Karl Chapek │──┘        
                     └─────────────┘           

Now I'm working on the next focus error. In sub-menu "Submenu" focus is drawn incorrectly: when I'm press down-array from menu-item "Hekkle Berry Fin", sub-menu "Europa" is now shown highlighted in spite of the fact, that it gains the focus. Also "Hekkle Berry Fin" still stays highlighted in spite of the fact, that it looses focus. I'm trying to find out, why it happens. I want to clarify some moments:
1. screen().at(..) calls take a Point as argument. Does this point must be in absolute coordinates? In my case coordinates are absolute and hasFocus() return correct values. So at this moment I don't have suspects.
2. During menu-items traversal at some moment I got the message "Procedure complete. Press space bar to continue" and application closes. Logs do not contain errors. Does it mean that some uncaught exception happen, that causes application quit or there could be some other reason?

Thank you

Vadim Gindin wrote:

Now I'm working on the next focus error. In sub-menu "Submenu" focus is drawn incorrectly: when I'm press down-array from menu-item "Hekkle Berry Fin", sub-menu "Europa" is now shown highlighted in spite of the fact, that it gains the focus. Also "Hekkle Berry Fin" still stays highlighted in spite of the fact, that it looses focus. I'm trying to find out, why it happens.

This may be some redrawing issue. Check if when focus is lost/gained a repaint is performed and if so, from where it is done (if it's done in an eventDrawingBracket or not).

1. screen().at(..) calls take a Point as argument. Does this point must be in absolute coordinates? In my case coordinates are absolute and hasFocus() return correct values. So at this moment I don't have suspects.

Yes, screen().at() requires absolute coordinates.

2. During menu-items traversal at some moment I got the message "Procedure complete. Press space bar to continue" and application closes. Logs do not contain errors. Does it mean that some uncaught exception happen, that causes application quit or there could be some other reason?

Usually when there is a hidden abend the client just terminates, so I doubt it was an abend. I think for some reason, your event processing loop decides to finish early.

If you are using the testcase from note 243, then I wonder if a CHOOSE event is (accidentally?) being raised for menu-item mi. If so, then the WAIT-FOR would return and the procedure would end.

Strange behavior for MENU-DROP event.
This event is sent when sub-menu (or pop-up menu) is dropped down. I've wrote simple case and trigger for that event, that outputs the string "menu-drop":

def sub-menu sm
  menu-item mism label "1984".
  menu-item mism1 label "John Fowles" toggle-box.

def menu m menubar
  sub-menu ssm label "Orwell".
  menu-item qqq label "qqq".

on menu-drop of sub-menu sm
  message "menu-drop".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

As a result the string "menu-drop" was printed twice. It looks like trigger works not only when sub-menu is dropped but when first item gaining a focus. What do you think?

The question about sub-menu width.
Constantin, you wrote, that I should set dimension field AbstractContainer.size when layouting sub-menu. In my current implementation sub-menu has title and body, i.e. body is not a separate component. So width and height of sub-menu depends on its state: whether body is visible or not. So what to do when body is being shown or being hidden? Should I change that field: AbstractContainer.size?

By the way, why AbstractContainer.dimension() creates new Dimension instead of returning AbstractContainer.size? That probably confused me at start of implementation.

Vadim Gindin wrote:

Strange behavior for MENU-DROP event.
This event is sent when sub-menu (or pop-up menu) is dropped down. I've wrote simple case and trigger for that event, that outputs the string "menu-drop":
[...]

I can't run this test on lindev01, I get this error:

│DEFINE sub-menu ssm statement missing. (3554)                              │
│ │**  Could not understand line 5. (196)                                     │

Vadim Gindin wrote:

The question about sub-menu width.
Constantin, you wrote, that I should set dimension field AbstractContainer.size when layouting sub-menu. In my current implementation sub-menu has title and body, i.e. body is not a separate component. So width and height of sub-menu depends on its state: whether body is visible or not. So what to do when body is being shown or being hidden? Should I change that field: AbstractContainer.size?

I don't understand why you need to change the size when the body is shown/hidden. The correct way of showing/hiding a widget is set its visible flag to true or false. Relying on the widget's size to draw/not to draw it doesn't look good.

By the way, why AbstractContainer.dimension() creates new Dimension instead of returning AbstractContainer.size? That probably confused me at start of implementation.

1. you don't want to expose the size instance to other code, as you will lose control over it. The only way to change it is via the setWidth/Height APIs
2. the width() and height() APIs can be overridden by sub-classes to return some custom value, thus the size field may not be in sync with what these methods return

Constantin Asofiei wrote:

Vadim Gindin wrote:

Strange behavior for MENU-DROP event.
This event is sent when sub-menu (or pop-up menu) is dropped down. I've wrote simple case and trigger for that event, that outputs the string "menu-drop":
[...]

I can't run this test on lindev01, I get this error:
[...]

It is just my misprint in sub-menu reference. Sorry. Fixed:

def sub-menu sm
  menu-item mism label "1984".
  menu-item mism1 label "John Fowles" toggle-box.

def menu m menubar
  sub-menu sm label "Orwell".
  menu-item qqq label "qqq".

on menu-drop of sub-menu sm
  message "menu-drop".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Constantin Asofiei wrote:

Vadim Gindin wrote:

The question about sub-menu width.
Constantin, you wrote, that I should set dimension field AbstractContainer.size when layouting sub-menu. In my current implementation sub-menu has title and body, i.e. body is not a separate component. So width and height of sub-menu depends on its state: whether body is visible or not. So what to do when body is being shown or being hidden? Should I change that field: AbstractContainer.size?

I don't understand why you need to change the size when the body is shown/hidden. The correct way of showing/hiding a widget is set its visible flag to true or false. Relying on the widget's size to draw/not to draw it doesn't look good.

Hiding sub-menu body doesn't mean hiding sub-menu itself. Sub-menu body is not a separate widget. In my procedure sub-menu widget will consist of label "Orwell" and body with menu-items "1984" and "John Fowles". At least that was my approach at this moment. Do you advice me to assume that visible flag in my case will control only body visibility?

By the way, why AbstractContainer.dimension() creates new Dimension instead of returning AbstractContainer.size? That probably confused me at start of implementation.

There are two reasons:

1. you don't want to expose the size instance to other code, as you will lose control over it. The only way to change it is via the setWidth/Height APIs
2. the width() and height() APIs can be overridden by sub-classes to return some custom value, thus the size field may not be in sync with what these methods return

I've seen a contradiction here in the following circumstance: if I will override width() and height() methods in my sub-class, than AbstractContainer.dimension() method will return the other dimension than stored in size field. That was happen for me because I intuitively overridden width(), height() and dimension() methods so It happens my implementation doesn't use AbstractContainer.size field at all.

Vadim Gindin wrote:

Hiding sub-menu body doesn't mean hiding sub-menu itself. Sub-menu body is not a separate widget. In my procedure sub-menu widget will consist of label "Orwell" and body with menu-items "1984" and "John Fowles". At least that was my approach at this moment. Do you advice me to assume that visible flag in my case will control only body visibility?

I think you need to split the sub-menu and its body into separate widgets. Check how the DropDown class works with the ComboBox widget in P2J.

If I understand correctly, currently the SubMenu class is responsible for drawing both the sub-menu's text and it's body, in case is activated, correct?

I've seen a contradiction here in the following circumstance: if I will override width() and height() methods in my sub-class, than AbstractContainer.dimension() method will return the other dimension than stored in size field. That was happen for me because I intuitively overridden width(), height() and dimension() methods so It happens my implementation doesn't use AbstractContainer.size field at all.

Why do you need to override dimension() too - doesn't this give the same values as width() and height(), in the SubMenuChuiImpl class?

Constantin Asofiei wrote:

I think you need to split the sub-menu and its body into separate widgets. Check how the DropDown class works with the ComboBox widget in P2J.

I already did it earlier. See note #225. There some differences. The main difference is a sub-tree of components corresponds the source sub-tree. DropDown contains not components but strings. Sure some analogy could be implemented, but I'll have to change all my client implementation but there is no time for that.

If I understand correctly, currently the SubMenu class is responsible for drawing both the sub-menu's text and it's body, in case is activated, correct?

Yes.

Why do you need to override dimension() too - doesn't this give the same values as width() and height(), in the SubMenuChuiImpl class?

It just happen in evaluational manner. I think I can get rid of it.

Yesterday I worked on some unusual focusing behavior. When some sub-menu is dropped down first time, its first element gains a focus, but when sub-menu is dropped down second and other times - first element does not gain a focus. I thought that when sub-menu body becomes hidden its focus is reset to null and do not reinitialized next time.

I fixed this behavior except one interesting moment: at the second and following dropping down of sub-menu body there are message "Press space bar to continue." appears in the status/message area:

Orwell qqq
┌───────┐
│ sm -> │
└─┌──────────────┐
  │ 1984         │
  │  John Fowles │
  └──────────────┘

1984
menu-drop
Press space bar to continue.

Here is my testcase:

def sub-menu sm
  menu-item mism label "1984".
  menu-item mism1 label "John Fowles" toggle-box.

def sub-menu ssm
  sub-menu sm label "sm".

def menu m menubar
  sub-menu ssm label "Orwell".
  menu-item qqq label "qqq".

on choose of menu-item mism
  message "1984".

on menu-drop of sub-menu sm
  message "menu-drop".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

I've tested this case again and found that it is related to triggers. Note that there are 2 triggers are defined. If I'll leave any one of them this peculiarity will gone (first element will gain focus each time sub-menu body is dropped and no messages will be in status/message area).

I've searched our code for this message and found, that it is used in hide* and pause* methods.

How to test what operation is really used and why and how to correctly implement it?

I would expect that the ThinClient.needPause flag would possibly be involved. Are you sure this is about the triggers and not about the hiding of the menus? When we hide frames, we pause (just like the 4GL) because the user must have a chance to see the frame before it is hidden. We also do this for down frames when they must scroll some data off (for the same reason that in the 4GL the user is given a chance to see the content). Could you have duplicated this behavior for menus when you copied frame-like behavior?

Greg Shah wrote:

I would expect that the ThinClient.needPause flag would possibly be involved. Are you sure this is about the triggers and not about the hiding of the menus? When we hide frames, we pause (just like the 4GL) because the user must have a chance to see the frame before it is hidden. We also do this for down frames when they must scroll some data off (for the same reason that in the 4GL the user is given a chance to see the content). Could you have duplicated this behavior for menus when you copied frame-like behavior?

If I comment both triggers from previous procedure or only one of them - the message "Press space bar to continue" is gone. So triggers at least influence on this behavior.

As I remember pause in hiding frame causes the user to press some additional key to really hide a frame. In menus cases there are no such effect. It looks like only a message is displayed. On the other side it also can look like Progress makes pause before focusing default (first) menu-item and outputs this message. This scenario is really similar to what you've wrote. Thank you, I'll think about it.

P.S. As I remember I didn't copied frame-like behavior at all. There are no scroll data or other so probably there was no necessity to do that.

Another question about pop-up menu hierarchy. Pop-up menu can be assigned for the following widgets: Browse, Button, Combo-box, Dialog-box, Editor, Fill-in, Frame, Radio-set, Selection-list, Slider, Toggle-box, Window. Some of them aren't Container descendants (for example Button). It means that I can't call menu.setParent(button) because of that reason.

The focus methods depend on parent() method, for example setFocus, hasFocus and so on. So these methods will return incorrect/unexpected result for the cases when menu is assigned to simple widget, not a Container. Here is 2 ways to fix this issue:
1. For all possible pop-up menu owners implement Container interface.
- not fully corresponds to source widgets hierarchy, because menu is assigned to new property popup-menu not to parent property
+ focusing should work "from the box" i.e. without additional work.
2. For Menu classes override necessary focus* methods adding owner processing logic there.
- necessity to fix focusing methods to make focusing work.
+ fully corresponds to source widgets hierarchy, because menu is assigned to new property popup-menu not to parent property
3. A cardinal way is to show pop-up menus in a separate window, like it happens for AlertBox
Here is some similarity with alert-box. May be it would be simpler.

What do you think?

I think your option 3 is probably the best. The normal focus processing should be "suspended" during menu processing because the user cannot operate outside of the menus without also dismissing the menus. In other words, we are not in the normal tab/back-tab focus processing when menus are being traversed. Keeping a separate window that gets dismissed seems to be a good way to do this. The menu processing doesn't depend on the widget type that is the owner of the popup, right? For this reason, we don't want to put menu-specific logic in those widgets and we want to minimize changes to those widgets.

Code Review vig_upd20150408a.zip

1. It seems to me that there is quite a bit of potentially generic functionality that is being implemented in ChUI-specific classes. For example, the MenuChuiImpl, SubMenuChuiImpl and@MenuItemChuiImpl@ has key event processing, an event listener and layout processing which may not be specific to ChUI and/or could be made general purpose. By implementing everything in the ChUI layer, you make it much harder to implement GUI.

2. Why are we registering a ChUI-specific class (PopupListener) in a generic class (ButtonImpl)? This may just be part of problem 1 above because if the PopupListener is not ChUI specific then it is not a problem to reference it from ButtonImpl.

3. I don't think we always want to have the popup listener registered for every button. Shouldn't we only worry about that if the button actually has a popup menu?

4. My biggest concern with the focus processing changes in AbstractContainer is that it now has logic hard coded that is specific to its own child classes. Usually this is design approach to avoid unless there is no other alternative.

5. Why are you using tc.isChui() in FocusManager? That code should not be ChUI-specific.

6. MenuItemConfig.setModified() doesn't actually save the modified flag itself. Shouldn't it do the same thing as BaseConfig.setModified()? And I wonder if setModified() should really be implemented at WidgetConfig instead?

7. I'm wondering if the menu widgets should inherit from BaseEntity/BaseConfig? You are adding support for attributes like color, font, modified... which are already implemented in BaseEntity.

8. MenuItemConfig.is{Normal|Rule|Skip}() should not exist in a config class. Please move these to the widget itself.

9. ButtonImpl, MenuChuiImpl, SubMenuChuiImpl, MenuItemChuiImpl, Menu, SubMenu, AbstractContainer imports should use wildcards.

10. MessageAreaImpl is missing a history entry.

Greg Shah wrote:

Code Review vig_upd20150408a.zip

1. It seems to me that there is quite a bit of potentially generic functionality that is being implemented in ChUI-specific classes. For example, the MenuChuiImpl, SubMenuChuiImpl and@MenuItemChuiImpl@ has key event processing, an event listener and layout processing which may not be specific to ChUI and/or could be made general purpose. By implementing everything in the ChUI layer, you make it much harder to implement GUI.

I hadn't have a goal to make the most of logic in ChUI-specific classes. I'm not sure about location, dimension and other related methods, if this logic is generic, but I'll move event processing to base classes. done.

2. Why are we registering a ChUI-specific class (PopupListener) in a generic class (ButtonImpl)? This may just be part of problem 1 above because if the PopupListener is not ChUI specific then it is not a problem to reference it from ButtonImpl.

Ok, there are only a different key: ESC-U for ChUI and SHIFT-F10 for GUI. Done.

3. I don't think we always want to have the popup listener registered for every button. Shouldn't we only worry about that if the button actually has a popup menu?

I tried to add the check to initialize method and to constructors, but I found that popupMenuId in those places contains not initialized value. So I can't check it there - before adding listener.

4. My biggest concern with the focus processing changes in AbstractContainer is that it now has logic hard coded that is specific to its own child classes. Usually this is design approach to avoid unless there is no other alternative.

I'll check it again.

5. Why are you using tc.isChui() in FocusManager? That code should not be ChUI-specific.

Removed.

6. MenuItemConfig.setModified() doesn't actually save the modified flag itself. Shouldn't it do the same thing as BaseConfig.setModified()? And I wonder if setModified() should really be implemented at WidgetConfig instead?

done.

7. I'm wondering if the menu widgets should inherit from BaseEntity/BaseConfig? You are adding support for attributes like color, font, modified... which are already implemented in BaseEntity.

There were several reasons: BaseConfig contains many attributes that are not used in menu widgets, some attributes like parent are used differently in menus; and some other reasons.

8. MenuItemConfig.is{Normal|Rule|Skip}() should not exist in a config class. Please move these to the widget itself.

done.

9. ButtonImpl, MenuChuiImpl, SubMenuChuiImpl, MenuItemChuiImpl, Menu, SubMenu, AbstractContainer imports should use wildcards.

done.

10. MessageAreaImpl is missing a history entry.

done.

I'll post update later.

The question about conversion bug with handles. Lets look at the following procedure:

def menu m.
def var mi as handle.

create menu-item mi
   assign parent = menu m:handle
          label = "Dynamic mi".

Here we have menu reference: menu m. We should get the attribute "handle" value from this reference. This attribute is unusual. I expect that this attribute "handle" reference should be converted to m.asWidgetHandle() as in case of any other widget type. Am I right? It doesn't work at this moment and I can't find where it is processed in rules. Could you help me?

Search on prog.kw_handle in methods_attributes.rules. There are places there where we emit a special accessor to get the handle from a system handle. There is also the location with this:

               <!-- let it have a methodText, so that it can emit to readOnlyError, when is on the
                    left-side of an assignment. else, it will be ignored. -->
               <rule>ftype == prog.kw_handle and isAssign
                  <action>hwrap = ""</action>
                  <action>methodText = "readOnlyError"</action>
               </rule>

This ignores the hw_handle for the common case because generally we pass around the widget/resource itself and not a handle. A handle type can be directly assigned (using assign()) from a resource instance and we are trying to avoid extra layers of wrapping/unwrapping.

Even assigning the parent of a widget is done using setParent(GenericWidget widget) in BaseEntity, bypassing the need to wrap the widget in a handle. For instances that are already a handle (like a handle variable), we have setParentHandle(handle).

We do have some special cases where we emit frames using the CommonFrame interface instead of emitting an asWidget() accessor. But for non-frame widgets generally we just emit the widget's accessor (set in frame_scoping.rules). The widget reference itself is emitted in widget_references.rules. Menus don't have a frame-id annotation, so they get bypassed by this code.

You have menu-specific code in widget_references.rules that will probably need updates.

From time to time I'm facing with the following conversion problem. Look at the following procedure:

def button b.
def frame f b.
def var h as widget-handle.
assign h = b:handle.

I'm getting the following error when trying to convert this procedure:

------------------------------------------------------------------------------
Code Conversion Annotations
------------------------------------------------------------------------------

Optional rule set [customer_specific_annotations_prep] not found.
./menu/but_handle.p
EXPRESSION EXECUTION ERROR:
---------------------------
throwException(sprintf(spec, bufname, reftype), this)
^  { Cannot find buffer named p2j_test.book_p2j_test.book for ref type 21! [FIELD_INT id <257698037802> 0:0] }
---------------------------
Elapsed job time:  23:00:00.810
ERROR:
java.lang.RuntimeException: ERROR!  Active Rule:
-----------------------
      RULE REPORT      
-----------------------
Rule Type :   WALK
Source AST:  [ book-id ] BLOCK/STATEMENT/DEFINE_FRAME/FORM_ITEM/FIELD_INT/ @0:0 {257698037802}
Copy AST  :  [ book-id ] BLOCK/STATEMENT/DEFINE_FRAME/FORM_ITEM/FIELD_INT/ @0:0 {257698037802}
Condition :  throwException(sprintf(spec, bufname, reftype), this)
Loop      :  false
--- END RULE REPORT ---

    at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:1006)
    at com.goldencode.p2j.convert.ConversionDriver.processTrees(ConversionDriver.java:973)
    at com.goldencode.p2j.convert.ConversionDriver.back(ConversionDriver.java:834)
    at com.goldencode.p2j.convert.ConversionDriver.main(ConversionDriver.java:1799)
Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:1 [FIELD_INT id=257698037802]
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:225)
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:160)
    at com.goldencode.p2j.pattern.PatternEngine.apply(PatternEngine.java:1515)
    at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1413)
    at com.goldencode.p2j.pattern.PatternEngine.processAst(PatternEngine.java:1361)
    at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:974)
    ... 3 more
Caused by: com.goldencode.expr.ExpressionException: Expression execution error @1:1
    at com.goldencode.expr.Expression.execute(Expression.java:434)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:401)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.Rule.executeActions(Rule.java:640)
    at com.goldencode.p2j.pattern.Rule.coreProcessing(Rule.java:609)
    at com.goldencode.p2j.pattern.Rule.apply(Rule.java:440)
    at com.goldencode.p2j.pattern.RuleContainer.apply(RuleContainer.java:530)
    at com.goldencode.p2j.pattern.RuleSet.apply(RuleSet.java:1)
    at com.goldencode.p2j.pattern.AstWalker.walk(AstWalker.java:212)
    ... 8 more
Caused by: com.goldencode.p2j.pattern.CommonAstSupport$UserGeneratedException: Cannot find buffer named p2j_test.book_p2j_test.book for ref type 21! [FIELD_INT id <257698037802> 0:0]
    at com.goldencode.p2j.pattern.CommonAstSupport$Library.throwException(CommonAstSupport.java:2227)
    at com.goldencode.expr.CE3328.execute(Unknown Source)
    at com.goldencode.expr.Expression.execute(Expression.java:341)
    ... 24 more

Because of some reason b is treated as FIELD_INT. Lets look at corresponding AST:

    <ast col="0" id="257698037767" line="0" text="statement" type="STATEMENT">
      <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
      <ast col="1" id="257698037768" line="3" text="def" type="DEFINE_FRAME">
        <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        <ast col="11" id="257698037771" line="3" text="f" type="SYMBOL">
          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        </ast>
        <ast col="13" id="257698037774" line="3" text="b" type="TABLE">
          <annotation datatype="java.lang.String" key="schemaname" value="p2j_test.book"/>
          <annotation datatype="java.lang.String" key="bufname" value="p2j_test.book"/>
          <annotation datatype="java.lang.String" key="dbname" value="p2j_test"/>
          <annotation datatype="java.lang.Long" key="recordtype" value="12"/>
          <annotation datatype="java.lang.Boolean" key="reachable" value="true"/>
        </ast>
      </ast>
    </ast>

b ast node has type "TABLE"..

Here is the *.parser file:

block [BLOCK] @0:0
   statement [STATEMENT] @0:0
      def [DEFINE_BUTTON] @1:1
         b [SYMBOL] @1:12
   statement [STATEMENT] @0:0
      def [DEFINE_FRAME] @3:1
         f [SYMBOL] @3:11
         b [TABLE] @3:13
   statement [STATEMENT] @0:0
      def [DEFINE_VARIABLE] @5:1
         h [SYMBOL] @5:9
         as [KW_AS] @5:11
            widget-handle [KW_WID_HAND] @5:14
   statement [STATEMENT] @0:0
      assign [KW_ASSIGN] @7:1
         = [ASSIGN] @7:10
            h [VAR_HANDLE] @7:8
            expression [EXPRESSION] @0:0
               : [COLON] @7:13
                  b [WID_BUTTON] @7:12
                  handle [ATTR_HANDLE] @7:14

b here has also type "TABLE". It looks like parser problem..

I faced with this error before with other procedures. Could you try to convert this procedure? Will you get the same error?

This is a problem with the name resolution: in 4GL, a table can be abbreviated using only the first letter, so a find first b. will find you the first book. I think P2J is too aggressive when resolving these names; looks like 4GL looks into table names only if the name is used in a context where a table name is needed (i.e. find first b.). The define button b. is not a context where a table can be used, so the button will be created in 4GL.

For now, go around this problem by renaming your button to something else (i.e define button btn.).

Greg, I've fixed "handle" problem for menu as you advised. Now assign mi:parent = menu m:handle is converted to mi.unwrapWidget().setParentHandle(m.getSelf().asWidgetHandle());. Is it right way or you would want that I'll emit mi.unwrapWidget().setParent(m.getSelf())?

mi.unwrapWidget().setParent(m.getSelf()) seems best. It is also more consistent with the approach we use for the rest of the widgets.

What is <shadow-node .. /> in AST tree?

I made the test procedure that is similar to mine. This procedure creates dynamic frame widget (FillIn) and tries to assign a parent attribute of it:

def var somstr as char.
def frame f somstr.

def var ch as handle.
create fill-in ch.

assign ch:parent = frame f:first-child.

ch is usual frame-level widget and for ch the last statement is converted also as "setParentHandle": ch.unwrapWidget().setParentHandle(fFrame.getFirstChild());

Converted class also has compile error here because fFrame.getFirstChild() returns GenericWidget. So It seems that frame widgets do not process parent attribute somehow separately than other attributes. I also didn't find such rules for frame widgets.

--

setParentHandle method is emitted in /rules/convert/method_attributes.rules, line 2673:

               <rule>ftype == prog.kw_parent
                 <action>methodText = "getParentHandle"</action>
                  <rule>isAssign
                     <action>methodText = "setParentHandle"</action>
                  </rule>
               </rule>

I tried to paste some AST type specific logic here to emit "setParent" instead of "setParentHandle" here when widget is a menu-item or sub-menu, but I faced with a problem (see below). Here is the AST sub-tree of the last assignment statement.

   <ast col="0" id="249108103209" line="0" text="statement" type="STATEMENT">
      <ast col="1" id="249108103210" line="10" text="assign" type="KW_ASSIGN">
        <annotation datatype="java.lang.Boolean" key="bracket" value="false"/>
        <ast col="18" id="249108103213" line="10" text="=" type="ASSIGN">
          <annotation datatype="java.lang.Long" key="peerid" value="253403070504"/>
          <ast col="10" id="249108103216" line="10" text=":" type="COLON">
            <ast col="8" id="249108103217" line="10" text="mi" type="VAR_HANDLE">
              <annotation datatype="java.lang.Long" key="oldtype" value="2400"/>
              <annotation datatype="java.lang.Long" key="refid" value="249108103191"/>
              <annotation datatype="java.lang.Long" key="peerid" value="253403070506"/>
            </ast>
            <ast col="11" id="249108103219" line="10" text="parent" type="ATTR_HANDLE">
              <annotation datatype="java.lang.Long" key="oldtype" value="1864"/>
            </ast>
          </ast>
          <ast col="0" id="249108103221" line="0" text="expression" type="EXPRESSION">
            <ast col="31" id="249108103222" line="10" text=":" type="COLON">
              <ast col="20" id="249108103223" line="10" text="sub-menu" type="KW_SUB_MENU">
                <ast col="29" id="249108103226" line="10" text="sm" type="WID_SUB_MENU">
                  <annotation datatype="java.lang.String" key="first-menu-path" value="m"/>
                </ast>
              </ast>
              <ast col="32" id="249108103228" line="10" text="handle" type="ATTR_HANDLE">
                <annotation datatype="java.lang.Long" key="oldtype" value="1467"/>
              </ast>
            </ast>
          </ast>
        </ast>

As you can see from AST mi has type VAR_HANDLE. The piece of source menu procedure:

create menu-item mi.

assign mi:parent = sub-menu sm:handle.

I tried to find a way to get a real widget type from a handle "mi" but unsuccessfully. "refid" annotation only references to a shadow-node in the bottom of tree. "oldtype" is a SYMBOL.

Conclusion.
As a result I didn't find how "parent" attribute is process for frame-level widgets and I also can't define real widget type from menu-item handle "mi" to paste type specific logic in a rule that emits "setParentHandle". More, I found that the analogue procedure also converted with compile error and with "setParentHandle". Could you help me?

The question about mnemonic bug. Mnemonic is a char in the menu-item title that is a menu-item hot-key. It is specified as a character following after '&' symbol in a title. For example, assume we have "Me&nu item" menu-item. So mnemonic for this menu-item is 'n'. If '&' is not specified, than the first symbol is assumed as mnemonic, i.e. 'M' in this example.

There are non-obvious behavior of disabled flag. If menu-item is initially defined with this flag, than Progress assumes, that mnemonic is default (first character) even if '&' is specified in a menu-item title. In other words, if "Me&nu item" is defined as following menu-item mi label "Me&nu item" disabled, than Progress will not see 'n' it will assume the first letter 'M' as mnemonic independently of real "disabled" state in runtime, i.e. even we will enable menu-item in runtime.

Moreover in this case menu-item will be drawn as follows Me&nu item without focus, and _Menu item with focus.

I'll probably need additional state attribute in config class, for example "initialDisabled" and set it only during construction. I'll also find a right place where it happens (only construction place). What place it could be? I thought that I'll create a separate method setDisabled that will be emitted only from rules and will not be called from any other place. What do you think?

Vadim Gindin wrote:

What is <shadow-node .. /> in AST tree?

These are nodes that store hidden tokens like whitespace and comments. They do not get walked in most cases so they generally can be ignored.

As you can see from AST mi has type VAR_HANDLE.

That is as it should be. mi is really a reference to a handle variable, right?

I tried to find a way to get a real widget type from a handle "mi" but unsuccessfully.

Handles can contain anything, even non-widgets. It is something that is only known at runtime. You can't reliably determine widget type from a handle var in the tree.

"refid" annotation only references to a shadow-node in the bottom of tree.

This part is strange. It should be referencing the original define variable statement.

Please confirm this.

However, this would still not solve your problem since it will just tell you that the var is a handle type.

"oldtype" is a SYMBOL.

This is expected and is OK.

As a result I didn't find how "parent" attribute is process for frame-level widgets and I also can't define real widget type from menu-item handle "mi" to paste type specific logic in a rule that emits "setParentHandle". More, I found that the analogue procedure also converted with compile error and with "setParentHandle". Could you help me?

setParentHandle() should be used to set the parent when the type is a handle. When the type is a GenericWidget, then setParent() should be used.

We must make all of this the same for menus and for all other widget types, because we can't know what is the type of a handle until runtime.

Constantin is doing some work on widget parent support in #1798. He is unavailable until tomorrow, but will probably have some input for you.

I'll probably need additional state attribute in config class, for example "initialDisabled" and set it only during construction.

Yes, this makes sense.

I'll also find a right place where it happens (only construction place). What place it could be?

This seems like the menu definition itself, right?

I thought that I'll create a separate method setDisabled that will be emitted only from rules and will not be called from any other place. What do you think?

Yes, this makes sense.

1. there should be only one parentId attribute, now we have one in BaseConfig and one in MenuItemConfig. So, why not make MenuItemConfig extend BaseConfig - lots of attributes are duplicated in MenuItemConfig, beside parentId. Same for MenuItemWidget - why not extend from BaseEntity, as this already adds support for lots of attributes (parent, dcolor, fgcolor, etc).
2. about CommonWiget.get/setParent - these are for internal usage (as decided in note 239/241/244 of #2068). The conversion rules need to emit get/setParentHandle, for all PARENT attribute usage (acting as a getter or setter). If you find a case where the setParentHandle receives a CommonWidget instead of a handle, you can add a CommonWidget.setParentHandle(CommonWidget) version, so the compile works.
3. when the setter is called, is the widget's responsibility to validate that the argument is a widget of expected type. So doing this work in MenuItemWidget looks OK, but if the same error number (but with different text) is shown when PARENT fails validation (with other widgets), we may want to abstract this in BaseEntity or GenericWidget

Vadim Gindin wrote:

P.S. Constantin, about base classes for config and MenuItemWidget. I can't remember concrete reasons.

Can the reason be related to the fact that menu widgets don't have coordinate/size attributes?

Can it really be a potential problem? Will you insist to fix it?

I don't like it when we repeat things, this is a bad practice which in the long term will bring more harm than good. Plus, if at some point other menu-related attributes are needed (like NAME, WINDOW, etc), we will need to duplicate work across the config instances, instead of having only a central point for them.

An alternative would be to take the fields from SubMenuConfig and MenuItemConfig which reside also in BaseConfig and move them from BaseConfig to WidgetConfig, but this would confuse things for BaseEntity and other classes which inherit directly from GenericWidget.

You can continue your work without this change for now, but we should find a way to improve this (or if we decide to leave it as is, document this in BaseConfig/BaseEntity/etc so that future readers can find easily that this duplication was done and why).

• you have the SubMenuConfig.displayed field: if this is something that the server-side never reads or changes, then mark it as transient and do not transfer it via applyConfig/writeExternal/readExternal. This will eliminate it from the config auto-sync and will reside only on the client-side.
• MenuItemConfig.mnemonic - this is a Character, but you (de)serialize it as native char - if at some point the field becomes null, we will have an unexpected NPE during unboxing.

As I stated in note 271 item 7, I really think that inheriting from BaseEntity/BaseConfig does make sense. Otherwise there is too much duplicate code.

Please do go ahead and change this now.

1. The trigger for sub-menu "sm" is working twice. Why? It prints "menu-drop" string twice first time sub-menu is dropped. Only menu-bar is visible
2. Then if I'll choose menu-item "1984" - opened sub-menus become closed. When I'm opening sub-menu "sm" next-time (each next-time), the following happens:
• trigger of sub-menu "sm" prints "menu-drop" first time.
• pause happen with the message in status line: "Press space bar to continue.", sub-menu "sm" becomes opened but no item is focused yet.
• When I press space bar, the trigger of sub-menu "sm" prints "menu-drop" second time and first item of sub-menu "sm" gains focus.

The first question is why the trigger of sub-menu "sm" works twice? It looks like pause happens between 2 triggers runs. Could you help me to interpret it and find a way to reproduce this behavior?

P.S. I've change base classes of MenuItemWidget and MenuItemConfig to BaseEntity/BaseEntityConfig. But I can't do the same for SubMenu because there are common ancestor of this class and Menu: MenuContainerWidget and those configs are different. MenuConfig does not contain color attributes and other.

Could you help me to interpret it?

P.S. I've change base classes of MenuItemWidget and MenuItemConfig to BaseEntity/BaseEntityConfig. But I can't do the same for SubMenu because there are common ancestor of this class and Menu: MenuContainerWidget and those configs are different. MenuConfig does not contain color attributes and other.

I think the MenuContainerWidget needs to inherit from BaseEntity. By the way, the menu widget in the 4GL does in fact support both DCOLOR and PFCOLOR. So it seems this is the correct thing to do.

Vadim Gindin wrote:

I'm testing pausing bug from the note №265 and I need help. Here is what I've founded during today's testing.

1. The trigger for sub-menu "sm" is working twice. Why? It prints "menu-drop" string twice first time sub-menu is dropped. Only menu-bar is visible
2. Then if I'll choose menu-item "1984" - opened sub-menus become closed. When I'm opening sub-menu "sm" next-time (each next-time), the following happens:
• trigger of sub-menu "sm" prints "menu-drop" first time.
• pause happen with the message in status line: "Press space bar to continue.", sub-menu "sm" becomes opened but no item is focused yet.
• When I press space bar, the trigger of sub-menu "sm" prints "menu-drop" second time and first item of sub-menu "sm" gains focus.

Please post your merged code.

Here it is:

def sub-menu sm
  menu-item mism label "1984".
  menu-item mism1 label "John Fowles" toggle-box.

def sub-menu ssm
  sub-menu sm label "sm".

def menu m menubar
  sub-menu ssm label "Orwell".
  menu-item qqq label "qqq".

on menu-drop of sub-menu sm
  message "menu-drop".

on choose of menu-item mism
  message "1984".

on value-changed of menu-item mism1
  message "John Fowles".

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

• Please check the header for LogicalTerminal, is not merged properly.
• MenuItemWidget - don't add multiple header numbers, if the change is part of the same work.
• MenuItemWidget.setChecked - please ensure that the error message is NOT recorded in the ERROR-STATUS:GET-MESSAGE queue, as you are using displayError. Otherwise, use recordOrShowError with the modal/prefix/isError params set to false. To check this, use the NO-ERROR clause and then check ERROR-STATUS:ERROR/NUM-MESSAGES-GET-MESSAGE
• MenuChuiImpl and others: when you are adding 1/2/3/4 units to the menu's width/location/etc, please add a comment with why this was needed (border, something else?)
• WindowChuiImpl.draw: this code I don't think is ok to be here:
  

        if (config.menubarId != 0)
        {
           Widget<ChuiOutputManager> menu = screen().getRegistry().getComponent(config.menubarId);
  
           if (menu != null)
           {
              if (menu.parent() == null)
              {
                 getContentPane().add(menu);
              }
  
              menu.setVisible(true);
           }
        }
  

  
  If repaint posts an event, this can not be captured by the EventManager, as draw is called when the event bracket ends, not during event processing... Shouldn't this better be in the WindowChuiImpl.syncWithConfig ?
• please explain why prepParentLocation and childrenLocation are needed: I guess you added this for some menu quirk, correct?
• Menu$PopupKeyListener.onKeyPressed: why not call menu.repaint() instead of menu.draw? Is it because the event is not processed in a drawing bracket?

About your testcase: in 4GL, it really seems that the MENU-DROP event is raised twice... are there other type of events for sub-menu which can be caught by a trigger? On a side note, don't forget to check RETURN NO-APPLY, when working with triggers. Also, you can use the SELF and LAST-EVENT handles to check details about the event which invoked the trigger.

Now about your pause issue. When messages are shown in the message area, in ChUI a PAUSE is raised before cleaning up the messages, if both message lines are occupied, but only if the message area is not cleaned up automatically. When the trigger is invoked, the stacktrace is this:

ThinClient.invokeTriggers(Widget, int, boolean) line: 15196
ThinClient.processProgressEvent(Event) line: 14646
ThinClient.processEventsWorker() line: 14085
ThinClient.pop() line: 13164
ThinClient.eventBracket(boolean, Runnable) line: 13147
ThinClient.eventDrawingBracket(Widget<?>, boolean, boolean, Runnable) line: 13073
ThinClient.independentEventDrawingBracket(Widget, Runnable) line: 12972
SubMenuChuiImpl(AbstractWidget<O>).processSystemKey(KeyInput) line: 1625
ThinClient.checkForSystemEvent(KeyInput) line: 12358
ThinClient.waitForEvent(int, boolean, boolean, boolean) line: 12253

Constantin Asofiei wrote:

About 0417a.zip:

• Please check the header for LogicalTerminal, is not merged properly.

done.

• MenuItemWidget - don't add multiple header numbers, if the change is part of the same work.

done.

• MenuItemWidget.setChecked - please ensure that the error message is NOT recorded in the ERROR-STATUS:GET-MESSAGE queue, as you are using displayError. Otherwise, use recordOrShowError with the modal/prefix/isError params set to false. To check this, use the NO-ERROR clause and then check ERROR-STATUS:ERROR/NUM-MESSAGES-GET-MESSAGE

I've checked it again. Error message is really not recorded to ERROR-STATUS message queue and do not change ERROR-STATUS:ERROR flag.

• MenuChuiImpl and others: when you are adding 1/2/3/4 units to the menu's width/location/etc, please add a comment with why this was needed (border, something else?)

yes, borders and some shifts. done.

• WindowChuiImpl.draw: this code I don't think is ok to be here:
  [...]
  If repaint posts an event, this can not be captured by the EventManager, as draw is called when the event bracket ends, not during event processing... Shouldn't this better be in the WindowChuiImpl.syncWithConfig ?

It seems that synWithConfig is not a good place because when repaint is needed without config changes menu will not be redrawn and will disappear.

• please explain why prepParentLocation and childrenLocation are needed: I guess you added this for some menu quirk, correct?

Yes, you are right. childrenLocation is the relative location point of children. For the Frame it is the same as location() of frame itself. bodyLocation is defined for sub-menu and it returns location of sub-menu body include borders - right under the sub-menu title. Both methods are needed because body is not a separate widget but it is a part of sub-menu.

• Menu$PopupKeyListener.onKeyPressed: why not call menu.repaint() instead of menu.draw? Is it because the event is not processed in a drawing bracket?

I've replaced it and it works. done.

...

I'm working on the last founded bug with pause. I've simplified testing procedure as it was possible:

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "qqq".
  sub-menu sbm label "Orwell".

on menu-drop of sub-menu sbm do:
  message "menu-drop".
  display self:type self:name self:label last-event:label.
end.

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

This procedure defines MENUBAR with one sub-menu, containing only one menu-item. Only one trigger is defined in this procedure and it is defined for this sub-menu (event: MENU-DROP). I'll analyze how behavior will differ depending on concrete keys, used for opening sub-menu body and closing it. I also added some debug info from self and last-event to target "menu-drop" trigger as Constantin advised earlier to find some differences in to subsequent trigger calls.

P.S. In both trigger calls all used attributes of SELF or LAST-EVENT are equal, except LAST-EVENT:LABEL, that differs depending on key used to open sub-menu body, so I didn't find the difference between 2 trigger calls.

1. How to duplicate double call of trigger?
2. Why actions for RETURN and CURSOR-DOWN keys (used to open sub-menu body) are different? What is the difference?
3. What happens after 4 iterations: why sub-menu is shown without focus?

Why this test is so mystical? I'm still don't understand Progress behavior and there for can't implement it. Please help.

Vadim Gindin wrote:

Questions.

1. How to duplicate double call of trigger?

I think you need to explicitly treat this case somewhere in TC.processProgressEvent, so that the event is triggered twice... just post it again into the queue via EventManager.postEvent (check how RETURN NO-APPLY or RETURN ERROR behaves) and it will be processed again. Or, just invoke the trigger again, for MENU-DROP event.

1. Why actions for RETURN and CURSOR-DOWN keys (used to open sub-menu body) are different? What is the difference?

You mean why there is no pause on RETURN? I think this might be because RETURN clears some window state, so that when the next message is shown, it no longer needs to do a "pause before cleanup". Or maybe is related to how the menu is exited: are there ENTRY/LEAVE events raised for menus/menu-items/etc?

1. What happens after 4 iterations: why sub-menu is shown without focus?

This is because you are using frames. 4GL shows frames in a "top-to-bottom" order. And if the screen is full (no space to show a frame bellow the last displayed frame), then a it starts back from the top, hiding any frame which are intersected - thus a pause is triggered.

For your menu tests, use this version; it avoids using frames and adds some tracking to know surely if an event was raised or not:

def var r as int init 0.

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "qqq".
  sub-menu sbm label "Orwell".

on menu-drop of sub-menu sbm do:
  message r "menu-drop".
  def var ch as char.
  ch = string(r) + " " + self:type + " " + self:name + " " + self:label + " " + last-event:label.
  put screen row (r mod default-window:screen-lines + 1) col 12 string(ch, "x(60)").
  r = r + 1.
end.

message "bogus message so that the message area is not empty".
assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Constantin Asofiei wrote:

Vadim Gindin wrote:

Questions.

1. How to duplicate double call of trigger?

I think you need to explicitly treat this case somewhere in TC.processProgressEvent, so that the event is triggered twice... just post it again into the queue via EventManager.postEvent (check how RETURN NO-APPLY or RETURN ERROR behaves) and it will be processed again. Or, just invoke the trigger again, for MENU-DROP event.

Ok.
RETURN NO-APPLY doesn't affect behavior: body is shown as it is when trigger used without RETURN NO-APPLY.
RETURN ERROR is not permitted in an user interface trigger, so cant be used (leads to error).

1. Why actions for RETURN and CURSOR-DOWN keys (used to open sub-menu body) are different? What is the difference?

You mean why there is no pause on RETURN? I think this might be because RETURN clears some window state, so that when the next message is shown, it no longer needs to do a "pause before cleanup". Or maybe is related to how the menu is exited: are there ENTRY/LEAVE events raised for menus/menu-items/etc?

It seems ENTRY/LEAVE events are not raised for menu widgets: I wrote triggers for these events and they wasn't called.

About "no pause on RETURN" strange, but after I ran your procedure, choose sub-menu "Orwell", and pressed Enter - pause happen. At the same moment in my procedure (with frames) there were no pause for RETURN.. Why it could be?
Probably some default state, because the followed RETURN commands are executed without pause. But how to find this default state, that leads to pause?

1. keycode("MENU-DROP") is -1. Why?
2. last-event:code is 1079. What is this? Key codes from 1057 to 1150 correspond to "ALT-plus corresponding character" (as documented in proghand.pdf) Where it is set?

There are some interesting information in langref in the definition of ON statement:

If event-list includes a MENU-DROP event for a menu or submenu, do not interact with
the window manager from within the trigger-block. Doing so causes the window
manager to lose control of the system, forcing you to reboot or restart the window
manager. Actions to avoid include any window system input/output (I/O) or any lengthy
processing, especially in statements that cause process interruptions, such as the PAUSE
statement with or without I/O. These also include actions that can generate a warning or
error message, forcing window system output. Use the NO-ERROR option on supported
statements to help avoid this situation. Otherwise, check valid values, especially for
run-time resources like widget handles, to prevent Progress from displaying unexpected
messages.

What does it mean "interating with the window manager from within the trigger-block"?

keycode("MENU-DROP") is -1. Why?

I assume you are describing the 4GL here? I guess the reason is simply that there is no hard coded key on the keyboard that is being pressed to generate this event. I think 4GL generally matches a key code up with a specific keyboard scan code. This is not an event that is tied to a fixed key, so there is no key code.

last-event:code is 1079. What is this?

Are you talking about the 4GL or P2J?

My understanding is that for events that are not keys or mouse, these are like a "fake" key code that represents the high level event. See Keyboard.eventCode() in P2J. We have implemented it as the same thing as LASTKEY. I'm not sure if or how we confirmed that they are equivalent but it is likely to be pretty close. See KeyReader.getLastKey().

What does it mean "interating with the window manager from within the trigger-block"?

I'm not exactly sure, but I suspect this means that you should not do anything that causes changes to window visibility, window size or window position. Their description of any window system input/output (I/O) probably means that one should not display/view/enable/message/pause any content in a window.

It seems to me that the menu-drop event is very special because it occurs in the middle of a complex set of state processing. Progress themselves essentially are telling you to do ALMOST NOTHING in such triggers or else the system can be forced to reboot/restart! In other words, I don't know that we need to duplicate every possible behavior of this event. Please avoid anything in your testcases that uses I/O including the message or pause statements. If you want to just track which menu-drop events have occurred, just store the details of the event into some data structure (e.g. an array) and then "dump" or display this later when the trigger is done.

As long as we provide the basic functionality of this event and match how Progress works in the cases which Progress has documented are safe, then we are OK.

I'll let Constantin respond on the default state question in note 298.

Greg Shah wrote:

keycode("MENU-DROP") is -1. Why?

I assume you are describing the 4GL here? I guess the reason is simply that there is no hard coded key on the keyboard that is being pressed to generate this event. I think 4GL generally matches a key code up with a specific keyboard scan code. This is not an event that is tied to a fixed key, so there is no key code.

Yes I mentioned 4GL. Ok. Are there some artificial events like MENU-DROP, that also have no key?

The problem in this case is the following. I intuitively added MENU-DROP event to Keyboard.evNames1 array. Because of that Keyboard.eventCode("MENU-DROP") will return concrete value, that depends on index of "MENU-DROP" in evNames1 array in -154. Now I see that it was wrong because MENU-DROP event has no key (key=-1).

This key is set to Event fields such as key, action or, probably, function and will be used later to lookup trigger. See ThinClient.invokeTriggers(..):

TriggerMatch result = new TriggerMatch(evtCode);
currentEventList.lookup(frameId, widgetId, true, isKey, result);

So, if I'll remove "MENU-DROP" from evNames1, than ThinClient.invokeTriggers will not find the trigger assigned for this event. If key is -1 and other int fields of Event will be -1. How to identify that some trigger is assigned exactly to "MENU-DROP" event?

last-event:code is 1079. What is this?

Are you talking about the 4GL or P2J?

My understanding is that for events that are not keys or mouse, these are like a "fake" key code that represents the high level event. See Keyboard.eventCode() in P2J. We have implemented it as the same thing as LASTKEY. I'm not sure if or how we confirmed that they are equivalent but it is likely to be pretty close. See KeyReader.getLastKey().

How should I implement LAST-EVENT:CODE for MENU-DROP event? Are there some other similar "fake" codes for some other similar event implemented?

What does it mean "interating with the window manager from within the trigger-block"?

I'm not exactly sure, but I suspect this means that you should not do anything that causes changes to window visibility, window size or window position. Their description of any window system input/output (I/O) probably means that one should not display/view/enable/message/pause any content in a window.

It seems to me that the menu-drop event is very special because it occurs in the middle of a complex set of state processing. Progress themselves essentially are telling you to do ALMOST NOTHING in such triggers or else the system can be forced to reboot/restart! In other words, I don't know that we need to duplicate every possible behavior of this event. Please avoid anything in your testcases that uses I/O including the message or pause statements. If you want to just track which menu-drop events have occurred, just store the details of the event into some data structure (e.g. an array) and then "dump" or display this later when the trigger is done.

Ok, I'll try. Constantin proposed PUT SCREEN instead of MESSAGE. Don't you know if it uses window manager?

As long as we provide the basic functionality of this event and match how Progress works in the cases which Progress has documented are safe, then we are OK.

Do you mean, that we can stay current implementation as it is implemented this moment? If yes, what about key of MENU-DROP event (see my problem in the beginning of this note)?

I'll let Constantin respond on the default state question in note 298.

I've tried do not use I/O operations in MENU-DROP trigger and accumulate strings in a global array and got the same result as for Constantin procedure. Probably PUT SCREEN do not use window manager. Never mind. I found that "default state" is a pause called from the first message statement. So it seems that pause prevent menu-item to be focused in dropped down sub-menu body. How to check it that we are "in pause"?

Are there some artificial events like MENU-DROP, that also have no key?

Yes, everything in Keyboard.evNames1. These are not real keys, but higher level "functions" (e.g. GO) or higher level events (e.g. MENU-DROP).

The high level functions can actually be assigned to a key (ON F1 GO.), but higher level events are simply generated by widgets. In both cases there are no key codes involved.

I intuitively added MENU-DROP event to Keyboard.evNames1 array. Because of that Keyboard.eventCode("MENU-DROP") will return concrete value, that depends on index of "MENU-DROP" in evNames1 array in -154.

This all seems fine.

Now I see that it was wrong because MENU-DROP event has no key (key=-1).

Why do you think it was wrong?

So, if I'll remove "MENU-DROP" from evNames1, than ThinClient.invokeTriggers will not find the trigger assigned for this event.

Correct. So don't remove it.

How should I implement LAST-EVENT:CODE for MENU-DROP event? Are there some other similar "fake" codes for some other similar event implemented?

Does the 4GL actually report something useful during the MENU-DROP trigger? What is it?

Ok, I'll try. Constantin proposed PUT SCREEN instead of MESSAGE. Don't you know if it uses window manager?

I don't know, but it is a kind of I/O so I think you should avoid it. As I noted above, can't you just record the state into some procedure-scoped variables and then examine the state later?

Do you mean, that we can stay current implementation as it is implemented this moment? If yes, what about key of MENU-DROP event (see my problem in the beginning of this note)?

Possibly. It depends on whether we are working the same way as the 4GL for the cases that are allowed in the 4GL. We do NOT have to support behavior that is reported by the 4GL as INVALID.

My MENU-DROP trigger is the following:

on menu-drop of sub-menu sbm do:
  message r "menu-drop".
  def var ch as char.
  ch = last-event:label + " " + string(keycode(last-event:label)) + " " + last-event:event-type + " " +
       string(last-event:code) + " " + last-event:function + " " + last-event:type + " " + string(LASTKEY).
  outp[r] = ch.
  r = r + 1.
end.

     MENU-DROP -1 PROGRESS 1079 MENU-DROP PSEUDO-WIDGET -1

The first "MENU-DROP" is a last-event:label value (4GL). P2J outputs previous key, for example "CURSOR-DOWN" because of lastKey is not saved for MENU-DROP event. Let's try to look what is happening. Start from ThinClient.processProgressEvent lines 14487-14496:

      // skip developer events U1-U10
      if (!Keyboard.isDevEvent(action)) 
      {
         String evName = Keyboard.eventName(action);
         this.functionKey = action;
         if (key != KeyInput.CHAR_UNDEFINED)
         {
            this.lastEvent = key;
         }
      }

   public KeyInput(Widget src, int id, int key, boolean mode, boolean isKey) 
   {
      super(src, id);
      this.key = key;

      this.eventMode = mode;
      this.isKey     = isKey;

      actionCode = Keyboard.keyAction(key);

      if (actionCode == CHAR_UNDEFINED)
      {
         actionCode = key;
      }
      else if (actionCode < CHAR_UNDEFINED)
      {
         // a valid key action is defined; keep the original key code if we
         // have a real key, or try to "guess" a hypothetical key code that
         // would correspond the high level event generated by APPLY
         // statement.
         int standardCode = isKey ? key :
                                    Keyboard.keyCodeForAction(actionCode);
         if (standardCode != CHAR_UNDEFINED)
            setKeyCode(standardCode);

         // do not keep key code as action code
         if (key == actionCode && actionCode < 0)
            setKeyCode(CHAR_UNDEFINED);
      }
   }

I went further and artificially set key for MENU-DROP event to -154 after constructor is executed and find out that for that last-event:label became "". Why? Have a look at Keyboard.keyLabel():

      if (code < 0)
         return "";

      ..

As you can see Keyboard.keyLabel() is also don't work for negative code.

Conclusion:
2 problems "key override in KeyInput constructor" and "keyLabel returns empty string" are happen because of key of MENU-DROP event is negative (-154), and it really is negative because MENU-DROP is added to evNames1.

Vadim Gindin wrote:

About "no pause on RETURN" strange, but after I ran your procedure, choose sub-menu "Orwell", and pressed Enter - pause happen. At the same moment in my procedure (with frames) there were no pause for RETURN.. Why it could be?

In your testcase where you were using frame, you didn't have a MESSAGE just before WAIT-FOR. That line was added by me, to check how RETURN behaves when the message area already has some content.

Probably some default state, because the followed RETURN commands are executed without pause. But how to find this default state, that leads to pause?

There are these flags in Window class, on client-side:

   /**
    * Flag to force message clearing on the next MESSAGE stmt.
    */
   private boolean clearMessage = false;

   /**
    * true if automatic message clearing occurred during trigger
    * processing.
    */
   private boolean autoCleared = false;



	
/**
 * Separate pause before hide flag for messages. true means
 * pause is due.
    */
   private boolean messageNeedPause = false;

They are used to force a pause when the MESSAGE area needs to be cleared, implicitly or explicitly, or to automatically clear the message area.  You might need to turn on/off some of these flags, so that in case of RETURN there will be no pause, and in case of other events, there will be a pause, when clearing the message area.  I don't know the exact rules for your MENU-DROP case.

Vadim Gindin wrote:

There are some interesting information in langref in the definition of ON statement:
[...]

What does it mean "interating with the window manager from within the trigger-block"?

As Greg noted already, and from the comment above, we should not duplicate behaviour which is not recommended by Progress... so unless you find an easy way to fix the PAUSE statement (when MESSAGE is used), we can add this as a limitation. So, when using MENU-DROP trigger ensure no I/O is used (this includes MESSAGE statement), to avoid implicit PAUSE, for example.

Here is the sample output string for one trigger run:

MENU-DROP -1 PROGRESS 1079 MENU-DROP PSEUDO-WIDGET -1

I assume this is 4GL output?

If so, then this means that the 4GL is setting a "fake" keycode as 1079 and lastkey is -1. This means you would pass 1079 in as the key to the KeyInput constructor, right? Of course, it would be best to create a Keyboard.SE_MENU_DROP "synthetic event" constant that is set to 1079 and then reference that constant. Wouldn't this solve both issues?

You should probably also model your event after other last-event:type = "PROGRESS" events. See Keyboard.EVENT_TYPES_PG_CODES, Keyboard.isPGEvent().

Greg Shah wrote:

Here is the sample output string for one trigger run:

MENU-DROP -1 PROGRESS 1079 MENU-DROP PSEUDO-WIDGET -1

I assume this is 4GL output?

If so, then this means that the 4GL is setting a "fake" keycode as 1079 and lastkey is -1. This means you would pass 1079 in as the key to the KeyInput constructor, right? Of course, it would be best to create a Keyboard.SE_MENU_DROP "synthetic event" constant that is set to 1079 and then reference that constant. Wouldn't this solve both issues?

I tried that earlier. It breaks trigger search. Trigger search works using the event name from EventDefinition.events. It calculates event name calling Keyboard.eventName() that returns "ESC-7". This key label is really corresponds to the key=1079.

You should probably also model your event after other last-event:type = "PROGRESS" events. See Keyboard.EVENT_TYPES_PG_CODES, Keyboard.isPGEvent().

I've added SE_MENU_DROP there, but this method is utilitarian and is not used in my scenarios.

If so, then this means that the 4GL is setting a "fake" keycode as 1079 and lastkey is -1. This means you would pass 1079 in as the key to the KeyInput constructor, right? Of course, it would be best to create a Keyboard.SE_MENU_DROP "synthetic event" constant that is set to 1079 and then reference that constant. Wouldn't this solve both issues?

I tried that earlier. It breaks trigger search. Trigger search works using the event name from EventDefinition.events. It calculates event name calling Keyboard.eventName() that returns "ESC-7". This key label is really corresponds to the key=1079.

It finds "ESC-7" instead of "MENU-DROP" by the key=1079, and that is why it do not find target trigger.

I will look at the code next.

What is left to do to make ChUI menus fully functional?

PAUSE behavior fix is left. The current bug is the following. In Constantin's procedure message "bogus message so that the message area is not empty" is displayed but become hidden after menu is drawn.

After that I'll need to fix that first element if sub-menu "Orwell" will not gain focus first time sub-menu body is drawn because of the pause of message statement.

Is the pause behavior related to MENU-DROP?

At the first time I met this bug I thought that it is related to MENU-DROP. Now I think it is not related.

Code Review vig_upd20150423a.zip

This is quite good.

1. In Menu.java, what is the requirement for import of MenuChuiImpl and ChuiOutputManager? Neither of these things should be accessed from the general purpose code in Menu.

2. I'd like to hear if Constantin has any ideas on how to resolve the MENU-DROP event issue. The KeyInput/KeyReader hacks are not good.

3. Do we really want to register the popup key listener for every button or just for the buttons that have a popup menu?

Vadim Gindin wrote:

If so, then this means that the 4GL is setting a "fake" keycode as 1079 and lastkey is -1. This means you would pass 1079 in as the key to the KeyInput constructor, right? Of course, it would be best to create a Keyboard.SE_MENU_DROP "synthetic event" constant that is set to 1079 and then reference that constant. Wouldn't this solve both issues?

I tried that earlier. It breaks trigger search. Trigger search works using the event name from EventDefinition.events. It calculates event name calling Keyboard.eventName() that returns "ESC-7". This key label is really corresponds to the key=1079.

It finds "ESC-7" instead of "MENU-DROP" by the key=1079, and that is why it do not find target trigger.

I think MENU-DROP acts like an OS event, the same way i.e. WINDOW-CLOSE acts. The same problem was found for the windows events: the event codes are colliding with existing events from the standard key table (see testcases/uast/keyboards/linux-chui.txt for the standard keys). So a new table was added, Keyboard.osEvents and Keyboard.osEventCodes, which has precedence (when searching) over events in the standard key table.

See how the WINDOW-CLOSE event was added and add something similar for the MENU-DROP event.

Constantin Asofiei wrote:

..
I think MENU-DROP acts like an OS event, the same way i.e. WINDOW-CLOSE acts. The same problem was found for the windows events: the event codes are colliding with existing events from the standard key table (see testcases/uast/keyboards/linux-chui.txt for the standard keys). So a new table was added, Keyboard.osEvents and Keyboard.osEventCodes, which has precedence (when searching) over events in the standard key table.

See how the WINDOW-CLOSE event was added and add something similar for the MENU-DROP event.

I tried this (set SE_MENU_DROP=1079), but for our test procedure it returns:

KEYCODE(LAST-EVENT:LABEL)=1079
LAST-EVENT:CODE=1079
LASTKEY=1079

KEYCODE(LAST-EVENT:LABEL)=-1
LAST-EVENT:CODE=1079
LASTKEY=-1

Vadim Gindin wrote:

Constantin Asofiei wrote:

..
I think MENU-DROP acts like an OS event, the same way i.e. WINDOW-CLOSE acts. The same problem was found for the windows events: the event codes are colliding with existing events from the standard key table (see testcases/uast/keyboards/linux-chui.txt for the standard keys). So a new table was added, Keyboard.osEvents and Keyboard.osEventCodes, which has precedence (when searching) over events in the standard key table.

See how the WINDOW-CLOSE event was added and add something similar for the MENU-DROP event.

I tried this (set SE_MENU_DROP=1079), but for our test procedure it returns:
[...]
And Progress outputs:
[...]

I suspect this happens for WINDOW-CLOSE too. In any case, put a comment in P2J's KEYCODE implementation and will fix it some other time (it might be specific to all our registered Keyboard.osEvents).

Constantin, could you advice my why message (that is just before WAIT-FOR) is cleared when menu is drawn? In analogical procedure with frame it is not cleared. Menu and Frame a both contentPane children. I couldn't find why.

Vadim Gindin wrote:

Constantin, could you advice my why message (that is just before WAIT-FOR) is cleared when menu is drawn? In analogical procedure with frame it is not cleared. Menu and Frame a both contentPane children. I couldn't find why.

Please give an example or point me to the test + scenario you are using. The idea is: the MENU-DROP trigger must have no I/O operations, this includes MESSAGE,DISPLAY,PUT SCREEN, etc or other cases which might result in showing an implicit (i.e. error) message on screen.

My question is not about MENU-DROP now. I use the following procedure:

def var r as int init 1.
def var i as int init 1.

def var outp as char extent 24.

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "qqq".
  sub-menu sbm label "Orwell".
  menu-item print label "print".

on choose of menu-item print do:
  i = 1.
  repeat while i < r:
    put screen row i col 22 string(outp[i], "x(60)").
    i = i + 1.
  end.
end.

on menu-drop of sub-menu sbm do:
  message r "menu-drop".
  def var ch as char.
  ch = last-event:label + " " + string(keycode(last-event:label)) + " " + last-event:event-type + " " +
       string(last-event:code) + " " + last-event:function + " " + last-event:type + " " + string(LASTKEY).
  outp[r] = ch.
  r = r + 1.
end.

message "bogus message so that the message area is not empty".
assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Vadim Gindin wrote:

My question is not about MENU-DROP now. I use the following procedure:
[...]
I found yesterday, that message "bogus message so that the message area is not empty" is drawn, than it is cleared and menubar is drawn. It happens during the start (no user actions needed).

You mean that is cleared in P2J, correct? Because in 4GL the message remains there.

Yes, in P2J. You can use my last update.

Vadim Gindin wrote:

Yes, in P2J. You can use my last update.

1. in WindowConfig.syncWithWidget, we are triggering a repaint for the entire window
2. when drawing, BorderedPanel.drawPanel clears the entire window (as the repaint event is for the entire window)
3. AbstractContainer.draw calls getDrawableWidgets, which returns only the menubar (because this is the top-most found AbstractContainer, so drawing is assumed to start from it...)

• what happens if there is another frame already displayed in the window, when attaching the menubar? I suspect that is cleared, too, by P2J. Please check this.
• why is the menubar on top, in z-order? It might work if we keep this as low as possible... but I can't tell what happens if there are other frames on the screen.

• there are frames on the screen
• you are in the middle of an update operation (attach/detach the menubar via a trigger?)
• you have PUT SCREEN content on the first row - does this get overwritten by the menubar?
• you have a frame with height 21 (24 - 2 messages - 1 status area), and a menubar is added: what happens with the frame, as it no longer fits the screen height?

Constantin Asofiei wrote:

Some other thoughts about my notes in 325: when attaching/detaching the menubar, the "workspace" area were drawing is done loses 1 row, so all existing content must be redrawn... you need to find what happens if the menubar gets attached/detached while:

• there are frames on the screen

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "Exit".
  sub-menu sbm label "Orwell".

def var i as int init 0.

def frame f i with 22 down no-labels no-box.
/*enable all with frame f.*/

repeat while i < 21:
  display i with frame f.
  i = i + 1.
end.

message "bogus1".
message "bogus2".
pause.

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Frame is shifted down.

• you are in the middle of an update operation (attach/detach the menubar via a trigger?)

the same as previous - shifted down

• you have PUT SCREEN content on the first row - does this get overwritten by the menubar?

Menu is drawn over the string that was output by PUT SCREEN without clearing.

• you have a frame with height 21 (24 - 2 messages - 1 status area), and a menubar is added: what happens with the frame, as it no longer fits the screen height?

It is shifted down and the last row become hidden.

--

P2J doesn't clear frames before output of menu-bar.

Vadim Gindin wrote:

Constantin Asofiei wrote:

Some other thoughts about my notes in 325: when attaching/detaching the menubar, the "workspace" area were drawing is done loses 1 row, so all existing content must be redrawn... you need to find what happens if the menubar gets attached/detached while:

• there are frames on the screen

[...]

Frame is shifted down.

I don't think your conclusion is correct. Add a border/side-labels and you will see that the frame's height is adjusted:

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "Exit".
  sub-menu sbm label "Orwell".

def var i as int init 0.

def frame f i with 19 down side-labels.
/*enable all with frame f.*/

repeat while i < 19:
  display i with frame f.
  i = i + 1.
end.

message "bogus1".
message "bogus2".
pause.

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

Exit Orwell
┌─────────────┐
│i: 0          
│i: 1
│i: 2          
│i: 3          
│i: 4          
│i: 5          
│i: 6          
│i: 7          
│i: 8          
│i: 9          
│i: 10         
│i: 11         
│i: 12         
│i: 13         
│i: 14         
│i: 15         
│i: 16         
│i: 17         
└─────────────┘

And if there are are multiple frames on screen:

def sub-menu sbm
  menu-item mii label "iii".

def menu m menubar
  menu-item qqq label "Exit".
  sub-menu sbm label "Orwell".

def var i as int init 0.

def frame f1 i with 4 down side-labels title "f1".
def frame f2 i with 5 down side-labels title "f2".
def frame f3 i with 6 down side-labels title "f3".

view frame f1.
view frame f2.
view frame f3.

message "bogus1".
message "bogus2".
pause.

assign current-window:menubar = menu m:handle.
wait-for choose of menu-item qqq.

┌─────f1──────┐
│i:           │
│             │
│             │
│             │
└─────────────┘
┌─────f2──────┐
│i:           │
│             │
│             │
│             │
│             │
└─────────────┘
┌─────f3──────┐
│i:           │
│             │
│             │
│             │
│             │
│             │
└─────────────┘
bogus1
bogus2
Press space bar to continue.

Exit Orwell
┌─────f1──────┐
│i:           │
│             │
│             │
│             │
└─────────────┘
┌─────f3──────┐
│i:           │
│             │
│             │
│             │
│             │
│             │
└─────────────┘

Greg: in ChUI, I think some relayout is being performed, so that the frame's shown on the visible area now fit the new virtual window height. My suggestion is to finish the menu's drawing/event/etc first and work on this in a second phase, as some refactoring might needed (I think the frames in ChUI can't be parented to the window anymore, we need a "workspace" container as we use in GUI).

My suggestion is to finish the menu's drawing/event/etc first and work on this in a second phase, as some refactoring might needed (I think the frames in ChUI can't be parented to the window anymore, we need a "workspace" container as we use in GUI).

Agreed. And I think that this is a task for someone that has more experience in the layout and frames processing.

Vadim: please create a new task for the UI project which documents this limitation and the known cases/problems. Link to this task as a related task.

Should I include "message hiding" and "pausing bugs" there or fix it?

Vadim Gindin wrote:

Should I include "message hiding" and "pausing bugs" there or fix it?

Yes, include these two issues in the new task, do not work on them now.

The task #2557 is created. Can I run regression testing for current update? If yes, should I add some menu scenarios to MAJIC?

Please address my issues 1 and 3 from the code review in note 316. Post that update here. Hopefully it is the final one for ChUI support. Is there anything else you know of that is needed?

If yes, should I add some menu scenarios to MAJIC?

No I don't want to change MAJIC.

We will add automated testing for menus later.

Code Review vig_upd20150427a.zip

I'm fine with the changes.

but I found that at the moment of initialization config().popupMenuId has no value

I guess that only a dynamic widget case might have this assigned at creation time:

CREATE button hbutton
   ASSIGN popup-menu = hmenu
   ...

So, it seems like we can still check for something that is not -1 and register at init in that case. Please check it.

BUT for all other cases, we already do a pushScreenDefinition() when popup-menu is assigned. Can't we detect when some different menu is assigned and only in that case register the listener?

POPUP-MENU attribute can be set for the following components: Browse, Button, Combo-box, Dialog-box, Editor, Fill-in, Frame, Radio-set, Selection-list, Slider, Toggle-box, Window. I tried it only for Button and I'm going to set it for all remained components. How to do it better: add listener in some base class(like AbstractWidget or in each of listed components separately?

I agree, this needs to be in code that is shared for all relevant widgets. AbstractWidget seems to be a good choice.

Code Review vig_upd20150429a.zip

I'm good with this approach.

It is worth running regression testing on this update. However, please note that Hynek is about to check in a massive update that needs to go first. It will require very careful merging of your update.

You won't be able to check in until after Hynek, but a regression run will tell us if there are any issues to fix.

We can assume that current update has passed regression testing. Only 2 tests are failed: gso_269 and tc_job_002. Should I run testing again after merging with Hynek changes?

Vadim Gindin wrote:

We can assume that current update has passed regression testing. Only 2 tests are failed: gso_269 and tc_job_002. Should I run testing again after merging with Hynek changes?

Yes, merge the update, post it for review, and then runtime testing.

But first please confirm that GSO 269 is a false negative, by running it by hand.

Code Review vig_upd20150430a.zip

1. In WidgetConfig.applyConfig(), shouldn't the assignment be done using ConfigHelper.setModified(this, cfg.modified); as is done in the latest BaseConfig? The WidgetConfig.setModified() would then be removed.

2. AbstractContainer imports need to use wildcards.

Code Review vig_upd20150430b.zip

This is good.

This update (after merge) failed regression testing: gso_tests (19) tc_tests (24). I'm working on it.

There are a lot of similar errors where generated files differ from expected. But there are another errors exist. I've looked at tc_codes_employees_024 test.

   23 CHECK-SCREEN-BUFFER
wait = true; millis = ʼ300000ʼ; failing screen = 
04/30/2015                  EMPLOYEE MASTER (5 of 5)                    17:28:38
┌──────────────────────────────────────────────────────────────────────────────┐
│           Employee: 1029    **************** JR., ** E.                      │
│              Skill:                                                          │
│          Crew Code:                                                          │
│              Shift:   0     Days-Off:                                        │
└──────────────────────────────────────────────────────────────────────────────┘
 Start      Start End        End        Crew Crew       Skil     Shift      Days
 Date       Time  Date       Time  Dept Code Eff Date   Code Sft Eff Date   Off
 ────────── ───── ────────── ───── ──── ──── ────────── ──── ─── ────────── ────
>07/02/1990 07:00 07/02/1990 15:30 7000 D40  01/01/1990 7300  11 01/01/1990 67
 11/01/1995 07:00 11/01/1995 15:30 7000 40D  01/01/1990 7300  11 01/01/1990 67
 09/25/1999 07:00 09/25/1999 15:30 1500 070  01/01/1990 1540  11 01/01/1990 67
 05/18/2002 07:00 05/18/2002 15:30 1100 050  01/01/1990 1130  11 01/01/1990 67
 10/18/2003 07:00 10/18/2003 15:30 7000 070  01/01/1990 1540  11 01/01/1990 67
 12/20/2003 07:00 12/20/2003 15:30 7000 070  01/01/1990 7500  11 01/01/1990 67
 06/12/2004 07:00 06/12/2004 15:30 7000 060  01/01/1990 7500  11 01/01/1990 67
 11/01/2004 00:00 11/01/2004 00:00      62E  01/01/1990 1130  11 01/01/1990 67
 11/02/2004 00:00 11/02/2004 00:00      62E  01/01/1990 1130  11 01/01/1990 67
(N)ext (P)rev (U)pdate (A)dd (C)opy (F)ind (D)el (O)utput (T)ext (X)Crew (H)elp
(E)xtend (1)Up (2)Down (3)Pg Back (4) Pg Frwd (5)Beg (6)End (R)eturn: F        

   FAILED 300.103
timeout before the specific screen buffer became available (Mismatched data at line 3, column 22. Expected '1' (0x0031 at relative Y 3, relative X 22) and found ' ' (0x0020 at relative Y 3, relative X 22).)

05/01/2015                  EMPLOYEE MASTER (5 of 5)                    02:17:19
┌──────────────────────────────────────────────────────────────────────────────┐
│           Employee: 1029    *** JR., ********* E.                            │
│              Skill: 1130    IT SOFTWARE DEVEL                                │
│          Crew Code: 62E     *** *** *** *** - *******                        │
│              Shift:  11     Days-Off: 67                                     │
└──────────────────────────────────────────────────────────────────────────────┘
 Start      Start End        End        Crew Crew       Skil     Shift      Days
 Date       Time  Date       Time  Dept Code Eff Date   Code Sft Eff Date   Off
 ────────── ───── ────────── ───── ──── ──── ────────── ──── ─── ────────── ────
>07/02/1990 07:00 07/02/1990 15:30 7000 D40  08/04/2007 7300  11 01/01/1990 67
 11/01/1995 07:00 11/01/1995 15:30 7000 40D  08/04/2007 7300  11 01/01/1990 67
 09/25/1999 07:00 09/25/1999 15:30 1500 070  08/04/2007 1540  11 01/01/1990 67
 05/18/2002 07:00 05/18/2002 15:30 1100 050  08/04/2007 1130  11 01/01/1990 67
 10/18/2003 07:00 10/18/2003 15:30 7000 070  08/04/2007 1540  11 01/01/1990 67
 12/20/2003 07:00 12/20/2003 15:30 7000 070  08/04/2007 7500  11 01/01/1990 67
 06/12/2004 07:00 06/12/2004 15:30 7000 060  08/04/2007 7500  11 01/01/1990 67
 11/01/2004 00:00 11/01/2004 00:00      62E  08/04/2007 1130  11 01/01/1990 67
 11/02/2004 00:00 11/02/2004 00:00      62E  08/04/2007 1130  11 01/01/1990 67
...

LE: GES redacted customer personnel info.

Vadim Gindin wrote:

There are a lot of similar errors where generated files differ from expected. But there are another errors exist. I've looked at tc_codes_employees_024 test.
[...]
I've ran it manually and got the following screen:
[...]
As you can see the header of the table is filled but in my manual run, but that header is empty in testing results (lines "Skill", "Crew Code" are empty and "Shift" contains wrong values). I.e. I couldn't reproduce this error. Could you help me?

This is a known issue, if manually works fine, you can ignore it.

Vadim Gindin wrote:

I've checked gso_17 test, the first failed test and found the following. During the test there are gso_17.txt is generated and the test fails in text files comparison of generated file and prepared earlier (expected). I've attached archive with these files and those diff file. Could you help me interpret it and find how it can be related to my changes?

This one is a real issue, as a text/fill-in is not displayed in the report. It might be related to positioning or visible flag...

Constantin Asofiei wrote:

Vadim Gindin wrote:

I've checked gso_17 test, the first failed test and found the following. During the test there are gso_17.txt is generated and the test fails in text files comparison of generated file and prepared earlier (expected). I've attached archive with these files and those diff file. Could you help me interpret it and find how it can be related to my changes?

This one is a real issue, as a text/fill-in is not displayed in the report. It might be related to positioning or visible flag...

How to debug it effectively?

Vadim Gindin wrote:

Constantin Asofiei wrote:

Vadim Gindin wrote:

I've checked gso_17 test, the first failed test and found the following. During the test there are gso_17.txt is generated and the test fails in text files comparison of generated file and prepared earlier (expected). I've attached archive with these files and those diff file. Could you help me interpret it and find how it can be related to my changes?

This one is a real issue, as a text/fill-in is not displayed in the report. It might be related to positioning or visible flag...

How to debug it effectively?

I think for a first step is better to try to duplicate it. Find which program generates this report (execute it by hand until report output and press F2 to see the program, or search for the frame/report title in the *.p.cache files), the frame which updates this data and create a test similar to how the problematic frame is used.

Vadim, what update are these results from? Because I can recreate it with your 0430a.zip but not with 0430b.zip.

Also, I see that in your folder you have been testing 0430a.zip (/home/vig/testing/updates/p2j/01_vig_upd20150430a.zip), not 0430b.zip, which addresses the issues mentioned by Greg - and inherently fixes GSO 17.

Thus: please merge the update with latest rev and start a runtime testing (and post it here, too).

Code Review vig_upd20150501b.zip

Please just add a history entry to MenuWidget. Otherwise it looks good.

You can check in your code as soon as it passes, unless you have to make some other fix besides the history entry mentioned above.

Constantin Asofiei wrote:

Vadim, what update are these results from? Because I can recreate it with your 0430a.zip but not with 0430b.zip.

Also, I see that in your folder you have been testing 0430a.zip (/home/vig/testing/updates/p2j/01_vig_upd20150430a.zip), not 0430b.zip, which addresses the issues mentioned by Greg - and inherently fixes GSO 17.

Thus: please merge the update with latest rev and start a runtime testing (and post it here, too).

I've tested vig_upd20150501b.zip. Constantin, thank you very much. You're right. Only the following tests are failed: gso_269 and tc_code_employees_21. Because of my mistake we loose some time. Sorry.

So, I've checked gso_269. It also is not reproduced as tc_code_employees_261 manually (as tc_code_employees_264 in the note 347 with the same screen). The screens are the following. Manual run:

05/02/2015               MATERIAL ORDER REQUEST                       02:28:59
┌──────────────────────────────────────────────────────────────────────────────┐
confidential info redacted
└──────────────────────────────────────────────────────────────────────────────┘

Enter Required by date

05/01/2015               MATERIAL ORDER REQUEST                       19:24:48                      
┌──────────────────────────────────────────────────────────────────────────────┐                    
confidential info redacted
└──────────────────────────────────────────────────────────────────────────────┘                    
 INVALID FACILITY CODE - PLEASE RETRY                                                               
Enter ? for Facility Codes                                                                          

   FAILED 180.081                                                                                   
timeout before the specific screen buffer became available (Mismatched data at line 21, column 1. Ex
pected '' (0x0000 at relative Y 21, relative X 1) and found 'I' (0x0049 at relative Y 21, relative X
 1).) 

The mismatched character coordinates are also confusing..

I've also ran single scenario in automated testing mode (specified gso_269 in majic_single.xml and ran run_single.sh). It asked me a password and now It is working about 10 minutes with the message "Running test plan...". How long it could be executed?

Should I ignore this error?

Should I ignore this error?

If I understand correctly, you are saying that your manual testing could not recreate the problem. Right?

If so, then yes, this can be ignored.

I'm trying to draw simple menubar and I have coordinates problem. I expected that [0,0] point will reside under the window title bar, but when I call draw at that point rectangle appears right over window title bar. More, I'm trying to output menu-item title with the color (0, 0, 0) RGB - one of system colors also at that point but it is not printed. Could you advice me why?

Vadim Gindin wrote:

I'm trying to draw simple menubar and I have coordinates problem. I expected that [0,0] point will reside under the window title bar, but when I call draw at that point rectangle appears right over window title bar. More, I'm trying to output menu-item title with the color (0, 0, 0) RGB - one of system colors also at that point but it is not printed. Could you advice me why?

This is for GUI, right? First of all, never use fixed coordinates in GUI. Second, where is the menubar widget attached currently? To the WindowGuiImpl instance or to the WindowWorkspace? In GUI, WindowGuiImpl emulates the OS window (with all its decorations - border, title, etc). If the menubar must be parented to it, ensure that it is placed properly by WindowLayout.

But before doing this, try to determine what the 4GL reports as coordinates for the menubar; also, what happens if the window becomes scrollable - does the menubar stay fixed or it gets scrolled, too? If it gets scrolled, then the menubar should belong to the WindowWorkspace.

Constantin Asofiei wrote:

Vadim Gindin wrote:

I'm trying to draw simple menubar and I have coordinates problem. I expected that [0,0] point will reside under the window title bar, but when I call draw at that point rectangle appears right over window title bar. More, I'm trying to output menu-item title with the color (0, 0, 0) RGB - one of system colors also at that point but it is not printed. Could you advice me why?

This is for GUI, right? First of all, never use fixed coordinates in GUI. Second, where is the menubar widget attached currently? To the WindowGuiImpl instance or to the WindowWorkspace? In GUI, WindowGuiImpl emulates the OS window (with all its decorations - border, title, etc). If the menubar must be parented to it, ensure that it is placed properly by WindowLayout.

But before doing this, try to determine what the 4GL reports as coordinates for the menubar; also, what happens if the window becomes scrollable - does the menubar stay fixed or it gets scrolled, too? If it gets scrolled, then the menubar should belong to the WindowWorkspace.

MENU widget does not support coordinate attributes and WIDTH/HEIGHT attributes too..

Vadim Gindin wrote:

MENU widget does not support coordinate attributes and WIDTH/HEIGHT attributes too..

OK, I did some checks and the menubar is not scrolled. So it must be placed by WindowLayout and parented to the WindowGuiImpl instance.

More, if the menubar doesn't fit the window width, menus can be placed on a second, third etc line - does this happen for ChUI, too?

Vadim Gindin wrote:

It could print the title on the top left corner when it draws rectangle but it doesn't happen.

Once again: 2 difficulties: coordinates and text.

Could you look at my changes?

I think they are both related to the same problem: you are not doing the layout for the GUI menubar impl, which determines the location for the menu items/sub-menus (what MenuChuiImpl.doLayout does for ChUI). You might need something similar, but this time using string widths in pixels and not number of chars in string.

As always, please use the -Dwidgetbrowser=yes option and check the runtime values for i.e. location/size - this can save you a lot of headaches, as it tells you where the widget should be placed and if its coordinates are computed properly.

Constantin Asofiei wrote:

Vadim Gindin wrote:

It could print the title on the top left corner when it draws rectangle but it doesn't happen.

Once again: 2 difficulties: coordinates and text.

Could you look at my changes?

I think they are both related to the same problem: you are not doing the layout for the GUI menubar impl, which determines the location for the menu items/sub-menus (what MenuChuiImpl.doLayout does for ChUI). You might need something similar, but this time using string widths in pixels and not number of chars in string.

Oh, Here’s where the dog is buried. Now it is drawn correctly, but menu item titles are still not drawn..

As always, please use the -Dwidgetbrowser=yes option and check the runtime values for i.e. location/size - this can save you a lot of headaches, as it tells you where the widget should be placed and if its coordinates are computed properly.

Menubar is not always displayed in the widget browser because of some reason..

Vadim Gindin wrote:

Constantin Asofiei wrote:

Vadim Gindin wrote:

It could print the title on the top left corner when it draws rectangle but it doesn't happen.

Once again: 2 difficulties: coordinates and text.

Could you look at my changes?

I think they are both related to the same problem: you are not doing the layout for the GUI menubar impl, which determines the location for the menu items/sub-menus (what MenuChuiImpl.doLayout does for ChUI). You might need something similar, but this time using string widths in pixels and not number of chars in string.

Oh, Here’s where the dog is buried. Now it is drawn correctly, but menu item titles are still not drawn..

Titles is also coordinates problem. I'll fix it. Thank you for help!

I faced with the problem of defining a widget, mouse is clicked over. I have simple menubar with 3 menu-items and one submenu called "Submenu". When I click on submenu the following is happen. WindowGuiImpl.findMouseSource() (line 579) gets the point clicked and tries to find the widget containing this point:

         java.awt.Point p = evt.getEvent().getPoint();

         // search the other widgets
         widget = contentPane.findMouseSource(new NativePoint(p.x, p.y));

contentPane calls AbstractContainer.findMouseSource(p) method. Have a look at it. It takes all container widgets and for each of them calls w.findMouseSource(p.minus(w.physicalLocation())). In my case it works in the following way.
p=[160, 44] - is the clicked point.
Window.findMenuSource finds menubar containing it first and calls w.findMouseSource(p.minus(w.physicalLocation())) for it.
menubar location = [4, 29], so findMouseSource takes: p=[156, 15].
Submenu location = [130, 29] - the absolute point relative to window left-top corner, therefore it has the same y-axis coordinate as menubar itself.

As you can see p=[156, 15] is not resides in submenu and menu.findMouseSource returns null.

Why? I can only propose that w.physicalLocation() always returns absolute location of widget (relative to left-top corner of window - not relative to parent widget). Am I right? If yes why p.minus(w.physicalLocation()) is needed, i.e. why not to pass p itself?

Vadim Gindin wrote:

I faced with the problem of defining a widget, mouse is clicked over. I have simple menubar with 3 menu-items and one submenu called "Submenu". When I click on submenu the following is happen. WindowGuiImpl.findMouseSource() (line 579) gets the point clicked and tries to find the widget containing this point:
[...]

contentPane calls AbstractContainer.findMouseSource(p) method. Have a look at it. It takes all container widgets and for each of them calls w.findMouseSource(p.minus(w.physicalLocation())). In my case it works in the following way.
p=[160, 44] - is the clicked point.
Window.findMenuSource finds menubar containing it first and calls w.findMouseSource(p.minus(w.physicalLocation())) for it.
menubar location = [4, 29], so findMouseSource takes: p=[156, 15].
Submenu location = [130, 29] - the absolute point relative to window left-top corner, therefore it has the same y-axis coordinate as menubar itself.

As you can see p=[156, 15] is not resides in submenu and menu.findMouseSource returns null.

Why? I can only propose that w.physicalLocation() always returns absolute location of widget (relative to left-top corner of window - not relative to parent widget). Am I right? If yes why p.minus(w.physicalLocation()) is needed, i.e. why not to pass p itself?

How have you implemented the menu's location? When a container (parent-child relation) is used, the child widget has its coordinates always relative to the parent. And Widget.physicalLocation will return always these coordinates. For absolute location, we have screenPhysicalLocation - but this can't be used for your case.

So: you either adjust the menu's location to be relative to its parent or override the findMouseSource in your MenuBar implementation, so that it searches the the menus using absolute coordinates. But I would prefer to adjust the menu's location to be relative...

adjust the menu's location to be relative to its parent

This one.

It is important that the physicalLocation() contract is honored.

If so, what about methods like GuiDriver.drawString(String, int, int), GuiDriver.drawLine(int, int, int, int) or others. Are they accept absolute coordinates in those parameters?

If yes, will be the following code correct in a menu-item draw method:

   NativePoint parentLoc = SubMenuGuiImpl.this.parent().screenPhysicalLocation(); // absolute parent location

   // absolute point to draw in, calculated as absolute parent location shifted by some constant values
   NativePoint tp = new NativePoint(parentLoc.x + 7, parentLoc.y + 5 + fh); 

   // draw menu-item title
   gd.drawString(title, tp.x, tp.y);

Vadim Gindin wrote:

If so, what about methods like GuiDriver.drawString(String, int, int), GuiDriver.drawLine(int, int, int, int) or others. Are they accept absolute coordinates in those parameters?

That's why we always include these in a GuiDriver.draw(origin, clip, runnable) call. So, assuming you have a widget tree like (simplified): window > frame > widget, when draw is called in GUI, the graphics physical origin is translated as this, from an initial (0, 0) point:
- for window: (wx, wy) (this is actually the workspace's location)
- for frame: (wx + fx, wy + fy) where (fx, fy) is frame's location relative to the window workspace
- for widget: (wx + fx + gx, wy + fy + gy) where (gx, gy) is the widget's location relative to the frame

When the gd.draw stack unwinds, the graphics origin is restored to what it was before the gd.draw was called. Using this approach, you can assume that when the widget is being drawn, all APIs are working with a graphics origin of (0,0), which at this time is positioned on the the widget's top-left corner - so you don't need the widget's absolute coordinates to perform drawing.

If yes, will be the following code correct in a menu-item draw method:

No, because it doesn't follow the rules above. At the time when MenuItem.draw is called, the graphics origin must already be translated to the menu-items's parent, so the menu-item can execute a gd.draw using its location relative to the parent.

It will help if you post how the drawing stacktrace and the widget hierarchy (from the widget browser) looks with your current code.

Vadim Gindin wrote:

I've made the following changes:
#. I've set menu children physicalLocation to relative value
#. I used relative point to draw menu-item in gd.draw(origin, clip, core)

But menu-item was drawn over window title again.

Here is my stack:
[...]

I'm also attaching widget browser screen shot. I can recall that menu is a added to a window directly - not to a workspace. I set correct menubar location in a WindowLayout - the same as workspace location.

Have a look please.

I think your problem is this:

MenuGuiImpl(AbstractContainer<O>).draw() line: 312    
MenuGuiImpl.draw() line: 87   

For your current code: the graphics origin is never translated, that's why you end up needing absolute coordinates.

You are right and now it works. Thank you very much!

I wanted to ask some common question about repainting in GUI.

For ChUI I need to post new PaintEvent to event queue to actuate repainting of rectangle specified in this PaintEvent. More, rectangle is specified using Point class. At some moment all posted PaintEvent's are popped from the queue and those rectangles are unionized to one. This common rectangle is used when actual repainting is happen.

#. Does this scenario work for GUI in the same way? I recall that for GUI 2 classes are used to set a point: NativePoint and Point when for ChUI only Point is used.
#. Assume we have:

   NativePoint np1 = new NativePoint(someX, someY);
   Point p = wnd.screen().coordinates().widthFromNative(np1);
   NativePoint np2 = wnd.screen().coordinates().widthToNative(p);

   Point p1 = new Point(someX, someY);
   NativePoint np = wnd.screen().coordinates().widthToNative(p1);
   Point p2 = wnd.screen().coordinates().widthFromNative(np);

Vadim Gindin wrote:

#. Does this scenario work for GUI in the same way? I recall that for GUI 2 classes are used to set a point: NativePoint and Point when for ChUI only Point is used.

Yes, GUI works in the same way. It's the OutputManager's responsibility to convert these into native units (so that it can pass them to ScreenBitmap).

#. Assume we have:
[...]
Is np1 equal np2 for all possible values?
#. Assume we have:
[...]
Is p1 equal p2 for all possible values?

I assume you mean pointFromNative and pointToNative here. As we round the character units to 2 decimals, we can't assume pointToNative and pointFromNative to be each other's inverse function.

What are you trying to achieve with this conversion?

Constantin Asofiei wrote:

Vadim Gindin wrote:

#. Does this scenario work for GUI in the same way? I recall that for GUI 2 classes are used to set a point: NativePoint and Point when for ChUI only Point is used.

Yes, GUI works in the same way. It's the OutputManager's responsibility to convert these into native units (so that it can pass them to ScreenBitmap).

#. Assume we have:
[...]
Is np1 equal np2 for all possible values?
#. Assume we have:
[...]
Is p1 equal p2 for all possible values?

I assume you mean pointFromNative and pointToNative here. As we round the character units to 2 decimals, we can't assume pointToNative and pointFromNative to be each other's inverse function.

What are you trying to achieve with this conversion?

I'm just forced to use this conversion in some methods that uses Point, for example body location calculation. It has to return the point right under the sub-menu title (?) and I need to calculate font height that is calculated as int.

The question about widget (menu-item or sub-menu) state, that could be changed by mouse events: "mouse enter" (mouse pointer is over the widget), "mouse exit" (mouse pointer has left the widget), "mouse pressed" (mouse pointer is over the widget and the left button is pressed) and may be others. I need to draw the widget differently depending on type of concrete mouse event.

AbstractWidget has methods mouse*() that are called when corresponding event is happen and drawing is happen in draw() method.

How to correctly implement different drawing?

I suspect that widget configuration class could have some field or fields: one for each event type or common field for all such event types like WidgetConfig.state, and I should set correspondent field in correspondent mouse*() method and call repaint().

Am I right?

Are such fields exist or I should add them myself for example to WidgetConfig?

I've found that such fields are exist not in config classes, but in GUI widget classes itself. For example ButtonGuiImpl.mouseEntered or ComboBoxGuiImpl.pressed. I should probably do the same because such fields make sense only for GUI implementations. Is it correct?

I've found that such fields are exist not in config classes

They would only be put into config classes if the data has to be transferred between the server and client.

I should probably do the same because such fields make sense only for GUI implementations. Is it correct?

Yes.

I'm trying to implement the following behavior. Assume we have menubar. Mouse pointer is located somewhere outside of menubar. We are starting to drag mouse and moving the pointer to some item in menubar. P2J behavior is the same as P2J behavior, when I'm just pressing the mouse button over this item, i.e. draws its rectangle sunk into the surface of menubar.

I faced with the following problem here. MOUSE_DRAGGED event is targeted to the widget over which mouse button is pressed. So mouseDragged() is not be called for MenuItemGuiImpl and I only can implement MenuItemGuiImpl.mouseMoved() (MOUSE_MOVED event) to process founded P2J behavior. So while implementing MOUSE_MOVED event I need to determine is this "mouse move" is made with pressed button, i.e. it is a MOUSE_DRAGGED event with the source, different than menu-item.

MOUSE_DRAGGED event is implemented using additional field WindowGuiImpl.mousePressedSource, that contains dragging source widget. When mouse button is pressed this field is set to target widget. When mouse button is released this field is reset to null.

I can check this field WindowGuiImpl.mousePressedSource in MenuItemGuiImpl.mouseMoved() to determine is the currect MOUSE_MOVE event is MOUSE_DRAGGED event for some other widget and implement founded P2J drawing behavior based on this information.

Will it be correct?

Vadim Gindin wrote:

I can check this field WindowGuiImpl.mousePressedSource in MenuItemGuiImpl.mouseMoved() to determine is the currect MOUSE_MOVE event is MOUSE_DRAGGED event for some other widget and implement founded P2J drawing behavior based on this information.

Will it be correct?

What you are trying to determine is if the mouse ended up hovering the menu-item, while is being dragged, correct?

I think this is solved better using mouseEntered/mouseExited events; using mouseMove will notify the widget too aggressively. Please think of a way to change MouseHandler.applyMouseEvent so that entered/exited events are raised if mouse is dragged. You might need to track the mousePressedSource here, too.

Vadim Gindin wrote:

Here is my current update. Could you review and help me? I can't understand why nested sub-menu body isn't drawn when I click on "Insider". Here is my testcase:
[...]

There are some difference in rectangles merging. Probably there is a reason. Where to look at.

Please use the widget browser and verify the "insider" (and other) sub-menu's coordinates - although looks like is drawn in the right place, its coordinates are incorrect. Fixing these should fix the mouse click problems.

Please, hepl my with my previous question about duplication. I still didn't find the reason.

Additional question. How the 3D effect is implemented in P2J? I used GuiDriver.draw3DRect() to draw sub-menu body but it looks more flat than original sub-menu body in windev01. Shouldn't I use this method to draw sub-menu body?

Vadim Gindin wrote:

Please, help me with my previous question about duplication. I still didn't find the reason.

Additional question. How the 3D effect is implemented in P2J? I used GuiDriver.draw3DRect() to draw sub-menu body but it looks more flat than original sub-menu body in windev01. Shouldn't I use this method to draw sub-menu body?

Duplication error has gone after the last Hynek's update! Sorry to trouble.

Vadim Gindin wrote:

Duplication error has gone after the last Hynek's update! Sorry to trouble.

Please post your merged code.

Vadim Gindin wrote:

Please, hepl my with my previous question about duplication. I still didn't find the reason.

Additional question. How the 3D effect is implemented in P2J? I used GuiDriver.draw3DRect() to draw sub-menu body but it looks more flat than original sub-menu body in windev01. Shouldn't I use this method to draw sub-menu body?

Please try GuiDriver.fill3DRect instead of draw3DRect.

• you are calling a repaint() for a certain menu-item; this ensures that an invalidation (clipping) rectangle is added to the ScreenBitmap, only for that widget's bounds
• later on, MenuGuiImpl.draw is called; this is calling a fillRect for the entire menubar (so all menus are no longer on screen at this point)
• when calling MenuGuiImpl.super.draw();, although this collects all menu widgets, AbstractContainer.draw:290 (!clippings.isEmpty()) will be true only for the menu which called repaint (and ScreenBitmap's invalidation rectangle contains the widget's boundary) - thus menus which are not touched by the invalidation rectangle will not be repainted.

The same is happening for your sub-menu case.

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

Constantin, thank you, I'll try.

Greg, I've faced with parsing error with the following procedure:

define sub-menu sm
  menu-item mt1 label "dddd".

define menu men1 menubar
   sub-menu  sm   label "Submenu" disabled.

It will work if we'll change the order of disabled and label "Submenu": sub-menu sm disabled label "Submenu"., but in the source case it will throw an error during conversion:

menu/gui/parse_disabled.p
line 5:35: expecting DOT, found 'disabled'
    at antlr.Parser.match(Parser.java:213)
    at com.goldencode.p2j.uast.ProgressParser.define_stmt(ProgressParser.java:8797)
    at com.goldencode.p2j.uast.ProgressParser.stmt_list(ProgressParser.java:22098)
    at com.goldencode.p2j.uast.ProgressParser.statement(ProgressParser.java:5712)
    at com.goldencode.p2j.uast.ProgressParser.single_block(ProgressParser.java:4568)
    at com.goldencode.p2j.uast.ProgressParser.block(ProgressParser.java:4327)
    at com.goldencode.p2j.uast.ProgressParser.external_proc(ProgressParser.java:4253)
    at com.goldencode.p2j.uast.AstGenerator.parse(AstGenerator.java:1441)
        ..

I've looked at submenu_descriptor in progress.g and found that it contains the error. Here is it. Now:

submenu_descriptor   
   :
      KW_SUB_MENU^ lvalue (KW_DISABLED)? (label)?
   ;

submenu_descriptor
   :
      KW_SUB_MENU^ lvalue (label | KW_DISABLED)?
   ;

But when I did this change, the error didn't disappear. Was I wrong? Could you have a look at it?

Constantin Asofiei wrote:

What I think is happening is this:

• you are calling a repaint() for a certain menu-item; this ensures that an invalidation (clipping) rectangle is added to the ScreenBitmap, only for that widget's bounds
• later on, MenuGuiImpl.draw is called; this is calling a fillRect for the entire menubar (so all menus are no longer on screen at this point)
• when calling MenuGuiImpl.super.draw();, although this collects all menu widgets, AbstractContainer.draw:290 (!clippings.isEmpty()) will be true only for the menu which called repaint (and ScreenBitmap's invalidation rectangle contains the widget's boundary) - thus menus which are not touched by the invalidation rectangle will not be repainted.

The same is happening for your sub-menu case.

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

How to distinguish the state when menubar is displayed from the state when it is being repainted?

Vadim Gindin wrote:

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

How to distinguish the state when menubar is displayed from the state when it is being repainted?

Add a flag at the widget impl class to track if this is the first draw (after the widget was made visible) or not. This should be set to true after draw is called and set to false when the widget is hidden (visible is set to false).

But when I did this change, the error didn't disappear. Was I wrong? Could you have a look at it?

Your change does not work because only one of the two options can ever be matched. If they are both present, it will fail.

Please make this change to progress.g:

submenu_descriptor
   :
      KW_SUB_MENU^ lvalue (KW_DISABLED | label)*
   ;

This allows the parser to match any number of these options, in any order so long as there is no other unexpected option in between.

Constantin Asofiei wrote:

Vadim Gindin wrote:

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

How to distinguish the state when menubar is displayed from the state when it is being repainted?

Add a flag at the widget impl class to track if this is the first draw (after the widget was made visible) or not. This should be set to true after draw is called and set to false when the widget is hidden (visible is set to false).

I'm trying. During the start Menu.draw() is called several times in ThinClient.waitForWorker(). The first several calls draws correct menubar and the last 2 calls are refreshing the window and do not draw menubar rectangle. It looks like at some step the window fully resets its content rigth as it does during the first draw. Is it possible? What could be an other reason?

Vadim Gindin wrote:

Constantin Asofiei wrote:

Vadim Gindin wrote:

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

How to distinguish the state when menubar is displayed from the state when it is being repainted?

Add a flag at the widget impl class to track if this is the first draw (after the widget was made visible) or not. This should be set to true after draw is called and set to false when the widget is hidden (visible is set to false).

I'm trying. During the start Menu.draw() is called several times in ThinClient.waitForWorker(). The first several calls draws correct menubar and the last 2 calls are refreshing the window and do not draw menubar rectangle. It looks like at some step the window fully resets its content rigth as it does during the first draw. Is it possible? What could be an other reason?

So this can't work as there are legitimate cases where the entire window is redrawn.

Lets change the approach to this: take the menubar's clipping rectangle and, if the ScreenBitmap has an invalidation rectangle set (getOuterRectangle is not null), then intersect it (with the menubar's clipping rectangle) and the intersection result will be the menubar's real clipping rectangle. This ensures that only the portion of the menubar being repainted will be affected. Same for the sub-menu case.

Constantin Asofiei wrote:

Vadim Gindin wrote:

Constantin Asofiei wrote:

Vadim Gindin wrote:

A solution would be to repaint the entire menubar or sub-menu on mouseEnter/Exit. But I think the proper way to go is to refactor a litle the menu-bar (and sub-menu drawing): let each individual widget clear itself; if needed, the parent draws the rectangle only once (when is displayed).

How to distinguish the state when menubar is displayed from the state when it is being repainted?

Add a flag at the widget impl class to track if this is the first draw (after the widget was made visible) or not. This should be set to true after draw is called and set to false when the widget is hidden (visible is set to false).

I'm trying. During the start Menu.draw() is called several times in ThinClient.waitForWorker(). The first several calls draws correct menubar and the last 2 calls are refreshing the window and do not draw menubar rectangle. It looks like at some step the window fully resets its content rigth as it does during the first draw. Is it possible? What could be an other reason?

So this can't work as there are legitimate cases where the entire window is redrawn.

Lets change the approach to this: take the menubar's clipping rectangle and, if the ScreenBitmap has an invalidation rectangle set (getOuterRectangle is not null), then intersect it (with the menubar's clipping rectangle) and the intersection result will be the menubar's real clipping rectangle. This ensures that only the portion of the menubar being repainted will be affected. Same for the sub-menu case.

Do you propose to use this intersection rectangle as clipping rectangle in MenuGuiImpl.draw() and SubMenuGuiImpl.draw()?

Vadim Gindin wrote:

Lets change the approach to this: take the menubar's clipping rectangle and, if the ScreenBitmap has an invalidation rectangle set (getOuterRectangle is not null), then intersect it (with the menubar's clipping rectangle) and the intersection result will be the menubar's real clipping rectangle. This ensures that only the portion of the menubar being repainted will be affected. Same for the sub-menu case.

Do you propose to use this intersection rectangle as clipping rectangle in MenuGuiImpl.draw() and SubMenuGuiImpl.draw()?

Yes.

Vadim Gindin wrote:

Constantin, could you look at my update?

Is that what you meant?

When I'm moving mouse pointer to menu-item in MENUBAR there rectangle should be drawn around menu-item title. It happen in the method MenuItemGuiImpl.drawTitleRectangle(GuiColorResolver, NativeDimension). After I tried the current change with new clipping this rectangle stopped drawing. I can't find why.

Please, have a look it with the simple testcase:

[...]