bugGNU Octave - Bugs: bug #42705, Commands continue to be sent to...

 
 

bug #42705: Commands continue to be sent to the terminal if breakpoint hit

Submitter:  Richard <crobar>
Submitted:  Tue 08 Jul 2014 12:54:49 PM UTC
   
 
Category:  GUI Severity:  2 - Minor
Priority:  3 - Low Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Richard Crozier Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 25 Oct 2019 03:54:38 PM UTC, comment #15: 

Chnagest http://hg.savannah.gnu.org/hgweb/octave/rev/410622ac120f fixes this by using temporary files. Sourcing and removing the temp. file does not show up in the console or in the history but the commands selected in the editor do.

Torsten Lilge <ttl>
Group Member
Sun 03 Jul 2016 11:41:38 AM UTC, comment #14: 

Retagging from "patch submitted" to "in progress" because the patch causes messy output, and should possibly paste the commands to the screen as they are executed as the previous code did.

Lachlan Andrew <lachlan>
Fri 12 Feb 2016 06:27:58 PM UTC, comment #13: 

It can be avoided to show the commands by adding another derived octave_cmd or by using the run-file mechanism. I will have a look at it the next days.

Torsten Lilge <ttl>
Group Member
Thu 11 Feb 2016 10:58:08 AM UTC, comment #12: 

Thanks Torsten.  The message box you are seeing is probably the same one that I reported as "weakness 2" with the original patch.

The attached patch fixes that.

It is still messy that the "source" and "delete" commands are printed, but I don't know how to fix that.  We rely on the queueing mechanism, and so can't just call Fsource and Fdelete directly.  I wonder if there is demand for a general mechanism for triggering commands without them being echoed to the screen.

