[Octave-bug-tracker] [bug #39179] octave_qt_link::do_edit_file always returns true even when QScintilla not present

[Dan Sebald]

Follow-up Comment #3, bug #39179 (project octave):

Thank you!  (That behavior was annoying.)

I've attached a changeset that will return various results for the GUI open
file function and pass that back to the edit.m script.  This is done using a
mutex and illustrates a general approach to freezing the worker process
momentarily while the GUI does some work.  Please try it out and see what you
think, and give some thought about this general process because I've used this
strategy in other places.  One thing I've added is to lock the mutex before
going into a wait state, then on the other side try to lock the mutex as a
means to verify the worker process has actually gone into a sleep state.  (The
way a mutex works is that even if it is in a lock state, it gives up its lock
and becomes unlocked when in the sleep state.)  I'm wondering if this may be
overdoing the "hand-shaking" a bit because on other examples I've run millions
of trials with and without the extra lock attempt and neither ever faltered.

Note that this patch solves the bug of do_edit_file () always returning true,
but beyond that the behavior under certain conditions might not be worked out
as desired.  For example, try:


edit plot


then


edit foodoo


In the case where there is no file, the GUI currently indicates no success,
then edit.m responds by opening an empty file of that name using the custom
editor.  If that behavior is to be altered in some way (GUI opens empty file,
edit.m gives warning about file not found), then that is a different
changeset.

As to your question about whether do_edit_file () should actually be run, I
think probably so although I'm not of strong opinion on that.  There are two
distinct philosophies to this, as I see it:

1) Use "#ifdef HAVE_QSCINTILLA" liberally to weed out all code that isn't
necessary for proper operation.  This is the approach of the changeset because
that seemed the way things were tending.  There is the advantage of reducing
unused code hunks when the feature isn't needed.  However, there are a couple
drawbacks to this.  One is that there gets to be a lot of "#ifdef
HAVE_QSCINTILLA" placed about in a half dozen files or so.  Sometimes it is
nice to keep that limited to just one or two files (the second philosophy).  A
second is that the action this code performed in the presence of QScintilla is
no longer available to do other things.

2) The second philosophy is to carry out all the same code to the very last
possible point where a decision can be made as to whether to call the
QScintilla code.  In that scenario in only one or two files will there be
"#ifdef HAVE_QSCINTILLA", probably somewhere in file-editor.cc near:


          file_editor_tab *fileEditorTab = new file_editor_tab ();


and just pass back a code that means GUI editor support is not present.

The reason I prefer philosophy 2 in this case is that I like not having so
many instances of HAVE_QSCINTILLA scattered about the code, but more than that
(if I understand correctly) there is currently a way for the GUI to use a
custom editor with "open" menu.  I think, but I may be wrong, with the current
setup where all this supporting code is extracted when there is no
HAVE_QSCINTILLA then even that custom editor can no longer be used.  The
custom editor is the first option inside of file_editor::request_open_file. 
That is why I think isolating things to one instance of HAVE_QSCINTILLA is
good.  (If folks agree, let's do that as a separate followup patch so that we
have a record of both approaches.  Shouldn't be a difficult change.)

(file #28285)
    _______________________________________________________

Additional Item Attachment:

File name: octave-edit_file-2013jun09.patch Size:8 KB


    _______________________________________________________

Reply to this item at:

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

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