bugGNU Octave - Bugs: bug #53046, Variable Editor: feature request...

 
 

bug #53046: Variable Editor: feature request to make multiple tables visible at once.

Submitter:  Dan Sebald <sebald>
Submitted:  Fri 02 Feb 2018 03:00:55 AM UTC
   
 
Category:  GUI Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 01 Mar 2018 09:36:06 PM UTC, comment #52: 

Dan: I pushed your changeset here:

http://hg.savannah.gnu.org/hgweb/octave/rev/ed9ba20d6ed3

Thanks for working on this feature.  I'm closing this report as fixed.  If there are problems with this patch or other problems with the variable editor more generally, we can open separate reports for them.

John W. Eaton <jwe>
Group administrator
Tue 20 Feb 2018 10:19:41 PM UTC, comment #51: 

Before your latest patch I couldn't move subpanels such that they are vertically stacked (cause could have been between chair and keyboard), so yes I find it easier now.
Horizontal or vertical is equal to me, I don't mind much, I think it just depends on the problem at hand on how to optimally place / resize / dock / undock / stack / tabify data panels.

If I were you I wouldn't try to add more functionality now, the patch works and IMO it is good as it currently is :-)

BTW many icons in a task bar looks like a non-issue to me. I sometimes have 40 or more figures, the taskbar just groups them neatly for me.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 20 Feb 2018 09:54:12 PM UTC, comment #50: 

OK, thanks for testing.  Do you find it easier to position the dock panels in the latest patch?  Do you like the vertical tabs as opposed to horizontal tabs (or even notice)?

Tabs inside floated panels:  That would be doable, but it would involve creating a derived float window...actually pretty much the same as the current V.E. window, but disallow a toolbar and only allow a single dock area so that tabifying is the only option.  To have more flexibility in the floated panel seems it would be basically allowing multiple V.E. windows, just like there can be multiple figures.  However, that might be confusing to have so many V.E. tabs in the desktop task bar.

Nonetheless, future enhancements can come later, after multiple users test and then get some feedback on what exactly the behavior should be.

Dan Sebald <sebald>
Tue 20 Feb 2018 09:23:52 PM UTC, comment #49: 

OK it runs on Linux (cross-building now).

Ignoring the programming details for now, I can just say that it works and adds more flexibility for displaying data which is always a big plus IMO. Thanks.

What does not work (but no big deal) is dragging a tab outside the VE window, then drag another tab out of the VE window and trying to add it ("dock it") into the former dragged-out tab sub-pane.
Obviously that is asking too much :-)

I think with this latest patch enough functionality has been added.

BTW, in an earlier comment somewhere I said that not even Matlab can do this, but I just tried and at least r2018a prerelease CAN.
Matlab has exactly the same limitation as I mentioned above as regards docking dragged out panes into another dragged-out one, so yes we are fully Matlab-compatible in that respect 8-)

Philip Nienhuis <philipnienhuis>
Group Member
Tue 20 Feb 2018 07:18:32 PM UTC, comment #48: 

Yes, backout the previous patch.  I typically use plain "patch" rather than "hg patch" for something like this.  The commands I use are something like:


patch -p1 < previous_version.patch
hg revert --all
patch -p1 < octave-multipanel_variable_editor-djs2018feb19.patch
patch -p1 < bug53046_ttl_01.patch


Dan Sebald <sebald>
Tue 20 Feb 2018 07:09:16 PM UTC, comment #47: 

How to apply the patch? backout the patch from comment #24 first?

Philip Nienhuis <philipnienhuis>
Group Member
Tue 20 Feb 2018 02:24:28 AM UTC, comment #46: 

OK, this one is ready for review.  I've done some significant restructuring, but I think you'll like it.  Now the various functions are associated with the classes that are most relevant.  There is a fair amount of simplification and removal of a layer of indirection as a result.  In summary, there are now the following classes:


variable_editor
variable_dock_widget (now just a few routines related to focus)
[holds]
variable_editor_stack (a few routines that manages view)
[holds]
variable_editor_view (the bulk of routines moved from variable_editor)


and naturally there is the additional QTextWidget contained in variable_editor_stack, but no real benefit to deriving a class in that case.

So, think of this as the majority of functionality being in the two classes

variable_editor_view
variable_editor_model

and then a few "glue" classes to organize things.  Patch attached.

