[Octave-patch-tracker] [patch #8006] New marker class to maintain breakpoints and position in modified files

[Dan Sebald]

URL:
  <http://savannah.gnu.org/patch/?8006>

                 Summary: New marker class to maintain breakpoints and
position in modified files
                 Project: GNU Octave
            Submitted by: sebald
            Submitted on: Mon 08 Apr 2013 09:36:43 PM GMT
                Category: None
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any

    _______________________________________________________

Details:

The attached changeset adds a marker class for the purpose of translating
Octave core debugger line numbers to line numbers in the edit area which
become out of synch if the user edits the file.  I think the behavior is
reasonably good so that the edit area doesn't need to be restricted from
editing when in debug mode.  The only restriction is that a breakpoint cannot
be set in a file that has been edited, and I think that is reasonable.

If the debugger position lands on a breakpoint, the editor line number can be
gotten from the marker object.  The marker object retains both the original
number (from Octave debugger) and the marker handle--from that it can figure
out editor line number.  Otherwise, I call it "unsure position" and use a
"gray" arrow to indicate where position is.  There are instances where the
breakpoint can be unsure, and I created a grayed redled as well.

This probably needs a slight bit of work yet.  There could be some things
going on at the Octave core debugger level not accounted for yet.  See
comments below about buggy behavior.


1) I'm fairly certain that no matter what one chooses for a behavior, the
marker handle will have to be retained in some way.  Removing a marker via
handle is a sure bet.  Removing a marker by saying remove a marker of this
type at this *debugger* line is problematic if the file has been edited.  In
other words, the method of this changeset is very robust at keeping track of
breakpoints.  The pre-changeset approach was prone to all sorts of markers
stuck in the left margin.  (If there are some stuck markers now, it is because
the debugger core goes out of synch with the original marker line numbers.)

2) Bookmarks are fine as is, because there is no interaction with the
debugger.  The line number is retrieved from the editor area, which naturally
agrees with bookmark line number.  No mods for that.

3) QsciScintilla will move markers appropriately, but it doesn't remove a
marker when the line it is attached to is deleted.  This is a bit of a problem
and in some sense almost a bug.  Delete a line where there is a marker and the
marker is now associated with a different line.  Undo the removed lines and
the marker stays with the post-delete line, i.e., does not go back to the
original marker line.  There is a signal to indicate when the number of lines
changes.  I've attached a slot to that and printed out the line number but
found it not of much use.  There's no indication that the marker's line has
been deleted.  What QsciScintilla should have is a signal "markerLineDeleted
(int mhandle)" and "markerLineUndeleted (int mhandle)".  With that, the
breakpoint could be greyed to indicate that there is a breakpoint somewhere
around there, but unsure exactly where.

There is a "markerFindNext" and "markerFindPrevious" for QsciScintilla, but
I'm not sure that is of any help with the translation between debugger line
number and editor line number.  I've put some slots in the marker object
labelled FUTURE SUPPORT that could be connect to a markerLineDeleted() and
markerLineUndelete() signals.  I'm going to post to the QScintilla list and
lobby for such signals.

4) Restoring breakpoints upon saving a file is more an editor type of thing. 
The file_editor_tab code issues a clear of all breakpoints then attempts to
restore them one at a time using the editor line numbers.  Works in theory,
but there are issues yet to be resolved.  See #6.

5) There appears to be a bug in the Octave core debugger.  I click on a line
inside of a local function and Octave core indicates there is a debug
breakpoint there.  (I've printed out messages to stderr inside
libinterp/interpfcn/debug.cc to confirm it is Octave core declaring successful
add breakpoint.)  However, neither "dbstatus" indicates such a breakpoint, nor
does the debugger break at that point when run.  However, I'm able to step
into local functions, so I would imagine setting breakpoints there shouldn't
be too much extra work.  Also, notice in the local functions that even
non-valid command lines can have a breakpoint.  I'll wait until this patch is
applied to write a bug report for that.

6) Beyond the bug/behavior in #5, the debugger still seems a bit buggy when it
comes to saving a file and trying to restablish breakpoints.  Often it seems
like the debugger is getting the lines wrong or there is some kind of
conflict.  Furthermore, a little while later after trying to add/delete more
breakpoints the alignment kind of corrects itself.  What I'm wondering is
whether there might be some synchronization problems.  To illustrate this, try
putting some breakpoints in the testfun.m and then add a line or two and save.
 It just doesn't seem to get things right when it shouldn't be too difficult. 
I'm pretty certain there is no way of losing track of the line number in files
with what the changeset setup.  The save sequence I'm using is

clear_all_breakpoints --> callback
save the file
individual add_breakpoint --> callback

What worries me a bit is that saving the file is not in synch with the Octave
process.  The clear_all_breakpoints in the Octave core goes through the list
and removes individually.  Perhaps it would be better to just delete all
breakpoints and notify that all of them have been deleted.  Anyway, I've left
some comments in the code so that someone might track what is happening.

7) Upon opening a file, I've tried placing breakpoints if there are some set. 
That information is gotten via dbstatus.  However, the callbacks and so on are
a bit of a barrier, or I don't understand them well enough.  I've put some
signals and slots in the code as placeholders illustrating a more general (and
what I think is a preferred) approach.  Queue up an octave command for which
the result is eventually signaled back to the object.

8) Sometimes it feels like maybe, at least conceptually, a list of markers
would do as well a the signal/slot connections.  I suppose at the center of it
all, the signal/slots are a sort of the list management.

9) For the "unsure" marker, I initially didn't like all gray because it didn't
contrast enough with the margin.  So I created a custom icon in gimp with a
bit of desaturation and copy/paste.  I later tried the QIcon effects of
QIcon::Disabled and QIcon::Selected.  The "Selected" isn't too bad.  So, I
made this an option under Preferences... "Debugger" menu.  We can pick just
one at a later time if people prefer no option.  It's a bit funny in the sense
that selecting the icon has the effect of "Selected" for all of them...LOL.




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Mon 08 Apr 2013 09:36:43 PM GMT  Name: redled_gray.png  Size: 475B   By:
sebald

<http://savannah.gnu.org/patch/download.php?file_id=27816>
-------------------------------------------------------
Date: Mon 08 Apr 2013 09:36:43 PM GMT  Name: arrow_right_gray.png  Size: 3kB  
By: sebald

<http://savannah.gnu.org/patch/download.php?file_id=27815>
-------------------------------------------------------
Date: Mon 08 Apr 2013 09:36:43 PM GMT  Name: octave-breakpoint-2013apr08.patch
 Size: 42kB   By: sebald

<http://savannah.gnu.org/patch/download.php?file_id=27814>
-------------------------------------------------------
Date: Mon 08 Apr 2013 09:36:43 PM GMT  Name: testfun.m  Size: 133B   By:
sebald

<http://savannah.gnu.org/patch/download.php?file_id=27817>

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8006>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/