bugGNU Octave - Bugs: bug #60469, F9 to run selected code stopped...

 
 

bug #60469: F9 to run selected code stopped working

Submitted by:  Philip Nienhuis <philipnienhuis>
Submitted on:  Wed 28 Apr 2021 07:54:33 PM UTC  
 
Category:  GUI Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Ready For Test Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  Open
Release:  dev Operating System:  Microsoft Windows

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Thu 27 May 2021 02:41:14 PM UTC, comment #21: 

One example is the color picker used for several configurable colors. It is derived from QPushButton and its "triggered" signal (which opens the color selection dialog) still works on Windows and Linux. The issue might be somehow related to the qscintilla lib.

Torsten Lilge <ttl>
Project Member
Thu 27 May 2021 01:12:12 PM UTC, comment #20: 

For my most recent build for Windows, I disabled symbol visibility attributes and the signals/slots failed to be connected (the build was made using an Octave revision prior to 81be7e4ddb0f.  So I guess the problem is related to inheritance, not visibility.  If it is an issue with signals and inheritance, I still don't understand why it works on Linux and not on Windows.  Don't we have other cases where we derive an object and connect to signals that are defined for the base class and everything works just fine?

John W. Eaton <jwe>
Project Administrator
Thu 27 May 2021 08:28:07 AM UTC, comment #19: 

I am not sure if this would help in this case. Only signals from the base class QsciScintilla are affected, even when they are connected directly in octave_qscintilla. Signals that are added in the derived class octave_qscintilla can be connected using the new syntax.

Torsten Lilge <ttl>
Project Member
Thu 27 May 2021 08:13:27 AM UTC, comment #18: 

Re comment #16: I can't rule out that symbol visibility is causing issues here.

I noticed that since the changes related to symbol visibility made around the end of last year on the default branch, we are not exporting some symbols from liboctinterp that we probably should. (We should probably get that sorted out some time before Octave 7 hits stable. But that is trickier than one might expect.)
I wouldn't be surprised if the same holds for some symbols from liboctgui.

I don't know how these signal slot connections are working and how symbol visibility might interfere. So this is just a guess:
Does it help to add the OCTGUI_API attribute to the octave_qscintilla class for this particular issue?

Markus M├╝tzel <mmuetzel>
Project Member
Thu 27 May 2021 07:32:28 AM UTC, comment #17: 

Just verified that the fix from comment #9 works (on my windows box). I just tried the <F9> thing and noted the the "Step" button wasn't greyed out anymore after selecting code.

I'll leave it to you to close this bug or continue the discussion.

Philip Nienhuis <philipnienhuis>
Project Member
Wed 26 May 2021 08:00:44 PM UTC, comment #16: 

Or, it is possible that symbol visibility is causing trouble here?  I really don't know.

John W. Eaton <jwe>
Project Administrator
Wed 26 May 2021 07:58:45 PM UTC, comment #15: 

Torsten: It seems that way, at least when the base and derived objects come from different DLLs?

John W. Eaton <jwe>
Project Administrator
Wed 26 May 2021 07:55:29 PM UTC, comment #14: 

Thanks for clarifying this. This means each time we are connecting a signal of the base class in a derived class we have to use the syntax from comment #12 or the old  SIGNAL/SLOT syntax?

Torsten Lilge <ttl>
Project Member
Wed 26 May 2021 05:31:03 PM UTC, comment #13: 

I should also note that the cast and the use of the base class name to refer to the signal method are both required.  Although they both compiled, neither of the following worked for me:

    connect (m_edit_area, &QsciScintilla::copyAvailable,
             this, &file_editor_tab::handle_copy_available);

    connect (dynamic_cast<QsciScintilla*> (m_edit_area),
             &octave_qscintilla::copyAvailable,
             this, &file_editor_tab::handle_copy_available);

John W. Eaton <jwe>
Project Administrator
Wed 26 May 2021 05:26:56 PM UTC, comment #12: 

I did some checking and it works for me if I use the following pattern with the new signal/slot connection method:

    connect (dynamic_cast<QsciScintilla*> (m_edit_area),
             &QsciScintilla::copyAvailable,
             this, &file_editor_tab::handle_copy_available);

for the signals that are emitted from the base QsciScintilla object itself.

As noted earlier, this problem only occurs on Windows systems.  Is it related to the way pointers are handled in DLLs?  Or is it just a difference in the way pointers are handled in a class like this where signals are inherited?  I'm not sure whether it is the connection or the execution of the signal/slot connection that fails.

I suppose it is good that there is a way to use the new signal/slot syntax in this case, but it seems quite bad to me that there is no warning for an inherited signal like this.

John W. Eaton <jwe>
Project Administrator
Wed 26 May 2021 12:59:24 PM UTC, comment #11: 

No, I want to eliminate SIGNAL/SLOT macros where we can but I noted that we need to be careful so that any explicit calls to disconnect() use the same method as the corresponding signal connection.

John W. Eaton <jwe>
Project Administrator
Wed 26 May 2021 11:48:03 AM UTC, comment #10: 

Thanks, I'll give it a try tonight. If it works I can again use the latest bleeding-edge crossbuilds, currently I'm "stuck" with one from April 14 (the last one I built where <F9> still worked).

BTW, didn't JWE prefer to revert all signal-slot connection changes rather than just a few? ISTR remember him mentioning s/th like that.

Philip Nienhuis <philipnienhuis>
Project Member
Wed 26 May 2021 10:44:48 AM UTC, comment #9: 

Cset http://hg.savannah.gnu.org/hgweb/octave/rev/81be7e4ddb0f fixes this for my Win 7 by reverting the signal/slot connections of all native qscintilla signal back to the old syntax.

Torsten Lilge <ttl>
Project Member
Sat 08 May 2021 07:55:51 PM UTC, comment #8: 

I wasn't successful with searching the internet for known issues with signal/slot connections using the new syntax. I suggest to revert the changes affecting Qscintilla

@jwe, what do you think? 

Torsten Lilge <ttl>
Project Member
Mon 03 May 2021 07:05:20 PM UTC, comment #7: 

It looks like the connection of QScintilla's own signals is failing, among them &octave_qscintilla::copyAvailable (causing this bug) or, e.g., signals for indicating file mofification or updating line and row indicator in the status bar.

When octave is started with the environment variable "OCTAVE_SHOW_GUI_MESSAGES=1", there are altogether 13 occurrences of the error message "QObject::connect: signal not found in octave::octave_qscintilla" for each opened editor file.

Could the way how Qscintilla is linked to octave in the mxe build might prevent the new signal/slot connection method from finding the signals?

Torsten Lilge <ttl>
Project Member
Sun 02 May 2021 07:25:55 PM UTC, comment #6: 

As I wrote, it happened somewhere between 14th and 20th of April. The cset Rik refers to fits in perfectly.

Philip Nienhuis <philipnienhuis>
Project Member
Sun 02 May 2021 04:51:30 PM UTC, comment #5: 

comment #4:

> This might be related to jwe's attempt to get rid of SIGNAL/SLOT usage.


I was thinking about this, too and am starting to look for the break in the signal chain. But whatever the result will be, interestingly only Windows is affected. Let's see.

Torsten Lilge <ttl>
Project Member
Sun 02 May 2021 02:22:45 PM UTC, comment #4: 

This might be related to jwe's attempt to get rid of SIGNAL/SLOT usage.  See this cset

changeset:   29542:3d34b70b5a49
user:        John W. Eaton <jwe@octave.org>
date:        Fri Apr 16 23:06:32 2021 -0400
summary:     connect many Qt signals and slots without SIGNAL and SLOT macros

Using 'hg bisect' for Windows is slow because it takes so long to cross-build through the MXE system.  But this might be a clue.

Rik <rik5>
Project Administrator
Sun 02 May 2021 01:58:44 PM UTC, comment #3: 

After a complete rebuild of my MXE envrionment (I had to do a "make clean" after a system upgrade), I was finally able to test this on my windows virtual machine.

The "Run selected code" action remains disabled even when text is selected. The same holds for the copy action, which stays disabled, too, although selected text is available. From this, it seems that the signal chain that should inform the main editor widget from state changes in the current tab's qscintilla area is not working anymore in Windows.

Torsten Lilge <ttl>
Project Member
Thu 29 Apr 2021 06:19:39 PM UTC, comment #2: 

I will do a recent windows build (will take some time) and test on my virtual box.

Torsten Lilge <ttl>
Project Member
Wed 28 Apr 2021 09:59:57 PM UTC, comment #1: 

FWIW, I tried on a Linux system with the text 'x = pi' and F9 worked fine.  This is with cset

parent: 29565:7733ca1db419
use shared_ptr to manage memory for mutex object

Rik <rik5>
Project Administrator
Wed 28 Apr 2021 07:54:33 PM UTC, original submission:  

Steps to reproduce;

  • In GUI editor, enter any command, e.g.: a = 1
  • Select it  & press F9
  • expected result: code is executed in command window
  • observed result: nothing

The last version I built where it worked is from April 14.
The next version I built is from April 20, doesn't work there.

It does still work in the 6.2.1 nightlies

Philip Nienhuis <philipnienhuis>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by ttl
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-05-26 ttl StatusConfirmed => Ready For Test
    2021-05-08 ttl Carbon-Copy- => Added jwe
    2021-05-02 ttl StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.6