(file #43368)

Dan Sebald <sebald>
Mon 19 Feb 2018 10:43:26 AM UTC, comment #45: 

I'm working on restructuring things a bit in the changeset.  I want to take away most of the functionality from variable_dock_widget and just have variable_dock_widget handle the focus/dock/undock; it doesn't seem like that should be there because the dock widgets are primarily container classes.  I've grouped those member functions either with the stacked widget (new class) or the table view (new class).  This shouldn't take long to debug and repost.

Dan Sebald <sebald>
Sun 18 Feb 2018 09:57:48 PM UTC, comment #44: 

That's fine Philip.  It's a subtle change, but I think one can tell the difference after using a while.  JWE can test.

Reference:

https://savannah.gnu.org/bugs/?53046#comment8

JWE said:  "I didn't find a way to assemble windows in a way that split the main window vertically, like this:

  +-------------------+
  |                   |
  |       var 1       |
  |                   |
  +-------------------+
  |                   |
  |       var 2       |
  |                   |
  +-------------------+

Is that possible?"

https://savannah.gnu.org/bugs/?53046#comment9

Dan said: "Panels can be tiled vertically. It takes a little practice, but just think of the central-widget model, shown here:

http://doc.qt.io/qt-5/qmainwindow.html#qt-main-window-framework
"

With the QMainWindow::AllowNestedDocks option, creating vertical stacks is quite easy.

Dan Sebald <sebald>
Sun 18 Feb 2018 09:44:34 PM UTC, comment #43: 

@Dan:
Unfortunately due to private affairs this week I may not have much time for testing until Thursday evening, and it is late on a Sunday evening now here. Maybe maybe tomorrow night I can give it a try.

What should the patch be applied to? do I have to backout Torsten's focus patch and your patch from comment #24 first?

W/o empirical testing it isn't easy to grab the meaning of what you write in comment #42 about free-form window behavior. But I'ĺl gladly try it out.

As for me I like separate floating VE panes all over the place, arranged, resized and contents shifted by the slider bars in such a way that just the interesting cells are visible. That is already possible after your patch from comment #24. I think it is brilliant.
AFAIK not even Matlab can do this, maybe not even several other IDEs for other script languages like Spyder, R-Studio, etc.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Feb 2018 08:38:23 PM UTC, comment #42: 

@Philip, comment #39:  Yes, I think that is a window manager difference.  We can try to address it, but I worry making the change for KDE might affect some other window manager.  Sprinkled throughout the Qt documentation are statements, "if the window manger does this", "if the window manager does that".

Anyway, since you are testing just now, could you try the latest changeset I attached in comment #41 and see if you like that free-form windowing behavior more than the restricted 4-sides format?  My assessment is that it's easier to subdivide where desired, but still getting things just right will require moving a dock here or there temporarily and then shuffle things around.  Try something like


for i=1:8
  eval(['var' num2str(i) ' = eye(5); openvar var' num2str(i)]);
end


then tabify docks in groups and so on.  Thanks.

Dan Sebald <sebald>
Sun 18 Feb 2018 08:29:42 PM UTC, comment #41: 

@JWE: Here are the flags I think you were interested in:

http://doc.qt.io/qt-5/qmainwindow.html#storing-state


  m_main->setDockOptions (QMainWindow::AllowNestedDocks |
                          QMainWindow::VerticalTabs);


The former option the sub-dock areas to be further subdivided which is a bit like free-form placement.  The latter option isn't always vertical tabs, but makes them vertical if the dock is at the left or right edge area.  (To clarify, I'm attaching a screen shot.)  I think combining those two (without restriction) doesn't make sense because people will wonder what the rule is for the vertical/horizontal tabs.  However, I can place the dock widget in the right area rather than the top area by default--and then I can restrict the dock widgets individually to the left/right areas.


  page->setAllowedAreas(Qt::LeftDockWidgetArea |
                        Qt::RightDockWidgetArea);
  m_main->addDockWidget (Qt::LeftDockWidgetArea, page);


I've incorporated those changes in the attached changeset (which doesn't have Torsten's addition, but his changeset should still apply).

file #43350)

Dan Sebald <sebald>
Sun 18 Feb 2018 08:12:12 PM UTC, comment #40: 

This is with Dan's latest patch + Torsten's (comment #33) on top of it, with tip of yesterday (90bd5649983c  Fix make_int example.... by Carlo)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Feb 2018 08:09:36 PM UTC, comment #39: 

On the bright side:
On Windows things look all-right a far as loosing focus & disappearing child panes go.
I really like it, I can make more efficient use of my desktop's real estate while debugging.

So, considering Dan doesn't see it on Gnome, it is supposedly a window manager-specific thing, right?

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Feb 2018 07:46:52 PM UTC, comment #38: 

@Torsten, comment #36:  Philip probably sees no change in visibility because it sounds like the floated dock windows always go hidden when the parent goes out of focus (that's the kwin behavior, apparently).  If closing the V.E. causes it to go out of focus, the floated windows were already hiding on his system.  In other words, he's unaware of the issue that your changeset addressed.

Dan Sebald <sebald>
Sun 18 Feb 2018 07:35:54 PM UTC, comment #37: 

I can't keep track of all the changes in GUI frameworks either.  KDE would be the kwin windows manager, I think.

https://blogs.kde.org/2007/09/08/qdockwidget-annoyance-including-hack-gets-rid-it

Dan Sebald <sebald>
Sun 18 Feb 2018 07:33:30 PM UTC, comment #36: 

Philip, no change of widged visibility following focus and/or following closing on VE?

Torsten Lilge <ttl>
Group Member
Sun 18 Feb 2018 07:10:06 PM UTC, comment #35: 

@Dan, comment #30:
I have Mageia linux 6, with KDE or is it called Plasma these days.
Little correction of 3rd stanza/paragraph last sentence there:
It's really odd, what you describe, the floated window hides when parent is not focused yet remains visible when the parent is hidden.
The floating window remain invisible when (undocked) parent is out of focus.
The focus of the parent window controls visibility of its children.

@Torsten;
with your patch I see no difference in focus/visibility behavior.
But later tonight I'll report on what happens on Windows.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Feb 2018 07:02:13 PM UTC, comment #34: 

The patch works here, thanks.  The nice thing about organized signals/slots is simple changes like this.  (Would it worthwhile putting these changesets on a new branch off dev?)

Now, for Philip's case, it's sort of the other direction: wanting to unhide the floated dock windows when the V.E. window becomes unfocused.  We could try just that, e.g., rewrite the virtual routine focusOutEvent():


variable_editor::focusOutEvent(QFocusEvent *event)
{
  QList<variable_dock_widget *> widglist = findChildren<variable_dock_widget *> ();
  for (int i = 0; i < widglist.size (); i++)
    {
      widglist.at (i)->setVisible (widglist.at (i)->is_visible ());
    }
}


where the visibility state would have to be stored in the variable_dock_widget object (prior to going out of focus...otherwise it might simply be storing the visibility state at the wrong time).  If the windowing system changes the visibility of those sub-windows after focusOutEvent() is called, then there isn't much one can do.  (That's what I meant about the windowing system being a sort of independent player in the Qt system.)  I'd rather not experiment with that sort of thing at this point, though, as it would just confuse that we have working.

