[Octave-bug-tracker] [bug #44219] Deleted GUI file editor tabs not forgotten upon command line "exit"

[Dan Sebald]

Follow-up Comment #2, bug #44219 (project octave):

I've looked at this patch and would like to revisit this.  The close mechanism
looks a little more clumsy than it needs to be, highlighted by this line of
code:


  // Here, we really want to exit and all tabs are closed
  _check_closing_done = true;  // check is already done, prevent a second
check
                               // which would store an empty file list
-varbatim-

Whenever I see that kind of construct, I've learned that in an object-oriented
framework there is likely a much better way, one that typically involves
building from the ground up using more basic routines that can be pieced
together and reused in more comprehensive functions.

I assume that when not at exit, and the user presses close for a tab, there is
a dialog that pops up if the file is not saved asking SAVE | DISCARD | CANCEL
?.  Let's call this operation CheckSave().  (I'm just making up function names
for illustration.)  So, when a tab X is selected, we have something like:


void CheckClose()
{
  if (CheckSave() != CANCELLED)
    Close();
}


Now, this file_editor::check_closing (int closing_state) routine attempts to
combine the checking and closing at once for the whole list which leads to the
problem that _check_closing_done is meant to solve.  Instead, I think going
through the list twice, first checking/saving, and then closing solves the
problem.  If the first loop doesn't make it all the way through because the
user types cancel somewhere, then all the files will still be open and typing
"exit" again and again will always have the same files open in the editor. 
That is:


  // Save all tabs, checking if user cancels.
  file_editor_tab *editor_tab;
  for (int index = _tab_widget->count ()-1; index >= 0; index--)
    {
      editor_tab = static_cast <file_editor_tab *> (_tab_widget->widget
(index));
      if ((! editor_tab->CheckSave (closing_state)) && closing_state == 1)
        return false;
    }

  // Close all tabs, they were all just saved so checking is redundant.
  for (int index = _tab_widget->count ()-1; index >= 0; index--)
    {
      editor_tab = static_cast <file_editor_tab *> (_tab_widget->widget
(index));
      editor_tab->CheckClose ();
    }


Also, let's try to get that static_cast out of there.  It's pretty safe in
this case, but using a static cast, in my mind, means we are moving outside
the paradigm of the Widget framework.  There might be a way we can use one of
the widget's virtual functions to access the file editor tab's close
function.

Do you want me to work on a change set?  Or do you understand what I'm saying
and can rework things quickly?


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?44219>

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