FWD » Core Development » User Interface

Constantin:

Currently P2J doesn't collect strings from MENU widgets... support is only in frame_generator.xml, menu_generator.xml is missing support. We should collect these, too; if the label has accelerators, we need to prepare the string, so is recorded as is shown on screen, and not with the encoded accelerator (actually, I think this is for all labels, not just menu labels).

I've made the change, but should I create a separate branch for this task?

Yes.

Task branch 2952a is created (11049 trunk rev).

I see in frame_generator.xml the code that just removes the first '&' from the label before it will be placed to uiStrings file. Such preparation is wrong. The label can contain several symbols '&' and the algorithm is more complex. I'd implemented it in menus and used it for Button too.. Should I implement it here for uiStrings preparation?

I see in frame_generator.xml the code that just removes the first '&' from the label before it will be placed to uiStrings file. Such preparation is wrong. The label can contain several symbols '&' and the algorithm is more complex. I'd implemented it in menus and used it for Button too.. Should I implement it here for uiStrings preparation?

Yes.

Is there a way to know what the current environment is (CHUI, GUI) during conversion?

ui_strings.txt is only used in GUI (to obtain the variable length of each string based on a list of fonts). ChUI interfaces don't use variable width fonts, everything is monospaced and we draw based on whole characters of the same size.

Why do you ask?

Oh, really. You're right. Thanks. Another question. We have com.goldencode.p2j.ui.client.gui.MnemonicInfoGuiImpl class that contains preparation logic. Will it be correct to call this class (from ui.client.gui package) from conversion process?

Will it be correct to call this class (from ui.client.gui package) from conversion process?

We normally don't like to have such references, but it is OK in this case.

Could you recall, how to make a static call that will prepare the label and return a string? In other words there is nothing to emit, just process a string and return another string, so I've added static method to MnemonicInfoGuiImpl and wanted to call it from menu_generator.xml, but I can't find corresponding way..

It is not possible to make a static call from a TRPL ruleset. Today, we either:

1. Hide the call in a worker (e.g. something that implements the PatternWorker interface).

2. Create an instance in TRPL (using create("com.goldencode.p2j.ui.client.gui.MnemonicInfoGuiImpl")) and then call the static method on the instance. At least I think this will work if the static method name is unambiguous. You can try this to see.

Please review. Current branch revision is 11054 (rebased with current trunk). Should I change also frame_generator.xml in a same way?

1. menu_generator.xml - please move this code to the init-rules block (otherwise mnemInfo will be instantiated on each label...)
   

                  <action>
                     mnemInfo = #(com.goldencode.p2j.ui.client.gui.MnemonicInfoGuiImpl) 
                                 create('com.goldencode.p2j.ui.client.gui.MnemonicInfoGuiImpl')
                  </action>
   

2. does fprintf work in append mode? If yes, then does frame_generator.xml get called before or after the menu_generator.xml? Because frame_generator.xml deletes this file, when it is ran.
3. MnemonicInfo, MnemonicInfoGuiImpl - missing history entry

Should I change also frame_generator.xml in a same way?

Does the side-labels in frame follow the same mnemonic rules as the menu texts? If so, then yes, it should be changed. Maybe make a common function which is called by both files.

Vadim Gindin wrote:

1. Side-labels follow the same label preparation rules. In other words prepared labels are the same (labels are prepared to remove extra symbols '&'). There are some peculiarities related to defining mnemonic character, but it is not relates to this task. See labels comparison:

OK, please proceed and fix the frame_generator.xml in the same way. But also note that a text may exist in other areas (as in a combo box item), so leave the unprocessed text behind, too.

1. About frpintf. I've converted the test where the frame and menu exist simultaneously and it works. ui_strings.txt file contains frame labels first and menu labels after them, i.e. menu labels are processed after frame labels.

OK.

Where should I place noted common function? May be to rules/include/common-progress.rules?

Vadim Gindin wrote:

Where should I place noted common function? May be to rules/include/common-progress.rules?

Place it in convert/common-convert.xml.

This one rules/convert/common-convert.rules?

Vadim Gindin wrote:

This one rules/convert/common-convert.rules?

Yes.

I've added <include name="common-convert" /> in both files menu_generator.xml and frame_generator.xml. For menus it works but for frames it can't find the library:

Caused by: com.goldencode.p2j.cfg.ConfigurationException: Error loading configuration: convert/frame_generator; XML path: /cfg/include
    at com.goldencode.p2j.pattern.ConfigLoader.loadImpl(ConfigLoader.java:514)
    at com.goldencode.p2j.pattern.ConfigLoader.load(ConfigLoader.java:439)
    at com.goldencode.p2j.pattern.PatternEngine.initialize(PatternEngine.java:1093)
    at com.goldencode.p2j.pattern.PatternEngine.run(PatternEngine.java:955)
    ... 4 more
Caused by: com.goldencode.p2j.cfg.ConfigurationException: Error loading configuration: common-convert; XML path: ++ UNAVAILABLE ++
    at com.goldencode.p2j.pattern.ConfigLoader.loadImpl(ConfigLoader.java:514)
    at com.goldencode.p2j.pattern.ConfigLoader.include(ConfigLoader.java:934)
    at com.goldencode.p2j.pattern.ConfigLoader.processChildElements(ConfigLoader.java:601)
    at com.goldencode.p2j.pattern.ConfigLoader.loadImpl(ConfigLoader.java:484)
    ... 7 more
Caused by: com.goldencode.p2j.cfg.ConfigurationException: Error finding configuration file
    at com.goldencode.p2j.pattern.ConfigLoader.findConfigurationFile(ConfigLoader.java:392)
    at com.goldencode.p2j.pattern.ConfigLoader.loadImpl(ConfigLoader.java:473)
    ... 10 more