Dan Sebald <sebald>
Sun 18 Feb 2018 01:17:25 PM UTC, comment #33: 

Patch for the visibility of variabel_dock_widgets attached.
It has to be used on top of Dan's patch.

(file #43348)

Torsten Lilge <ttl>
Group Member
Sun 18 Feb 2018 12:22:49 PM UTC, comment #32: 

The octave_dock_widgets are "manually" unparented when floating (make_window), the general QDockWidget undocking operation does not unparent the widget.

But I see, that variable_dock_widget just uses setFloating without unparenting, so this is not the reason for a floating widget staying visible when VE is closed.

What we can do is emitting a signal when the VE is closed in order to hide floating variable widgets. Or we can more generally change the visibility of floating widgets depending on the VE's visibility.

Torsten Lilge <ttl>
Group Member
Sun 18 Feb 2018 10:12:24 AM UTC, comment #31: 

Reply to comment #29:  I didn't test whether undocking unparents (I wouldn't think it should not do that, but maybe it does).  It does create a new window, at least from the window-system's perspective.  (QApplication::activeWindow() should indicate what the top level window of the focused object is.)  Perhaps there is a base-level object type to search for rather than "octave_dock_widget *" that will catch both types.  I'd also say that having everything connected to QApplication::focusChanged isn't a significant drain on the system.  Changing focus isn't a high-rate type of operation on the user's part, and there are much more computationally intensive operations, relatively speaking, like drawing the window contents.

Reply to comment #30:  The reason for the intermediate label_dock_widget is to give the same look-and-feel for that thin header in both the V.E. and in the main GUI window (both octave_dock_widget and variable_dock_widget now inherit label_dock_widget).  By maintaining a similar appearance and action it means faster comprehension for the user.  And also to give access to the label that is placed in that header so that its background color can be altered.  So, yes in a way it is a natural way of doing this, more along the line of OOP philosophy.  Otherwise, the variable_editor is doing all these things, replicating the code that constructs the header needing to access the labels either by some member function or keeping track of labels somewhere.

Dan Sebald <sebald>
Sun 18 Feb 2018 09:25:32 AM UTC, comment #30: 

Is it required to derive the variable_dock_widget from label_dock_widget with custom title bar and custom actions for (un)docking and closing the widget instead of directly deriving from QDockWidget?

Torsten Lilge <ttl>
Group Member
Sun 18 Feb 2018 09:13:52 AM UTC, comment #29: 

Reply to comment #26:

For being handled as a separate window, a dock widget is un-parented when it is floating. Therefore,


findChildren<octave_dock_widget *> (QString (), Qt::FindDirectChildrenOnly);


does not work. Using


focusWidget ()


doesn't work either, maybe because the currently focused object is not necessarily the focusWidget of the dock widget.


Torsten Lilge <ttl>
Group Member
Sun 18 Feb 2018 07:38:49 AM UTC, comment #28: 

OK, glad that fixed a few things.

As for the focus issue--i.e., that the "floated" variable windows disappear when out of focus--I can't really debug that without it happening here.  I'll look for any pertinent Qt setting, but I can't recall seeing anything related to that.

What's not immediately obvious is that Qt doesn't seem to control the desktop windowing system.  The windowing system manages what happens when clicking in a window and related functionality.  So, there is going to be variation on different windowing systems.  I'm using Cinnamon, which mimics the Gnome 2 look-and-feel (but seems more robust); the floated windows stay visible when parent is out of focus.  What system are you using?  I see a couple icons in your screen capture that look like maybe Xfce or something.  It's really odd, what you describe, the floated window hides when parent is not focused yet remains visible when the parent is hidden.

Here is someone from back in 2007 that found the same behavior for kwin:

https://blogs.kde.org/2007/09/08/qdockwidget-annoyance-including-hack-gets-rid-it

From that person's description, it sounds like the floated windows go into hidden mode.  (One can hide sub-panes via right-mouse-click and check/uncheck next to the window name.)  Do the floated windows reappear when the V.E. is brought back in focus?

Dan Sebald <sebald>
Sat 17 Feb 2018 09:36:35 PM UTC, comment #27: 

@Dan:
Thanks for the new patch, now double-clicking a cell containing an array as outlined in comment #22 does open a new tab.

The focus issues are still there, in spit of Torsten's fix, in short:
Tabs dragged outside an undocked VE pane get hidden when the VE looses focus and pop up again when it gets its focus back. The VE pane itself remains visible as far as it isn't overlapped by a window with focus.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 17 Feb 2018 08:57:17 AM UTC, comment #26: 

I'll let you think about focus since you are working on that, but to reiterate what I think you are saying is that rather than having everything hooked to the one signal focusChanged(), there might be a sort of fanning out at each level.  Such a thing could be done at the octave_dock_widget level, e.g.


focusChange -> octave_dock_widget_slot
if (octave_dock_widget is active)
  octave_dock_widget_signal -> variable_dock_widget


in which case all that needs to be done is change the connection of the variable_dock_widget.

Again, I'll let you work out the general theme, but looking at some of the code around that changeset, you might achieve some of that using already existing Qt methods.  Have a look at something like this:


findChildren<octave_dock_widget *> (QString (), Qt::FindDirectChildrenOnly);


instead of


main_window::dock_widget_list (void)


(see http://doc.qt.io/qt-5/qobject.html#findChildren) and


if (new_widget == focusWidget ())


instead of that code that looks at all the objects checking if they are ancestors.  The QObject::focusWidget() effectively does that (QObject::focusWidget() is not the same as the QApplication::focusWidget(), see http://doc.qt.io/qt-5/qwidget.html#focusWidget)

Dan Sebald <sebald>
Sat 17 Feb 2018 08:08:47 AM UTC, comment #25: 

A first comment on focus: I have changed the detection of the currently focused dock widget of the main window yesterday (http://hg.savannah.gnu.org/hgweb/octave/rev/860860e2e69e) because of some issues with the documentation widget. The new version should be safe in a sense that it should not interfere with the dock widgets of the variable editor. For the old version, I am not 100% sure.

The focusChanged signal is already used in the main window for detecting the currently active dock widget. We could introduce a signal that is emitted each time the variable editor gets or loses focus in order to prevent handling many focus change signals that aren't related to the variable editor at all.




Torsten Lilge <ttl>
Group Member
Sat 17 Feb 2018 03:38:40 AM UTC, comment #24: 

Several signal/slot connections got lost in the shuffle.  I went through the features I'm aware of from looking at the code and confirmed they are working again.  I also restored the "level up" button, now that I know more about what it is supposed to do.

I also undid this


" (%1); title (\"%2\");"


for the title, having realized that I could use %1 more than once.

With your example, I experimented a bit and I see that the plotting will fail when what is displayed in the QTableView is a cell array.  I.e., "plot(%1)" doesn't work for %1 that are cells, just matrices.  Feature-wise the V.E. should be able to translate to plottable format, but I can see looking at the code it will complicate things because the point at where that plot-command string is constructed it doesn't know much about the type of data it is working with, it's just constructing and indexing expression.  That's down the road.

For me, the multiple-floated variable_dock_widgets seems to work fine.  I do see what you are saying about the floated windows not closing along with the main V.E. window un-displaying.  That is strange, but I did check that the parent hierarchy is complete (i.e., everyone has a parent), but it could be that Qt is making the choice in this case.  There might be flag settings.  If not, I'm sure it is a simple one or two line fix.

The focus isn't 100% perfect--I can see some issues here too.  When hiding and re-opening the V.E. the floated window no longer has the blue highlight (even though I can tell it still has the focus).  The question is should this be addressed at the octave_dock_widget level or some added logic at the inherited variable_dock_widget level.

So, I'd like to hold off perfecting the blue highlight and focus issues.  First, the main goal here is to get back to the current status with the new QMainWindow layout along with code cleanup, better reuse of code, etc.  Code evolves so it is good to halt at a certain spot and let others digest things.  Second, I'd like to get input from Torsten regarding focus and such because where someone like me could spend five hours perfecting the focus, someone else who's dealt with it already could see a more precise way of doing it in fifteen minutes.  I know what I have right now for turning the blue background on and off is close, so I don't want to go adding all sorts of complicated logic that makes it difficult for someone else to debug.  I do know the fixes shouldn't be more than a few lines.

Anyway, a new version is attached with all features restored.

(file #43338)

Dan Sebald <sebald>
Fri 16 Feb 2018 06:46:39 PM UTC, comment #23: 

Funny - if after the steps described below I close the undocked parent VE pane, the relict child pane inherits all properties & 'power" from its parent except title bar and toolbars and subsequently behaves as any other window.
I expected it to disappear together with its parent.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 16 Feb 2018 06:41:41 PM UTC, comment #22: 

I tried it - nice!

Something that worked previously but doesn't work here (yet?) is opening nested arrays in a new tab.

Try:
a = num2cell (rand (3));
a{2, 3} = rand (2);
openvar ('a')

... and then double-click cell a(3, 2) => nothing happens.

A really good enhancement would be to be able to have multiple floating (undocked) variable editor panes.
That does work (somewhat) but quite fragile. When I pull a second  variable's 'tab"/pane from the (undocked) variable editor to the desktop it looses focus after a second (I have "Focus follows mouse' set) and disappears completely, even parts that weren't covered by windows that overlap it partly (see pictures bug53046_Vared_1_PRN.png (focus on VE) and bug53046_Vared_2_PRN.png (focus elsewhere)).
Clicking on the taskbar Variable Editor icon makes the child tab appear for a while; in fact the parent VarEd window and all its child windows get focus and moves to the foreground. Once that VE window loses focus all its children again disappear from sight.



Philip Nienhuis <philipnienhuis>
Group Member
Fri 16 Feb 2018 10:12:10 AM UTC, comment #21: 

OK, this changeset should be in good shape.  Here's a short summary of changes from the previous candidate:

1) I incorporated the recent addition of scrollbar behavior (var_editor_tab::handle_horizontal_scroll_action, var_editor_tab::handle_vertical_scroll_action):

http://hg.savannah.gnu.org/hgweb/octave/rev/ed2d9ef336a7

There's a bug in the behavior of the column/row sizes (may have been there prior), but can address that later.

2) I moved some of the member functions I first moved to variable_editor_model back to variable_dock_widget.  They just didn't seem to belong there.  Specifically, the range_selected() function isn't really a model type of thing.  This actually gets rid of some pointer indirection.  It might be a worthwhile step to create a new class derived from QTableView and put some of the functions in there--since it has access of the model via "setModel()", might as well put a few routines, signals and slots with the QTableView.

3) There was a lot of repetition in the code, so I tried to improve code-reuse with routines like:


QSignalMapper *
make_plot_mapper (QMenu *menu)
{
  QList<QString> list;
  list << "plot" << "bar" << "stem" << "stairs" << "area" << "pie" << "hist";

  QSignalMapper *plot_mapper = new QSignalMapper (menu);

  for (int i = 0; i < list.size(); ++i)
    {
      plot_mapper->setMapping
        (menu->addAction (list.at (i), plot_mapper, SLOT (map ())),
         "figure (); " + list.at (i) + " (%1); title (\"%2\");");
    }

  return plot_mapper;
}


that can be used for the three different context menus and the plot button in the tool-bar.  variable_dock_widget::add_edit_actions() is another new one.  It handles all the edit actions in context menus...and it shares a lot of similarities with the tool-bar buttons, but I couldn't figure out a good way of reusing the code there.

4) Notice in the above code I added a second format string for a second argument (same as the first argument) which will put the title of the data plotted on the plot.  If it isn't to your liking, make adjustments with future changesets.  In that regard, as with the previous versions of the changeset, there is now more useful selection capability.  One can select a column of data and plot just that column rather than the whole matrix.

5) The main issue I had to fix was that focus of the container class variable_dock_widget, in particular make sure that the blue highlighted widget title makes sense.  This actually wasn't too bad, and I spent more time just thinking about it rather than coding away.  The Qt documentation states that


