ImpulseAdventure / GUIslice-Builder

Cross-platform drag & drop GUI builder for GUIslice
Other
168 stars 35 forks source link

Code generation deletes user code in BTN callbacks when SWITCH is used inside a callback handler #179

Closed kcobley closed 2 years ago

kcobley commented 2 years ago

Describe the bug problem occurs when code is generated from Builder after user edits made to button callbacks. If callback edits include a switch statement then the autogenerated code will delete part of the switch statement inside the handler.

To Reproduce Auto generate with button callbacks Edit user code in call back which include a SWITCH statement Re-generate code with Builder Observe deleted user code with hanging SWITCH statement.

Expected behavior I expected user code not to be deleted (Normally Builder does this exceptionally well; so well, I find it impressive :-) )

Example code from my project:

Code before generate:

  //<Button Enums !Start!>
  case E_ELEM_BTN1:
    gslc_SetPageCur(&m_gui, E_PG2);
    break;
  case E_ELEM_BTN5:
    // clicked on the graph so cycle to the next graph data changing the axis labels and axes to suit and pointing to the correct datasets
    switch (plotDataType)//  enums: {extTempHum, extIAQHum, extEffHum, srcTempHum, supTempHum, extHumPres};
    {
      case extTempHum:
        {
          plotDataType = extIAQHum;
          //left axis -> IAQ 0-500

          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis0 , "000" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis1 , "100" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis2 , "200" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis3 , "300" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis4 , "400" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis5 , "500" );
          //left label "IAQ   "
          gslc_ElemSetTxtStr  ( &m_gui, LeftLabel , "IAQ   " );
          //set data pointers to IAQ and extHum
          leftData=&IAQData[1];
          break;
        }

snip out extra cases to save verbosity

      case supTempHum:
        {
          plotDataType = extHumPres;
          //left label -> "Prs mb"
          gslc_ElemSetTxtStr  ( &m_gui, LeftLabel , "prs mB" );
          //left axis 910,940,970,000,030,060
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis0 , "910" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis1 , "940" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis2 , "970" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis3 , "000" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis4 , "030" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis5 , "060" );
          //right label -> "Hum % "
          gslc_ElemSetTxtStr  ( &m_gui, RightLabel , "Hum % " );
          //set pointers to pressure and extract hum.
          leftData=&PextData[1];
          rightData=&HextData[1];
          break;
        }
      case extHumPres:
        {
          plotDataType = extTempHum;
          //left label -> "Tmp ^C"
          gslc_ElemSetTxtStr  ( &m_gui, LeftLabel , "Tmp ^C" );
          //left axis -10 - 40
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis0 , "-10" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis1 , " 00" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis2 , " 10" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis3 , " 20" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis4 , " 30" );
          gslc_ElemSetTxtStr  ( &m_gui, LeftAxis5 , " 40" );
          //set pointers to extract temp (right already set :-) )
          leftData=&TextData[1];
          break;
        }
      default:
        {

        }
        //shouldnt get here...
    }
    graphDrawMode=redrawAll;// set the graph to redraw the plot window
    Serial.print("Plot Type: ");Serial.println(plotDataType);
    break;
  case E_ELEM_NUMINPUT3:
    // Clicked on edit field, so show popup box and associate with this text field
    gslc_ElemXKeyPadInputAsk(&m_gui, m_pElemKeyPadNum, E_POP_KEYPAD_NUM, m_pElemVal1_3);
    break;

Code after generate: (note no changes were made to the GUI between these two examples

//<Button Enums !Start!> case E_ELEM_BTN1: gslc_SetPageCur(&m_gui, E_PG2); break; case E_ELEM_BTN5: // clicked on the graph so cycle to the next graph data changing the axis labels and axes to suit and pointing to the correct datasets switch (plotDataType)// enums: {extTempHum, extIAQHum, extEffHum, srcTempHum, supTempHum, extHumPres}; { case E_ELEM_NUMINPUT3: // Clicked on edit field, so show popup box and associate with this text field gslc_ElemXKeyPadInputAsk(&m_gui, m_pElemKeyPadNum, E_POP_KEYPAD_NUM, m_pElemVal1_3); break; case E_ELEM_NUMINPUT4: // Clicked on edit field, so show popup box and associate with this text field gslc_ElemXKeyPadInputAsk(&m_gui, m_pElemKeyPadNum, E_POP_KEYPAD_NUM, m_pElemVal1_2_4); break;

Pconti31 commented 2 years ago

@kcobley Sorry, but that is just a limitation of a program that lacks a full C++ parser. The User Guide documents in Appendix E - Case Statement Generation what and how the builder deals with button callbacks.
This is not something I can fix without a major rewrite so we will have to live with it.

I would suggest you call a function within your button callback that implements your imbedded switch statement. Many would consider that a better style to follow. Paul--

kcobley commented 2 years ago

Understood and thanks for the suggestion which should make life more comfortable.

It doesnt quite fit the Appendix E. It looks like what the Builder is doing is detecting the inner case statements of the inner switch as not being required anymore and deleting them. It thinks its deleting redundant button cases.

However, I understand that writing a parser that is robust to all possibilities is tough, and I respect you position not to try :-) It already seems to do an impressive job.