Caused by: java.io.FileNotFoundException: File 'common-convert.rules' not found in path: .:/home/vig/projects/testcases/uast/pattern:/home/vig/projects/testcases/uast/p2j/rules/include:/home/vig/projects/testcases/uast/p2j/rules:
    at com.goldencode.p2j.util.Utils.findFileInPath(Utils.java:881)
    at com.goldencode.p2j.pattern.ConfigLoader.findConfigurationFile(ConfigLoader.java:388)
    ... 11 more

Use the folder name, too:

   <include name="convert/common-convert" />

This also doesn't work

Vadim Gindin wrote:

This also doesn't work

Are you sure you are using the right rules? I've just tested it and it worked.

Constantin Asofiei wrote:

Vadim Gindin wrote:

This also doesn't work

Are you sure you are using the right rules? I've just tested it and it worked.

First I've added <include filename="common-convert" /> to menu_generator.xml and it worked. After that I've added <include filename="convert/common-convert" /> to frame_generator.xml and it stopped working for menu_generator.. But the convert/common-convert variant now works for both. Thanks

I've fixed all remarks. Please review. Current branch 2952a revision is 11056.

The work is done. Please review and approve regression testing. I've rebased the branch. Current revision is 11057.

Code Review Task Branch 2952a Revision 11057

Constantin will be doing a code review, since he wrote the ui_strings logic originally.

In my review, the only concern I have is that the MnemonicInfo.init() is no longer a descriptive method name. When it was exclusively used for initialization of MnemonicInfo instances, the name init() was appropriate. Now it is also used for conversion processing as a helper method. Perhaps prepare() would be a better name?

Vadim, the 2952a rev 11057 is OK; the only change in 2952a is what Greg mentioned: rename MnemonicInfo.init() to MnemonicInfo.prepare().

I've renamed init() to prepare(). Should I run also runtime-regression or only conversion-regression is sufficient?

Please check in your changes.

Should I run also runtime-regression or only conversion-regression is sufficient?

Conversion regression should be executed for MAJIC. The MAJIC runtime doesn't use menus, but it does use buttons which are dependent on your changes. To be safe, please execute runtime testing too.

I'd also like you to do manual testing of the GUI menu testcases.

Conversion and regression testing are PASSED. I'm checking GUI menu testcases.

I've checked GUI menu cases simple_sm.p and popup_ext.p. They are broken. The error happen also in trunk. The exception is the following:

        at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:265)
Caused by: java.lang.RuntimeException: No renderer is registered for id = -19
        at com.goldencode.p2j.ui.client.gui.driver.GuiPrimitivesImpl.getWindowEmulator(GuiPrimitivesImpl.java:208)
        at com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver.moveToTop(SwingGuiDriver.java:503)
        at com.goldencode.p2j.ui.client.WindowManager.moveToTop(WindowManager.java:324)
        at com.goldencode.p2j.ui.client.TopLevelWindow.processEvent(TopLevelWindow.java:760)
        at com.goldencode.p2j.ui.client.gui.OverlayWindow.processEvent(OverlayWindow.java:613)
        at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:15899)
        at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:15374)
        at com.goldencode.p2j.ui.chui.ThinClient.pop(ThinClient.java:14385)
        at com.goldencode.p2j.ui.chui.ThinClient.eventBracket(ThinClient.java:14368)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:14286)
        at com.goldencode.p2j.ui.chui.ThinClient.applyWorker(ThinClient.java:14042)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForEvent(ThinClient.java:13369)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11255)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:10834)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10782)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:10736)
        ... 21 more

java.lang.RuntimeException: No renderer is registered for id = -19
        at com.goldencode.p2j.ui.client.gui.driver.GuiPrimitivesImpl.getWindowEmulator(GuiPrimitivesImpl.java:208)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.enableEvents(AbstractGuiDriver.java:2368)
        at com.goldencode.p2j.ui.client.WindowManager.disableOSEvents(WindowManager.java:1119)
        at com.goldencode.p2j.ui.client.gui.driver.GuiOutputManager.disableOSEvents(GuiOutputManager.java:249)
        at com.goldencode.p2j.ui.chui.ThinClient.getChanges(ThinClient.java:12531)
        at com.goldencode.p2j.net.Protocol.attachChanges(Protocol.java:275)
        at com.goldencode.p2j.net.Queue.enqueueOutbound(Queue.java:823)

It seems it is not related to my changes. Can I commit the changes?

Vadim Gindin wrote:

It seems it is not related to my changes.

Is this related to what you found in #2966?

Constantin Asofiei wrote:

Vadim Gindin wrote:

It seems it is not related to my changes.

Is this related to what you found in #2966?

Yes, it is the same bug.

OK, please merge to trunk.

It seems it is not related to my changes.

Is this related to what you found in #2966?

Yes, it is the same bug.

I thought the problem added in trunk 11053 and found in #2966 was a drawing issue (note 9). Your note 33 above describes a java.lang.RuntimeException: No renderer is registered for id = -19. Are these the same issue?

Greg Shah wrote:

It seems it is not related to my changes.

Is this related to what you found in #2966?

Yes, it is the same bug.

I thought the problem added in trunk 11053 and found in #2966 was a drawing issue (note 9). Your note 33 above describes a java.lang.RuntimeException: No renderer is registered for id = -19. Are these the same issue?

I'm not sure about the reason but I think this the same bug. We can also see some blinking during sub-menu navigation in both cases.

I'm not sure about the reason but I think this the same bug. We can also see some blinking during sub-menu navigation in both cases.

OK, make sure it is fixed in #2966. Go ahead with your merge to trunk.

Committed to trunk 11056.

The branch 2952a is archived.