focus : const bool

This property holds whether this widget (or its focus proxy) has the keyboard input focus


The "or its focus proxy" is the important part.  I reversed the proxy: rather than making variable_dock_widget a proxy for QTableView, I made QTableView the proxy for variable_dock_widget and then attached the application's focusChanged() signal to a new variable_dock_widget slot:


  connect (QApplication::instance(), SIGNAL (focusChanged (QWidget *, QWidget *)),
           page, SLOT (handle_focus_change (QWidget *, QWidget *)));


and that handle_focus_change() sets the label color appropriately.  And in set_editable() I set the proper proxy (and focus/no-focus) based upon whether it is the QTableView or QTextEdit that is the valid representation.

(file #43332)

Dan Sebald <sebald>
Wed 14 Feb 2018 10:02:13 PM UTC, comment #20: 
Dan Sebald <sebald>
Wed 14 Feb 2018 09:55:08 PM UTC, comment #19: 

I've restored proper operation of the QTableView/QTextEdit stack.  I also know that the table keyboard navigation is on account of the focus proxy.  Funny that Qt programmed the proxy that way and that there isn't a signal for when a QWidget comes into focus.  The fix should be simple, but I need to think a bit.  Either I have to find a way to pass keyboard events on to the container, or attach the variable_dock_widgets to the QAPplication::focusChanged() signal.  I won't have time to look at this until tonight.

JWE, another observance about the QTextEdit view of non-editable variables.  The QTextEdit should probably be set to non-editable mode.  Right now, a person can position a cursor in the text window, type, and characters start appearing.  I don't see a use for that because it's not like such text can be fed back into the core.  Also, shrinking the QTextEdit view shows that the text is wrapping rather than putting a scroll bar at the bottom.  I'd think that non-wrap would be nicer.  My guess is that both these changes should be simple attribute settings.

Dan Sebald <sebald>
Wed 14 Feb 2018 06:02:52 PM UTC, comment #18: 

Dan:  After using it for a while, I think it was the wrong decision to omit the numbers from the columns when adding new elements in those empty cells is allowed, for the same reason you note.  I'm going to restore them.  I don't think they need to be displayed in a different color.  People who use spreadsheets know that empty cells don't contain anything/are outside the bounds of the data.

John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 05:51:16 PM UTC, comment #17: 

"Say the matrix is 10x10, but only five columns is displayed at once"

I should add that in the patch you'll probably notice that clicking the "undock" icon the first time will expand the "is-floated" window to near maximum screen size.  That's my way of try to provide a magnify or maximize mode of sorts--just in case one wants to quickly expand a matrix window.  Such features could be improved with, say, an actual "maximize" button, etc.

Dan Sebald <sebald>
Wed 14 Feb 2018 05:42:12 PM UTC, comment #16: 

OK, got it.  Looks good.

I created a bug report here:

https://savannah.gnu.org/bugs/index.php?53158

I'll probably end up finding the fix for this when I freshen the patch and restore this QTableView/QTextEdit stack behavior.  It is probably in the same area where the QTableView not-navigable-by keyboard bug in my patch is.  So, if you don't want to search for the issue, I'll probably come across it.

@Rik, ha-ha, yeah software as usual.

BTW, one recent change I saw is that the QTableView view now only displays numbers out to the size of the matrix.  That's a good change.  (See my comment here:

https://savannah.gnu.org/bugs/?53110

)  However, as I'm using this I wonder if the numbers outside the matrix should still be included but instead display in light red, for example (or gray, but currently bold/normal is used for selected column and light gray might be too much info-overload for that parameter).  The problem without the numbers is if someone wants to extend the table by typing a number in a cell outside the matrix it can be a guess as to which column/row is selected.  Say the matrix is 10x10, but only five columns is displayed at once, and I want to place a 0 at cell 1,20.  The only accurate way would be by keystroke counting to that cell.  The way the mouse scrolls currently zips along, so no way of using that.