(file #36312)

Lachlan Andrew <lachlan>
Thu 11 Feb 2016 06:02:11 AM UTC, comment #11: 

When having the keyboard () command within the selection, the execution stops as expected showing the debug> prompt and the temp. file is opened in the editor. However, the queued delete command is already executed leading to a message box about the deletion of the file.

Torsten Lilge <ttl>
Group Member
Wed 10 Feb 2016 01:59:05 AM UTC, comment #10: 

Here is an updated changeset.

It deletes the script immediately after sourcing it.  One unexpected drawback is that single-stepping through the code can no longer show the commands as they are executed.

I modified the editor code so it doesn't prompt to re-create an empty file with the name of the temporary script.  In doing so, I noticed that there are two copies of the code that produce essentially the same dialog: one using QMessageBox in file_editor::request_open_file  and one using uiwidget_creator in octave_link::do_prompt_new_edit_file.  This is just like the case we have with dbstop in bug #46795.  In the long term, I think we should have a way of creating dialog boxes that doesn't depend so much on the context it is called from.  Perhaps uiwidget_creator could be adapted to detect the context.

There is also conflict between two things galled "octave_link".  One is a function that is octave's version of the "link" file system function call, and the other links the GUI to Octave.  Dan might argue that it breaks modularity to call file systems ops from the GUI.  I'm open to suggestions of how else to generate the temporary script name.

(Sorry this comment is so long...)

(file #36285)

Lachlan Andrew <lachlan>
Tue 09 Feb 2016 10:49:23 PM UTC, comment #9: 

Torsten, I don't think it is a problem to have the "delete" show in the command history; the "source" already shows.  In some ways it may be useful to see where in the history the run-selection occurred.  That is different from having 10 or more commands being added.  However, if you want to get rid of it, that's fine too.

My concern with deleting the file before the end was that it may stop Octave from executing properly after the breakpoint is hit, but it seems to be fine; the first command after the breakpoint is the delete, and Octave keeps going through the parse tree.

Also, if you don't see the dialog box when the file is deleted, that is a good thing -- it was an undesirable side effect.

I'll submit a new patch soon.

Lachlan Andrew <lachlan>
Mon 08 Feb 2016 09:27:17 PM UTC, comment #8: 

Lachlan, thanks for the patch. Some comments:

1. A possible solution is to emit the delete command directly after emitting the source command and not including delete into the script. As a drawback, the delete command is shown in the terminal. This in turn can be avoided when putting the delete command in a second file and running both files instead of executing the source command in the terminal. I will have a look at this possibility.

2. I am not seeing a dialog box (?)

Torsten Lilge <ttl>
Group Member
Mon 25 Jan 2016 03:20:59 AM UTC, comment #7: 

I like Richard's idea of treating the selection as a script, instead of using execute_command_in_terminal_signal.  It is already easy to copy-and-paste the selection into the command window.  Being able to run text without it cluttering the command history would be useful new functionality.

As a strawman, I've created a changeset that creates a temporary file and executes it.  It still has several weaknesses (below), but I thought I'd get your (Torsten's) opinion before going further.

1. If the script is terminated before the end, the temporary file is not deleted.
2. When it is deleted, a dialog box comes up mentioning that it has been deleted. (This is related to the bug where that dialog comes up when typing "delete".  Fixing that bug should fix this.)
3. The configure process should check that mkstemp is available.
4. Some of the code should probably be put somewhere else for modularity.

As you know, breakpoints are my current pet project, so I'd like this issue fixed :)



(file #36159)

Lachlan Andrew <lachlan>
Wed 30 Jul 2014 05:43:02 PM UTC, comment #6: 

You can't find the line in the sources. This would be the solution where all selected text is evaluated as one text block in the terminal. Please find attached the diff to the actual version.

(file #31801)

Torsten Lilge <ttl>
Group Member
Wed 30 Jul 2014 02:04:58 PM UTC, comment #5: 

Sorry, typo:


rcrozier@crozier-laptop ~/src/octave-src $ grep -rnI execute_command_in_terminal_signal .
./libgui/src/octave-qt-link.h:159: void execute_command_in_terminal_signal (const QString& command);
./libgui/src/octave-qt-link.cc:359: emit execute_command_in_terminal_signal (QString::fromStdString (command));
./libgui/src/m-editor/moc-file-editor.cc:212: "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-file-editor.cc:338: case 41: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-file-editor.cc:719:void file_editor::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/file-editor-tab.cc:87: SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/m-editor/file-editor-tab.cc:204: emit execute_command_in_terminal_signal (command); // connected to main window
./libgui/src/m-editor/octave-qscintilla.h:52: void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/file-editor-tab.h:135: void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/octave-qscintilla.cc:262: emit execute_command_in_terminal_signal (command + _word_at_cursor);
./libgui/src/m-editor/octave-qscintilla.cc:268: emit execute_command_in_terminal_signal (QString("edit ") + _word_at_cursor);
./libgui/src/m-editor/octave-qscintilla.cc:277: emit execute_command_in_terminal_signal (commands.at (i));
./libgui/src/m-editor/moc-file-editor-tab.cc:125: "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-file-editor-tab.cc:205: case 7: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-file-editor-tab.cc:364:void file_editor_tab::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/moc-octave-qscintilla.cc:54: "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-octave-qscintilla.cc:70: case 0: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-octave-qscintilla.cc:125:void octave_qscintilla::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/file-editor.h:114: void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/file-editor.cc:189: emit execute_command_in_terminal_signal ("edit " + new_name);
./libgui/src/m-editor/file-editor.cc:1409: connect (this, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/m-editor/file-editor.cc:1465: connect (f, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/moc-octave-qt-link.cc:65: "command\0execute_command_in_terminal_signal(QString)\0"
./libgui/src/moc-octave-qt-link.cc:91: case 4: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/moc-octave-qt-link.cc:179:void octave_qt_link::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/main-window.cc:1372: SIGNAL (execute_command_in_terminal_signal (QString)),
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:189: emit execute_command_in_terminal_signal ("edit " + new_name);
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:1405: connect (this, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:1461: connect (f, SIGNAL (execute_command_in_terminal_signal (const QString&)),


Richard <crobar>
Wed 30 Jul 2014 02:03:50 PM UTC, comment #4: 

Sorry, I'm still not sure about this. I don't see the line you quote anywhere in the sources:

++verbatib++
rcrozier@crozier-laptop ~/src/octave-src $ grep -rnI execute_command_in_terminal_signal .
./libgui/src/octave-qt-link.h:159:  void execute_command_in_terminal_signal (const QString& command);
./libgui/src/octave-qt-link.cc:359:  emit execute_command_in_terminal_signal (QString::fromStdString (command));
./libgui/src/m-editor/moc-file-editor.cc:212:    "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-file-editor.cc:338:        case 41: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-file-editor.cc:719:void file_editor::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/file-editor-tab.cc:87:           SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/m-editor/file-editor-tab.cc:204:  emit execute_command_in_terminal_signal (command); // connected to main window
./libgui/src/m-editor/octave-qscintilla.h:52:  void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/file-editor-tab.h:135:  void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/octave-qscintilla.cc:262:  emit execute_command_in_terminal_signal (command + _word_at_cursor);
./libgui/src/m-editor/octave-qscintilla.cc:268:  emit execute_command_in_terminal_signal (QString("edit ") + _word_at_cursor);
./libgui/src/m-editor/octave-qscintilla.cc:277:    emit execute_command_in_terminal_signal (commands.at (i));
./libgui/src/m-editor/moc-file-editor-tab.cc:125:    "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-file-editor-tab.cc:205:        case 7: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-file-editor-tab.cc:364:void file_editor_tab::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/moc-octave-qscintilla.cc:54:    "execute_command_in_terminal_signal(QString)\0"
./libgui/src/m-editor/moc-octave-qscintilla.cc:70:        case 0: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/m-editor/moc-octave-qscintilla.cc:125:void octave_qscintilla::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/m-editor/file-editor.h:114:  void execute_command_in_terminal_signal (const QString&);
./libgui/src/m-editor/file-editor.cc:189:      emit execute_command_in_terminal_signal ("edit " + new_name);
./libgui/src/m-editor/file-editor.cc:1409:  connect (this, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/m-editor/file-editor.cc:1465:  connect (f, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./libgui/src/moc-octave-qt-link.cc:65:    "command\0execute_command_in_terminal_signal(QString)\0"
./libgui/src/moc-octave-qt-link.cc:91:        case 4: _t->execute_command_in_terminal_signal((reinterpret_cast< const QString()>(_a[1]))); break;
./libgui/src/moc-octave-qt-link.cc:179:void octave_qt_link::execute_command_in_terminal_signal(const QString & _t1)
./libgui/src/main-window.cc:1372:           SIGNAL (execute_command_in_terminal_signal (QString)),
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:189:      emit execute_command_in_terminal_signal ("edit " + new_name);
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:1405:  connect (this, SIGNAL (execute_command_in_terminal_signal (const QString&)),
./.hg/merge/f886cf611844ebec7c8dd6e7a740454c68353356:1461:  connect (f, SIGNAL (execute_command_in_terminal_signal (const QString&)),
--verbatim--

I'm on the gui-release branch hg id returns: 12462638ab20+ (gui-release) tip

Richard <crobar>
Mon 21 Jul 2014 04:48:13 AM UTC, comment #3: 

With comment #1 I wanted to say that if the selection is not split into single lines but sent as one text block to the terminal, i.e.


emit execute_command_in_terminal_signal (selectedText ());


only the part before the first newline is evaluated. Your example then leads to


>> x = 1
y = 5
x =  1
>>


and y is not evaluated:


>> y
error: 'y' undefined near line 1 column 1



Torsten Lilge <ttl>
Group Member
Sun 20 Jul 2014 11:15:53 PM UTC, comment #2: 

I'm not sure what you mean by this? If I highlight the following in the editor and do 'Run Selection', all lines are sent to the terminal.


x = 1
y = 5


i.e. I get the following output:


>> x = 1

x =  1
>> y = 5

y =  5


This is what I expect since as per the followng code in octave_qscintilla.cc


octave_qscintilla::contextmenu_run (bool)
{
  QStringList commands = selectedText ().split (QRegExp("[\r\n]"),
                                                QString::SkipEmptyParts);
  for (int i = 0; i < commands.size (); i++ )
    emit execute_command_in_terminal_signal (commands.at (i));
}


Richard <crobar>
Sun 20 Jul 2014 08:17:17 PM UTC, comment #1: 

When sending all text as one block, only the part before the first newline is evaluated.

Torsten Lilge <ttl>
Group Member
Tue 08 Jul 2014 12:54:49 PM UTC, original submission:  


If you use 'Run Selection' to send commands to the command window, and one of those commands results in you hitting a break point in your code, flow stops at the break point, but the remaining commands then continue to be sent to the terminal, usually resulting in an error.

It would be better in general if commands sent in this way acted as a single 'block' sent to the interpreter (as though they were in a script file).

Might this also avoid the painfully slow printing of the commands on the GUI terminal when using 'Run Selection' too?

Looking into the code (octave_qscintilla.cc around line 244) I see that the command is split on [\r\n] and fed line by line to the octave link. Is the Octave interpreter unable to accept newlines in its input?

I'm on the default branch hg id returns: 24332f256940 tip @



Richard <crobar>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36312:  bug_42705_v2.cset added by lachlan (4KiB - application/octet-stream)
file #36285:  bug_42705_v1.cset added by lachlan (4KiB - application/octet-stream)
file #36159:  bug_42705_strawman.cset added by lachlan (2KiB - application/octet-stream)
file #31801:  execute_selected_text.diff added by ttl (618B - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by crobar (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-25 ttl StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2016-07-03 lachlan StatusPatch Submitted In Progress
    2016-02-11 lachlan Attached File- Added bug_42705_v2.cset, #36312
    2016-02-10 lachlan Attached File- Added bug_42705_v1.cset, #36285
    2016-02-08 lachlan Severity3 - Normal 2 - Minor
        Priority5 - Normal 3 - Low
        StatusNone Patch Submitted
    2016-01-25 lachlan Attached File- Added bug_42705_strawman.cset, #36159
    2014-07-30 ttl Attached File- Added execute_selected_text.diff, #31801

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code