bugGNU Octave - Bugs: bug #39723, history commands lost when...

 
 

bug #39723: history commands lost when selecting block and right-clicking "Evaluate"

Submitted by:  Dan Sebald <sebald>
Submitted on:  Thu 08 Aug 2013 01:38:18 AM UTC  
 
Category: GUISeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

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 Jump to the original submission)

Sun 25 Aug 2013 01:44:07 PM UTC, comment #9:

Fixed with changeset
http://hg.savannah.gnu.org/hgweb/octave/rev/352b442a72a5

Torsten <ttl>
Project Member
Sat 24 Aug 2013 03:50:08 AM UTC, comment #8:

I'm not sure grouping the commands into a single command is beneficial. I think I'd like the commands that are evaluated to appear in the history just as they were pulled from history. But more than that, the atomic or individual evaluation of commands allows some flexibility in the sense that the commands (presently callbacks) issued as a result of GUI interaction can get a chance to execute intermixed with longer processing commands that might be running. This would give the user more a sense of GUI responsiveness. The commands could also be prioritized. For example, most GUI operations like setting/clearing breakpoints are very short and don't have a command line text string. My druthers would be to make those operations (i.e., a "background" queue as opposed to a "foreground" queue) a higher priority, again, to make the GUI seem more responsive.

Anyway, thanks for taking the initiative on this one.

Dan Sebald <sebald>
Sat 24 Aug 2013 03:02:09 AM UTC, comment #7:

I guess a selection in the history widget is not disabled when a command finishes but when the next queued command starts (since then the command window gets focus). Maybe after this patch we can still make one multi-line command from all selected history entries (also for higher performance).

The limit was introduced for not accidentally consuming all system recources. But you are right here, the history is already limited. So I will remove this and apply the patch.

Torsten <ttl>
Project Member
Fri 23 Aug 2013 08:01:16 PM UTC, comment #6:

This does seem to behave better than the previous patch. In the previous patch I was able to get things into a state where the executed commands didn't appear until every one of the evaluate commands was through. Now, individual commands appear as they are completed and then Octave moves onto the next queued command. I'm able to intermix other callback items like debug commands and so on.

One behavior that would be worth changing (but as a separate patch) is the fact that if commands are running and one selects some lines in the history column (say I want to copy some lines from the history while evaluated commands are running), when the Octave command in the terminal is complete, the highlighted commands in the history are deactivated. I don't know how this works though--it might go into Octave core and eventually to a function called "rl_redisplay" and then come back to the terminal emulation that grabs focus when it redisplays. I can't find "rl_redisplay" in the source tree, so maybe that is a readline library kind of thing. Probably is because the same thing happens if I type a command in the command window when something is selected. Save this for another patch if it becomes too ancillary.

Why limit the queue to 512? Seems arbitrary and the queue, as far as I know, isn't like some buffer which takes up the maximum amount of space. I would think it is link-list-based. If the user finds a need to put so many commands in the queue and that bogs down the system, it is for his or her reasons. The worst that could happen is if a user accidentally types Cntrl-a which selects all of history and then executes the commands. How likely that is to happen, I don't know. The history is already limited (with the default being 1000), so if the user expands history, maybe they want to be able to select big blocks in history. (Of course, I'd advise them to create a script file to re-run commands.)

In summary, this is an improvement and works nicely. I'd just prefer it not be entwined with callbacks the way it is. On the whole, I guess it is worth applying the patch.

Dan Sebald <sebald>
Fri 23 Aug 2013 05:08:50 PM UTC, comment #5:

Thanks for the tip. Attached a patch with buffering.

(file #28915)

Torsten <ttl>
Project Member
Thu 22 Aug 2013 08:38:59 PM UTC, comment #4:

I've already done such a thing here:

https://savannah.gnu.org/patch/?8016

so before getting too far and reinventing the wheel, you might want to check that out. (Or not, to see if what you come up with is different.)

The queuing features are in the functions:

The additional feature in the above link is a way to send the result of the command back to the GUI but isn't essential for queuing, but instead avoiding callbacks:

The reason I suggest just going with what you have is that it is small change. We've had plenty of discussion on the list about a robust simple way of communicating between worker and GUI. I don't see the point of having a robust queuing system and callbacks and whatever else might be in the software right now.

Dan Sebald <sebald>
Thu 22 Aug 2013 08:12:03 PM UTC, comment #3:

I found an issue using the semaphore like this: When trying to evaluate long commands from the history, the third double click makes the gui not responding to user interaction until the commands are completed (which is normal since the gui waits for the semaphore). I guess this behavior is not expected by the users.

I am trying to extend the patch by queuing the commands.

Torsten <ttl>
Project Member
Thu 22 Aug 2013 12:40:15 AM UTC, comment #2:

Very creative solution given the constraints. It works fairly well here, and I'm inclined to say go with this because of the simplicity of it.

If I'm understanding correctly, the semaphore can remain within the GUI code because of the callback mechanism. That is, although the code looks like GUI code it is really being executed from the worker thread.

I'm not a fan of waiting on things (in this case, the next evaluated command is waiting for the previous to finish), but as I said, I don't think there is much else that can be done with the setup as it is. [The preference would be to simply queue the command and when the result is complete a signal is issued.]

I tried breaking things by evaluating long commands such as

and then try to inject other callback operations into the system like breakpoints and directory changes. It seems to behave fairly well, if only freezing the system for a while.

Dan Sebald <sebald>
Wed 21 Aug 2013 07:25:29 PM UTC, comment #1:

Attached a patch that fixes the issue for me. It is using a counting semaphore in a producer-consumer-scheme.

(file #28900)

Torsten <ttl>
Project Member
Thu 08 Aug 2013 01:38:18 AM UTC, original submission:

After the changeset in bug report #39722, selecting a block of commands in the history window and then evaluating via the right click mechanism will not process all commands. Some appear to be lost in the signal/slot communication with the core.

For example, I issue "test assert" several times at the command line, select a block of four of them and when right-clicking "Evaluate" get:

i.e., two were dropped. Also, it appears to be random in the sense that sometimes there are three commands processed. I have confirmed in this hunk of code from history-doc-widget.cc:

that in fact all commands are being emitted in the for-loop. So it seems to me that the slot on the receiving end of the signal is dropping commands if they come too quickly, not sure.

Dan Sebald <sebald>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #28915:  sync_hist_exec_02.patch added by ttl (5KiB - text/x-diff)
file #28900:  sync_hist_exec_01.patch added by ttl (2KiB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by ttl (Updated the item)
  • -unavailable- added by sebald (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 25 Aug 2013 01:44:07 PM UTCttlStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Fri 23 Aug 2013 05:08:50 PM UTCttlAttached File-=>Added sync_hist_exec_02.patch, #28915
    Wed 21 Aug 2013 07:25:29 PM UTCttlAttached File-=>Added sync_hist_exec_01.patch, #28900

    Back to the top


    Powered by Savane 3.1-cleanup1