Dan Sebald <sebald>
Wed 14 Feb 2018 04:57:56 PM UTC, comment #15: 

I think we all knew.  LOL.

John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 04:43:24 PM UTC, comment #14: 

Who knew that a simple thing like a spreadsheet interface could be such a royal PITA?

Rik <rik5>
Group administrator
Wed 14 Feb 2018 04:39:53 PM UTC, comment #13: 

Oops, something is broken with initialization so if you start with a value that can't be edited, it doesn't work.  But if you switch from one that can to one that can't, it does.

So try this instead:


x = rand (10);
openvar x
## make VE visible, you should see spreadsheet display.
x = rand (10,20,30)
## VE display should switch to simple display of 3-d array.


John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 09:01:27 AM UTC, comment #12: 

Oh, I see what you mean now.  If it is 3-d matrices or greater, or perhaps even something that can't be displayed in a table, the contents of that variable widget is just the text output...that presumably can't be edited, but will update accordingly.  Updating with internal changes is probably the way it should be.  (This feature was part of the code I didn't understand, i.e., a QTextEdit in the mix...I'll recheck this.)

I've pulled the latest development code, but I'm not seeing this.  For

x = rand  (10,20,30)

all I see is a blank gray window with a tab labeled "x".  If I then change to

x = rand (10,20)

I see the QTableView/spreadsheet view.

I'm compiling version 981990aaa93191849f64a811853d6cca529cc6db which is the parent of my changeset and... nothing.  I could paste a screenshot if you like, but there isn't much to see but a blank window.  I've turned off pager and tried a few things, but I can see anything.

The copy/paste, yeah, it's using ASCII transference of data.  I know this because this is at least one thing I fixed.  One issue was an extra CR at the end that was interpretted as N + 1 elements when copying N elements and then pasting into N elements.  That ASCII choice was probably the simplest means of, say, pasting into an external spreadsheet.  Clipboarding isn't the easiest thing.  I've programmed some X clipboards, which is superflexible, but tricky.  (Have to ping the receiving/sending application for the data formats it accepts and which is the preferred format.  Then hopefully one can supply that format.)  I would think that Qt has added some kind of feature that will take data from a model and copy to/from clipboard.

Dan Sebald <sebald>
Wed 14 Feb 2018 07:54:49 AM UTC, comment #11: 

I agree that cutting and pasting needs work.  I really haven't looked at it.  I suspect it just copies and pastes text now, but I'm not sure.  If possible, we should be preserving the full precision of numeric values, not just copying the display values.

John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 07:51:51 AM UTC, comment #10: 

Go back to a revision prior to your changeset and do


x = rand  (10,20,30)
openvar x


Then open the variable editor window and you should see the same output as you see in the terminal window.

Note that the variable editor window updates if you change the value at the command line.

At first I thought this was a pointless feature, but then I realized that it would be useful for viewing objects during debugging, even if you couldn't edit them.

John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 06:41:25 AM UTC, comment #9: 

I was going to make several comments, many along the lines that you mention.

Panels can be tiled vertically.  It takes a little practice, but just think of the central-widget model, shown here:

http://doc.qt.io/qt-5/qmainwindow.html#qt-main-window-framework

I chose the populate method to be along the top, and along the top docked widgets are horizontal.  I chose that because a screen naturally has more width, plus to start arbitrarily choosing top/right/bottom/left and/or tabified would get haphazard.  It's the side dock areas that stack vertically.  So, start dragging panels to the left/right edges and soon one will find it.  It can be tricky to get the mouse pointed at the exact edge where a space opens up, but it isn't any different than the main GUI window.

Tabs on top rather than bottom?  Mmm, if it is a simple setting, perhaps, but that's not a show-stopper item.  As a first step, not too much rewriting of the way Qt does things is best.  Think of all the work they put into making the central widget layout work.  There is MDI mode.  The central widget is an MDI area.  The panels can all be moved into that area (not now, but we could make that an option) and then move the windows to roughly where one wants them, press a button and it will automatically tile.  But that's for another day.  I'll look around for a tab location setting, though.

Closing.  The QMainWindow has a context menu for hiding.  Try right mouse click in an area where there are no widgets.  Right there is a checkmark for visible/hidden.  There's a comment in the code showing the setting that will delete those windows rather than hide them (called "DeleteOnClose" or something).  Look in the variable_editor::variable_editor constructor.

Let me look at some of these other things.  I think most are simple fixes.  I noticed that currently copy/paste doesn't work, so I made sure to make that work to verify the various features are working.  But the copy/paste needs a great deal of work, just from looking at the code.  E.g., see here:

https://savannah.gnu.org/bugs/index.php?53110

There is a "Table Paste", which I have no idea how that is supposed to work.  Why two pastes?  Could there be just one if the innards pick to copy rows/columns without the extra empty cells?

"Variables that can't be edited, like N-d arrays are no longer displayed."

Give me an example of N-d arrays.  I know I ran into the issue you mention when I first started and I tracked it down to


-variable_editor::up (void)
-{
-  QString name = real_var_name (m_tab_widget->currentIndex ());
-
-  // FIXME: Is there a better way?
-
-  if (name.endsWith (')') || name.endsWith ('}'))
-    {
-      name.remove (QRegExp ("(\\(|\\{)[^({]*(\\)|\\})$"));
-      edit_variable (name, octave_value ());
-    }
-}
-


The empty octave_value () in there causes the variable to be "non-editable" and does that "shrunk to zero cells" display.  That behavior was recently added by one of your changesets, IIRC.  This is the consequence of freshening patches which probably pasted back in an octave_value() where it would have been fixed prior.  So, I probably just have to search for where the leftover octave_value() is used to launch the edit_variable.  In any case, give me an example code sequence and what to click on.

I noticed the missing keyboard navigation as well, and definitely missed being able to select and copyby keyboard.  I thought that navigation (like copy/paste) wasn't working currently, but I see now that it does work.  I probably just need to set some flag somewhere to allow keyboard input.

Dan Sebald <sebald>
Wed 14 Feb 2018 05:44:57 AM UTC, comment #8: 

Thanks.  That is definitely more flexible than tabs alone.

After playing with it for a few minutes, I noticed a few things:

I can see how to overlap two variable windows so that they behave like a tabbed interface.  Having the tabs on the bottom is OK, but I think most people would expect the tabs to be on top, and with buttons on the tabs next to the variable names to allow them to be closed.

I didn't find a way to assemble windows in a way that split the main window vertically, like this:



  +-------------------+
  |                   |
  |       var 1       |
  |                   |
  +-------------------+
  |                   |
  |       var 2       |
  |                   |
  +-------------------+


Is that possible?

Double clicking in an editor cell that contains a struct sub-element doesn't open a new variable editor window for me.  Instead, nothing happens.  The same for a sub-element that is a cell array.

Variables that can't be edited, like N-d arrays are no longer displayed.  All I see is a variable editor window with a single empty cell.  Previously, these were printed to a variable editor window.

Navigating in a variable editor window with the arrow keys no longer works for me.

Editing no longer works unless I double click in an editor cell first.  Previously, I could navigate with the arrow keys and just type in a cell once it was highlighted.  Double-clicking also worked to display the full value of the cell, but it wasn't necessary to do that just to change a value.

With all these things, I don't think I can take this patch as it is.

It might be good to split out some of the hunks that just improve the structure of the code and apply them separately, but I don't have time to do that myself.

John W. Eaton <jwe>
Group administrator
Wed 14 Feb 2018 05:11:11 AM UTC, comment #7: 

OK... :-)

Attached is a patch to use a QMainWindow model for the variable editor.  Shrinking down the central widget to nothing makes for a very versatile collection of panels with the look-and-feel of the main GUI window and all the similar features.

Not too much description required--lots of diff hunks the bulk of which is moving things around--but I'll add a follow up list of comments.  Aside from lengthy debugging and finding an ancillary-related crash bug that I thought was part of what I was changing:

https://savannah.gnu.org/bugs/?53137

the big snag here was the tool-bar stealing active window and focus when it came time to doing an action.  I first looked for a way to have QToolButtons not change active window, but I just don't think Qt can control that.  In order for a button to activate and receive events, the underlying window system has to make it's window active.  So, then I tried ways to get around that and return focus to the variable widget that had focus when the buttons were hovered.  Half of that was no trouble, but restoring focus just before the action was tricky.  In principle, it's easy with simple child objects

HoverToolButton (derived from QToolButton)
ReturnFocusToolButton (derived form HoverTOolButton)
ReturnFocusMenu (derived from QMenu)

and eventFilters with a few conditionals.  The problem is that the telling widget to activateWindow() isn't something synchronous with Qt's signal/slot or event system.  Checking the active window and focus object immediately after that call doesn't indicate the object that was just activated.  So, that was a bit of a race.  However, I think I finally found a useful function at the QApplication level.  A comment in the code explains it:


  variable_dock_widget *tofocus = findChild<variable_dock_widget > (m_hovered_focus_vname);
  if (tofocus != nullptr)
     {
      // Note that this may be platform and window system dependent.
      // On a particular Linux system, activateWindow() alone didn't
      // immediately set the active window and there was a race
      // between the window focus and action signal.  Setting the
      // active window via the QApplication route did work.
      QApplication::setActiveWindow(tofocus->window());
      tofocus->activateWindow ();
      tofocus->setFocus (Qt::OtherFocusReason);
     }


If this shows signs of trouble on some OS's then there is a simply solution which is to add the QString name of the variable to the broadcast rather than check focus.  But I didn't want to go to that immediately and pass extra data around until necessary.  I'd rather simulate the appearance that focus hasn't changed when pressing a tool button...and if there is someone who has a better idea of how to do this more directly or with event filtering at the toplevel window (I attempted this a bit to no avail), I'm open to that.  Like I said, though, I think the windowing system sort of governs the possible options.

(file #43286)

Dan Sebald <sebald>
Mon 12 Feb 2018 11:52:50 PM UTC, comment #6: 

Yeah, I like that plan too.  Could I work on this through the evening still?  I found one sort of related bug here:

https://savannah.gnu.org/bugs/index.php?53137

and I'm slowly commenting out code hunks to find this cntrl-C error.  Minor, non-structural bugs are easy to find and fix, but control signal types of bugs aren't.

Dan Sebald <sebald>
Mon 12 Feb 2018 11:47:25 PM UTC, comment #5: 

How about posting the draft of your changes.  Maybe someone else will see the fix for the crash.

In any case, I'd rather have the changes merged now so we can move on rather than waiting (bad for me) or going on anyway and causing merge problems later (bad for both of us).

John W. Eaton <jwe>
Group administrator
Mon 12 Feb 2018 09:00:44 PM UTC, comment #4: 

Yeah, thanks.  I'm ready for a first draft, but I'm stuck on a crash problem in which the first time cntrl-C/V are used crashes (but copy/paste buttons work).  Had to freshen the patch yesterday from the recent variable editor changes.  If I can get past that crash bug (has to do with having the window's "setWidget()" main widget just so), I'll post something.  Or, we could live with it, say on another branch (but that would probably mean a lengthy merge of branches).

I'm attaching a screen shot of what this looks like.  At first it may not look like a QMainWindow because there is no "center widget", but compare it to the main window and one can see the tabified areas, the common dock/undock and close buttons in the upper right corners of dock widgets, and the hide/unhide context menu.


Dan Sebald <sebald>
Mon 12 Feb 2018 08:38:24 PM UTC, comment #3: 

Dan, in the comments for bug #53105 you say that you just "about have ready a revamped variable editor" and point to this report.  Any update on progress?  I'd like to move ahead with fixing the various bug reports related to the variable editor but I don't want to cause a lot of merge conflicts or duplicate work.

John W. Eaton <jwe>
Group administrator
Fri 02 Feb 2018 12:46:11 PM UTC, comment #2: 

Didn't dare to ask (as JWE is already very busy wih the VE ATM), but:
+1

Philip Nienhuis <philipnienhuis>
Group Member
Fri 02 Feb 2018 05:39:10 AM UTC, comment #1: 

I've looked into this a bit.  For reference, I think the way to achieve multiple V.E. tables visible at once is to use Qt's QMdiArea:

http://doc.qt.io/qt-5/qmdiarea.html

MDI stands for multiple document interface which is a common term:

https://en.wikipedia.org/wiki/Multiple_document_interface

I believe the idea is that multiple documents can be visible at once.

Qt's MDI area can be in "tiled" mode

http://doc.qt.io/qt-5/qmdiarea.html#tileSubWindows

which I'm guessing is similar to the way the Octave GUI main window works in which subwindows expand to butt up against other subwindows but never overlap.  I think this is what would be preferred for the V.E.

The nice thing is that QMdiArea has an option tab feature:

http://doc.qt.io/qt-5/qmdiarea.html#ViewMode-enum

For some reason Qt documentation doesn't give a good picture of that tab area, but just think of the pictures given here:

http://doc.qt.io/qt-5/qmdiarea.html#details

as having a series of tables along the top.

Dan Sebald <sebald>
Fri 02 Feb 2018 03:00:55 AM UTC, original submission:  

Currently the variable editor (V.E.) has tabs that the user can switch between.  See if it is possible to "undock" those tabs so that there can be multiple tables in the V.E. at once.  This is how the main window behaves: when there are "Command Window", "Editor" tabs, for example, one can grab/drag these sub-windows and then become visible at the same time.

There will be just one V.E. toolbox, of course, so there needs to be a way to indicate which variable table is active.  I suggest making the octave tab/window have a white background while the non-octave ones have a gray background.

Dan Sebald <sebald>

 

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

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by sebald (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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-03-01 jwe StatusNone Fixed
        Open/ClosedOpen Closed
    2018-02-20 sebald Attached File- Added octave-multipanel_variable_editor-djs2018feb19.patch, #43368
    2018-02-19 rik5 Carbon-CopyRemoved 72865 -
    2018-02-18 sebald Attached File- Added vertical_and_horizontal_tabs_Screenshot_from_2018-02-18_14-05-06.png, #43349
        Attached File- Added octave-multipanel_variable_editor-djs2018feb18.patch, #43350
    2018-02-18 ttl Attached File- Added bug53046_ttl_01.patch, #43348
    2018-02-17 sebald Attached File- Added octave-multipanel_variable_editor-djs2018feb17.patch, #43338
    2018-02-16 philipnienhuis Attached File- Added bug53046_Vared_1_PRN.png, #43333
        Attached File- Added bug53046_Vared_2_PRN.png, #43334
    2018-02-16 sebald Attached File- Added octave-multipanel_variable_editor-djs2018feb16.patch, #43332
    2018-02-14 sebald Attached File- Added octave-multipanel_variable_editor-djs2018feb13.patch, #43286
    2018-02-12 sebald Attached File- Added first_draft_multitable_Screenshot_from_2018-02-12_14-54-45.png, #43263
    2018-02-02 rik5 Severity3 - Normal 1 - Wish
        SummaryAttempt to make multiple variable-editor tables visible at once. Variable Editor: feature request to make multiple tables visible at once.

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code