FLTK logo

STR #3213

FLTK matrix user chat room
(using Element browser app)   FLTK gitter user chat room   GitHub FLTK Project   FLTK News RSS Feed  
  FLTK Apps      FLTK Library      Forums      Links     Login 
 Home  |  Articles & FAQs  |  Bugs & Features  |  Documentation  |  Download  |  Screenshots  ]
 

Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]

STR #3213

Application:FLTK Library
Status:1 - Closed w/Resolution
Priority:1 - Request for Enhancement, e.g. asking for a feature
Scope:3 - Applies to all machines and operating systems
Subsystem:FLUID
Summary:External Editor for Fluid.
Version:1.3-feature
Created By:matt
Assigned To:greg.ercolano
Fix Version:1.3.4 (SVN: v11816)
Update Notification:

Receive EMails Don't Receive EMails

Trouble Report Files:


Name/Time/Date Filename/Size  
 
#1 greg.ercolano
17:23 Jun 17, 2016
editor.patch
49k
 
 
#2 greg.ercolano
17:53 Jun 26, 2016
editor2.patch
51k
 
 
#3 greg.ercolano
23:38 Jul 07, 2016
erco-fluid-ext-editors-v1.patch
41k
 
 
#4 greg.ercolano
02:56 Jul 09, 2016
erco-fluid-ext-editors-v2.patch
51k
 
 
#5 greg.ercolano
10:48 Jul 09, 2016
erco-fluid-ext-editors-v3.patch
49k
 
 
#6 greg.ercolano
02:14 Jul 10, 2016
erco-fluid-ext-editors-v4.patch
70k
 
 
#7 greg.ercolano
17:10 Jul 11, 2016
erco-fluid-ext-editors-v5.patch
57k
 
 
#8 greg.ercolano
11:15 Jul 13, 2016
erco-fluid-ext-editors-v6.patch
63k
 
 
#9 AlbrechtS
07:20 Jul 15, 2016
update_fluid.patch
4k
 
 
#10 AlbrechtS
07:27 Jul 15, 2016
fix_mingw_build.patch
2k
 
 
#11 AlbrechtS
07:35 Jul 15, 2016
fluid-ext-editors-v7.patch
28k
 
 
#12 AlbrechtS
08:43 Jul 15, 2016
fluid-ext-editors-v7a.patch
67k
 
 
#13 greg.ercolano
10:49 Jul 15, 2016
fluid-ext-editors-v7b.patch
66k
 
     

Trouble Report Comments:


Name/Time/Date Text  
 
#1 matt
04:40 Apr 04, 2015
Should at least prevent multiple edits on same code.
      Should probably use threads to fire off editor, and when
      editor completes, apply changes using a global lock.
      Tree window should still be scrollable while edit(s) are in progress.
      On close or compile, should warn if any unsaved editors are still open.
      (Multiple editors open on different code should be OK, as long as
      they're all closed before Alt-G, Alt-S, etc)
      Use temp files that are named appropriately to the code being
      edited, so that the editor's window title gives a hint which code
      is being edited.
      I think mingodad has a patch already in STR#2794,
      not sure if he implemented it this way.
 
 
#2 engelsman
16:53 Apr 08, 2015
I just saw there's also a very old fluid_hack at http://www.fltk.org/links.php?V54+Q  
 
#3 greg.ercolano
17:23 Jun 17, 2016
Adding Muhammad Muquit's code here as a patch against 1.0.11
so one can see what's going on.
 
 
#4 greg.ercolano
17:53 Jun 26, 2016
Hmm, noticed I had the src/dest swapped when I made "editor.patch" (File #1),
old code was showing up as '+', and new code as '-'.

Posting new "editor2.patch" (File #2) which has the patch the right way around;
old code is '-', new code is '+'.
 
 
#5 greg.ercolano
19:13 Jun 26, 2016
Agree with most of Matt's comments in #1.

One thing:

    I'm not sure we have to make sure all open editors are closed
    to use Alt-G/Alt-S, etc.

    Why can't we just leave fluid-invoked external editors open
    at all times, and just suck in whatever code is in those live
    editor files during builds (Alt-X/Alt-G) or saves (Ctrl-S, Shift-Ctrl-C).

    This would mean perhaps we wouldn't even need threads or locks..
    we just have to make sure all editors are closed before quitting fluid,
    which I think would just be process management (e.g. checking for child
    processes), easy to do across unix + windows, and no threads.

    Perhaps I'm missing something, but I think that would maybe improve usability,
    as then one can operate pretty much just like normal; with multiple edits open
    at all times, without having to loose e.g. cursor positions in the code between
    builds, etc.

Comments?
 
 
#6 greg.ercolano
23:38 Jul 07, 2016
Attaching a v1 patch that allows external editors in fluid,
proof of concept regarding being able to manage multiple editors open,
and that they can remain open during Save / Write Code operations.

Currently linux only; not tested on Mac (though it might work),
and no code yet for Win32.

You should be able to apply the patch to svn current (r11800), e.g.

   patch -p0 < erco-fluid-ext-editors-v1.patch

To configure the external editor:

    1) Open Edit -> GUI Settings
    2) Enable "Use external editor"
    3) Set the "Editor Command" to e.g. "gvim -f"

The editor command must:

    (a) open its own GUI window, and
    (b) must not background itself (should remain in "foreground")

You should then be able to open an .fl file, double click on any methods/functions
that have "Code" (New->Code->Code) assigned, and it should open in the editor.

You should be able to make changes in the editor and save them, and those
changes should be seen inside fluid within a second or two of the save:

    a) Tree should show first line of code, whatever it is
    b) Any open "Source Viewer" with "refresh" enabled should auto-update as well
    c) File -> Save should work even with editors open
    d) File -> Write Code should work even with editors open
    e) File -> Save should be activated (not grayed out) whenever an edit is "saved"

FEATURES:

    > There can be multiple editors open at a time (one for each "Code" item)

    > The editor can remain open even when hitting "Save" or "Write code"

V1 PATCH CAVEATS:

    > Currently v1 patch is linux only

    > Tested only with "gvim -f" as the editor. Works with "gedit" too, didn't try others

    > Boundary cases not tested e.g. if editors left open when new .fl loaded, or Code
      deleted. When testing this patch, close the editors first before exiting or loading
      new .fl files. Handling these cases cleanly are a WIP.
 
 
#7 greg.ercolano
00:02 Jul 08, 2016
Here's a quick video demoing the v1 patch:
http://seriss.com/people/erco/fltk/tmp/fluid-ext-editor-demo-v1.mov
..which shows:

    > The new "GUI Settings" for configuring the editor command/mode
    > Opening a new .fl file, and opening two code windows in GVIM
    > Making changes in GVIM and seeing realtime changes in tree
    > Try to re-open already open code windows (warning dialog posted)
    > Enable "Show source code" viewer w/autorefresh, see realtime changes from GVIM
    > Closing and re-opening code editor windows
    > Show how "File -> Save" changes from grayed out to enabled when edits are made
    > Disabling external editor to revert to normal fluid code editing
 
 
#8 dtopham
23:13 Jul 08, 2016
Thank you for making this patch available. So far I have only built stable releases. (Most recently to enable cairo support.) 

I downloaded the r11800 code and applied the patch (no errors).  But building I encountered this unfamiliar error:

./configure --enable-cairo
configure: error: cannot run /bin/bash ./config.sub

Do you have any suggestions?

config.sub is there and with x permissions in the misc directory
 
 
#9 greg.ercolano
02:56 Jul 09, 2016
Attaching v2 patch to address some of the boundary issues.  
 
#10 greg.ercolano
03:01 Jul 09, 2016
dtopham: I don't recognize it either.

So as not to clutter this STR, suggest taking the question to fltk.general if the following doesn't work.

Error seems unrelated to the patch; suggest trying again with clean r11800 and first verify /that/ builds before applying patch, then do a 'make clean; make' rebuild.
 
 
#11 greg.ercolano
10:48 Jul 09, 2016
Tested on OSX with "mvim -f" (Mac version of gvim)
To install mvim, see: http://stackoverflow.com/questions/21012203/gvim-or-macvim-in-mac-os-x
Specifically:
    Step 1. Install homebrew from here: http://brew.sh
    Step 2. Run: brew update
    Step 3. Run: brew install vim && brew install macvim
    Step 4. Run: brew link macvim
Be sure /usr/local/bin is in your PATH.

Attaching v3 which removed some excess bookkeeping code that turned out to be unneeded (L_editors[], etc), some code + comment cleanup/simplification.

Wanted to tighten up code before I start on the Win32 version of the mods, which will be next. Want to actually close this out + commit before I move on to any other STRs.
 
 
#12 dtopham
20:51 Jul 09, 2016
Thanks Greg, You are correct; the problem I experienced (config.sub), was not an issue with the patch. It appeared to be an issue with building r11800 (which seems to require automake/autoconf) and building 1.3.3 stable (which does not).  When I got beyond that I could test the features on my Debian 8 system. All worked as shown in the video (very helpful). However I had been expecting it to also work with the callback area of widgets (such as a button). I used geany as my external editor, but could not see how to invoke it for callbacks. Is that possible?
-Dave
 
 
#13 greg.ercolano
02:08 Jul 10, 2016
dtopham: Yes, I suppose it could also handle the callback code area.
I haven't looked into it.
As you've seen, currently only only New -> Code -> Code is supported for the external editor.
 
 
#14 greg.ercolano
02:14 Jul 10, 2016
Attaching v4 patch which includes Win32 support.
So now supports all three platforms.

Includes an updated ide/VisualC2010/fluid.vcxproj that includes the correct files for building in the VS 2010 IDE. (I think I used 2012 VSE)

Tested with gvim74.exe as the external editor.
Overall behavior is the same with Linux, OSX and Windows.

This is the first pass at the win32 code; created the patch once it seemed to be working. Will give the win32 code a good once over tomorrow.
 
 
#15 greg.ercolano
17:10 Jul 11, 2016
OK, attaching a second pass code cleanup/simplification of win32+unix code as v5.

I'm thinking this is pretty close to committable.
I'd say safe to use this for code review.

* * *

Summary of changes:

fluid/Fl_Type.h              -- hooks for code writing and editor mod detection
fluid/Fl_Type.cxx            -- mods for writing code
fluid/Fl_Function_Type.cxx   -- hooks to open external editor on double click, write hooks
fluid/fluid.cxx            _ -- mods for timer checks, close editors on exit, new preference globals
fluid/alignment_panel.fl    \
fluid/alignment_panel.h      >-- mods for new config options to configure editor command + on/off flag
fluid/alignment_panel.cxx  _/      _
fluid/ExternalCodeEditor_WIN32.h    \__ new external editor class (windows implementation)
fluid/ExternalCodeEditor_WIN32.cxx _/ _
fluid/ExternalCodeEditor_UNIX.cxx      \__ new external editor class (unix implementation)
fluid/ExternalCodeEditor_UNIX.h       _/
fluid/Makefile                 -- mods to build new ExternalCodeEditor class files
ide/VisualC2010/fluid.vcxproj  -- mods to build new ExternalCodeEditor files
 
 
#16 greg.ercolano
11:15 Jul 13, 2016
Attaching erco-fluid-ext-editors-v6.patch which has IDE + CMake mods
so that the external editor mods will build with all IDEs and makefile
techniques:

   ide/VisualC2008  -- modified, build tested with VS2012
   ide/VisualC2010  -- modified, build tested with VS2012
   ide/VisualC6     -- modified, build tested with VS 7.net
   ide/Xcode4       -- modified, build tested with Xcode7
   fluid/CMakeLists.txt -- modified, build tested with cmake 3.5 on linux

I didn't test mingw builds with configure or cmake, and didn't test
cmake with OSX.

Would like to commit this patch.
Might be easier to commit it than to have folks apply the patch + test.

Requesting vote from devs to accept.
 
 
#17 AlbrechtS
07:20 Jul 15, 2016
Intermediate test status and fixes:

Cygwin (Posix/X11 build) worked, but I was able to make fluid hang after starting the editor (`notepad++ -multiInst`: -multiInst prevents backgrounding), probably with a bad commandline or something like that. The editor process seemed to crash, but fluid didn't recover from that (it hung). I needed to kill the fluid process. Still needs testing, I'll try to find and describe a reproducer. OTOH, using (Cygwin) vi as editor worked.

The sources created from the new fluid files seemed to have been created with an older version of fluid. This could be fixed easily, see attached file update_fluid.patch. This patch must be applied after the v6 patch.
 
 
#18 AlbrechtS
07:27 Jul 15, 2016
MinGW build didn't work OOTB, but I found fixes. Errors and warnings:

=== making fluid ===
Compiling ExternalCodeEditor_WIN32.cxx...
ExternalCodeEditor_WIN32.cxx: In function 'int terminate_app(DWORD, DWORD)':
ExternalCodeEditor_WIN32.cxx:108:11: warning: unused variable 'dwRet' [-Wunused-variable]
   DWORD   dwRet ;
           ^
ExternalCodeEditor_WIN32.cxx: In member function 'int ExternalCodeEditor::handle_changes(const char**, int)':
ExternalCodeEditor_WIN32.cxx:189:32: error: 'GetFileSizeEx' was not declared in this scope
   if ( GetFileSizeEx(fh, &fsize) == 0 ) {
                                ^
ExternalCodeEditor_WIN32.cxx: In function 'int save_file(const char*, const char*, FILETIME&, LARGE_INTEGER&)':
ExternalCodeEditor_WIN32.cxx:360:38: error: 'GetFileSizeEx' was not declared in this scope
     if ( GetFileSizeEx(fh, &file_size) == 0 ) {
                                      ^
ExternalCodeEditor_WIN32.cxx: In member function 'int ExternalCodeEditor::start_editor(const char*, const char*)':
ExternalCodeEditor_WIN32.cxx:405:28: warning: passing NULL to non-pointer argument 5 of 'BOOL CreateProcessA(LPCSTR, LPSTR, LPSECURITY_ATTRIBUTES, LPSECURITY_ATTRIBUTES, BOOL, DWORD, PVOID, LPCSTR, LPSTARTUPINFOA, LPPROCESS_INFORMATION)' [-Wconversion-null]
                     &pinfo_) == 0 ) {   // process info
                            ^
ExternalCodeEditor_WIN32.cxx:405:28: warning: passing NULL to non-pointer argument 6 of 'BOOL CreateProcessA(LPCSTR, LPSTR, LPSECURITY_ATTRIBUTES, LPSECURITY_ATTRIBUTES, BOOL, DWORD, PVOID, LPCSTR, LPSTARTUPINFOA, LPPROCESS_INFORMATION)' [-Wconversion-null]
make[1]: *** [ExternalCodeEditor_WIN32.o] Error 1

----------------

Note: dependencies are NOT set correctly in autoconf/make build. Changing one of the *_WIN32.{cxx|h} files did not recompile the file. Likely needs manual dependency editing in fluid/Makefile for Windows, since `make depend` won't do it for us. :-(

Note2: make depend should be executed (on a Linux system) since we're likely out of sync anyway.

Fixes are in attached file fix_mingw_build.patch. Still needs more tests...
 
 
#19 AlbrechtS
07:35 Jul 15, 2016
Notes concerning the MinGW patch:

I compiled with lots of warnings enabled (unused variable dwRet).

CreateProcess() used two pointers instead of variables. I guessed that we need 0 and FALSE, resp.. Maybe that's wrong. However, this should be fixed for all Windows compilations, it's not MinGW related.

MinGW's default is a lower WINVER number, hence we need to "fix" this to get the declaration of GetFileSizeEx().

I'm uploading the total patch in fluid-ext-editors-v7.patch.
 
 
#20 AlbrechtS
07:38 Jul 15, 2016
One more observation/question so far: The Xcode patch introduces:

  MACOSX_DEPLOYMENT_TARGET = 10.10;

Is this correct/appropriate? I have no idea, but I suspect this should not be there.
 
 
#21 greg.ercolano
08:19 Jul 15, 2016
Hmm, you might want to recheck your v7 patch; it doesn't seem
to have any differences from v6 other than:

    o The ExternalCodeEditor* files are absent..
      be sure to 'svn add' them before doing the svn diff
      so that they're included in the diff

    o There seems to be unrelated patches to the
      fluid/about_panel.cxx and fluid/print_panel.cxx?

Otherwise, there don't actually seem to be any changes relative
to v7; perhaps it's a diff of the wrong code base?

I just diffed v6 / v7 patches to see what changed, and after
rearranging the order of the files in the v6 patch to match v7's,
the code seemed all the same..? Only the rev#s in the diff headers
were different -- 11804 vs 11810.. which maybe explains the unrelated
fluid panel mods.
 
 
#22 AlbrechtS
08:24 Jul 15, 2016
More test results (Windows):

Using notepad++ under Windows works.
Interesting: no quoting needed for executable file in path with spaces.

FYI: This is what I used with notepad++ (under Windows 8.1):
  "C:\Program Files (x86)\Notepad++\notepad++ -multiInst"

Note: enter w/o quotes in the "Editor command" field in GUI settings. Works well.

notepad++ shows correct line endings, because it understands bare linefeed line endings, but ...


Using 'notepad' as editor (enter just "notepad" w/o quotes) doesn't show the correct line formatting. Under Windows the temporary file _should_ be written with cr/lf, i.e. using "text mode". I don't intend to fix this now, awaiting response. Greg, I'm sure you'll find that piece of code easier than I could do. AFAICT using text mode under Linux/UNIX doesn't do any harm, so there is probably no need to use platform specific code here.
 
 
#23 AlbrechtS
08:27 Jul 15, 2016
Oh, and I forgot: The previous tests under Windows were done after building the ide/VisualC2010 project with Visual Studio 2015 after "converting" the projects and running "demo .. fluid", so this seems to work well.  
 
#24 greg.ercolano
08:30 Jul 15, 2016
@Comment #17: cool about getting at least cywin + vi to work.
Not sure what's up with the "notepad++ -multiInst".. did you try
without the multiInst flag? It might work OK; Windows is weird in that
e.g. when a windows process appears to background, it doesn't loose
connection with CreateProcess().. so you might not need that flag on
Windows.

Regarding fluid versions, note my patches are against r11804; see the rev#s
in the v6 patch, e.g.

    Index: fluid/CMakeLists.txt
    ===================================================================
    --- fluid/CMakeLists.txt        (revision 11804)   <--
    +++ fluid/CMakeLists.txt        (working copy)

@Comment #20: Yes, the deploy target /was/ 10.6, but on my Xcode7/10.10 box,
it wouldn't run the 10.6 deployment, saying 10.10 was "too old", lol.
I think Xcode7 has a bug that thinks .10 is less than .6 (alphabetically?)
So I left it as 10.10, mostly out of frustration, but yes, should probably
leave it at 10.6 in the final, as presumably they'll fix that bug.
 
 
#25 AlbrechtS
08:43 Jul 15, 2016
@ #21: You're right, I missed 'svn add' for the new files. I worked with git primarily (it's much easier for me to push and pull commits between my Linux VM, the Windows host, and another Windows/Linux PC).

Please take a look at attached file fluid-ext-editors-v7a.patch.

The "unrelated" patches to the *panel source and header files are from using the latest fluid to generate these files from their .fl files. I mentioned this also in my comment #17. It's really not _directly_ related (the code works w/o these patches, and I did not change any source (.fl) files), but it should be fixed anyway, because this is "current" fluid code generation.
 
 
#26 greg.ercolano
08:45 Jul 15, 2016
@comment #22:
I'm curious about spaces being honored in the path w/out quotes,
as that /might/ mean command line flags /aren't/ being honored.
But just to clarify something, you mentioned you used:
"C:\Program Files (x86)\Notepad++\notepad++ -multiInst"
If you're trying to test with quotes, I'm thinking that should instead be:
"C:\Program Files (x86)\Notepad++\notepad++" -multiInst
..quotes around just the executable path, not the flag? Try that.
Or maybe CreateProcess is auto-detecting the executable, and
realizing it can honor any spaces after that?

Regarding notepad + bad line endings, yes.. that is 'normal';
notepad sucks ;)

I noticed fluid generates inconsistently generates crlf vs. lf
endings when you switch editing in fluid win32 <-> unix.
That behavior is unchanged with the external editor mods, and
it does come up, even when only using fluid's built in editor.

Note my external editor code uses CreateFile/WriteFile/ReadFile
which has no text vs. binary mode; it just writes the data unmodified.
The text v. binary is only in the win32 fopen stuff, which I chose not
to use, to ensure there's no weird permissions/acls/file locking during
write operations (I enable SHARING during write), so the editor doesn't
get confused.

I didn't try to 'fix' that, but that might be something that should
be fixed in a separate STR.

@Comment #23: good..! And I most appreciate testing/getting mingw
to work, as I currently don't have a way to build+test with that.
 
 
#27 greg.ercolano
09:04 Jul 15, 2016
OK, just checked v7a.

So mainly the changes wrt v6 are:

   1) Some CreateProcess() args were changed: NULL,NULL -> FALSE,0
   2) Got rid of unused dwRet variable

..which both seem good to me.

The macro mods to get GetFileSizeEx() working on MinGW is interesting.
Glad you figured that out ;) I might have switched back to GetFileSize()
(without the 'Ex') just to avoid the issue. I could do that BTW; the
only reason to use 'Ex' IIRC is the data type can handle super large
file sizes.. but in our use case, file sizes will be small, so using
'Ex' /could/ be avoided. But I think I'll leave it if the macro
technique solves it.
 
 
#28 greg.ercolano
09:08 Jul 15, 2016
OK, so assuming your happy with that Albrecht, I'll go for a commit.

I think the only thing I'll revert is that Xcode deploy target number,
making sure it stays 10.6 for fluid instead of 10.10.

Will try to make that happen later today; I really want to close this
so the brain cells I've used for this STR are free to be reassigned to
other purposes ;)
 
 
#29 AlbrechtS
09:41 Jul 15, 2016
WRT quoting: The MinGW fluid executable works with and w/o quoting. I don't know why, it's a mystery. I recommend correct quoting (as you suggested, only around the exe file name) to get the best results.

With notepad++ "-multiInst" (i.e. "Multi Instance") is necessary. I tried it w/o, and it seems to work "somehow" (sometimes), but it depends on the fact whether another notepad++ instance is already running. If another instance is active the new instance just sends a message to the running instance and terminates immediately. That's not what we want. It's the same with svn and git commits, BTW. The editor process must stay active for git and svn to see when it is time to read the commit message.

> The macro mods to get GetFileSizeEx() working on MinGW is interesting.
> Glad you figured that out ;)

That was easy. Just a little Google'ing to find the correct WINVER. I had to do this before in another case in my software, and (!) we have already similar code in src/Fl.cxx. So I just copied this and removed the #if WIN32 conditional because it was unnecessary here.

Regarding "text" vs. "binary" mode: yes, Windows sucks, but notepad is consistent with Windows and hence "correct". The problem is that other Windows users may use notepad and/or other Windows editors that can't cope with LF only line endings. So if we offer using a platform specific editor we should IMHO also write platform specific temp files.

> OK, so assuming your happy with that Albrecht, I'll go for a commit.

I'm not sure yet. I'd like to try the other IDE's first, but maybe we can fix this later if we have a first commit. Also note that I didn't review any code, except reading the diffs and looking for "strange" things. But if you think the code is good, then go for it...

> ... Xcode deploy target number ...

The patch adds one or more lines with just these deploy target statements I cited in my comment. I believe the best solution would be to use an editor and delete these statements (better than using the IDE so _set_ another one).


Okay, that said, going to test Visual2008 and Visual2010 IDE projects with exactly these two IDE versions, resp. on my other PC. Please wait a moment, I'll be back soon...
 
 
#30 greg.ercolano
09:59 Jul 15, 2016
@Comment #29:
> it depends on the
> fact whether another notepad++ instance is already running.

    Mmm, yeah, I never encountered that situation with Gvim;
    one process per edit. But I can see where e.g. notepad++
    and probably others like sublime might have that issue.
    Well, I guess we can only support them if they allow
    one-process-per-edit sessions.

> other Windows users may use notepad

    God help them ;)

    IIRC, notepad has no line numbering, and there were versions
    that couldn't handle files above some small limit (64k?).
    Its a horrible editor. Wordpad is at least somewhat semi-decent.

> So if we offer using a platform specific editor
> we should IMHO also write platform specific temp files.

    Mmm -- I think addressing this might be a larger issue
    that has to be dealt with in fluid's internals

    I'd thought about having the tmpfile writer doing crlf translations,
    but when I realized crlf handling was inconsistent across platforms
    (i.e. no internal translating was being done, so sometimes ^M's
    would appear in the line endings of some lines in the .fl and .cxx/h
    files), I didn't want to try to change that.

    But certainly it could convert to crlf's when writing the temp file
    on windows.. this just means tainting the code, as then editors that
    can handle/maintain unix style lf-only endings would get usurped.

    I think it's a larger issue that should probably be dealt with
    separately, as I think whatever solution you find for the temp files
    will have affects on the internal fluid code as well.

>> OK, so assuming your happy with that Albrecht, I'll go for a commit.
>
> I'm not sure yet. I'd like to try the other IDE's first, but maybe
> we can fix this later if we have a first commit.

    That's what I'm thinking -- I think it'll be easier to collaborate
    thru svn than patches at this point. I'm pretty sure all the build
    platforms should work now, and any that don't we can surely fix
    as per usual.

    I guess I'll have to repeat-apply to the 1.3-porting branch.
    Is that one cmake only now? Then we don't have to deal with the
    ide/xcode files, which would be great.
    I kinda forgot what the 1.3-porting branch addresses.

> But if you think the code is good, then go for it...

    I do.. it's "clean", no hacks.

    The only things that might be a problem are the issues with
    race conditions I mentioned, and some small/optional TODO items
    (grep TODO from the patch)

> Please wait a moment, I'll be back soon...

    OK -- I won't do any commits until we sync.
 
 
#31 greg.ercolano
10:49 Jul 15, 2016
Attaching v7b patch, same as v7a but removed unwanted Xcode mods,
just so we're on the same page with that.
 
 
#32 AlbrechtS
11:13 Jul 15, 2016
FTR: tested (with patch v7a) all Windows IDE's successfully with...

ide/VisualC6: Visual Studio 2008 Express
ide/VisualC2008: Visual Studio 2008 Express
ide/VisualC2010: Visual Studio 2010 Express
ide/VisualC2010: Visual Studio 2015 Express

Note: I did not test the latest patch (v7b) which shouldn't matter. I only tested one open editor at a time.

So far everything looks good, AFAICT.
 
 
#33 AlbrechtS
11:18 Jul 15, 2016
There is only one issue I'd like to have documented VERY clearly because it _could_ do harm: reaping the editor process(es) when FLTK exits may cause data loss if the user opens another file and edits that file in an editor instance started by fluid. This may be done after closing the fluid temp file or in an editor with tabs even while editing the temp (code) file.

Hence, AFAICT, reaping the editor(s) when the fluid window is closed can really be dangerous. Did you document that fact, or should it be documented? Or should we _not_ do this but let the user close the editor window(s)?

Last, but not least: excellent work, thanks a lot!
 
 
#34 greg.ercolano
11:24 Jul 15, 2016
@Comment #33, re. closing the editor on fluid exit

    Mmm, the reason to close editors for temp files is so that
    if one has two fluid instances running with open editors
    and closes one, it should close all windows related to that session
    to prevent confusion.

    I could make that an option in the GUI Settings window;
    "Close editors when fluid exits: yes/no"

    Perhaps a follow up STR for that?
    I'm not too concerned about this STR's mods, because users
    don't have to enable the feature, and they can always turn it off
    if they decide they don't like using it. Default is off.
 
 
#35 AlbrechtS
11:29 Jul 15, 2016
@Comment #30:

> I guess I'll have to repeat-apply to the 1.3-porting branch.

Yep, otherwise this would be missing in FLTK 1.4.

> Is that one cmake only now?

CMake + autoconf/make. Unfortunately we could not drop autoconf, hence we still have to deal with Makefile's and dependencies (for make).

> Then we don't have to deal with the ide/xcode files, which would be great.

Yep, this is really a big advantage.

> I kinda forgot what the 1.3-porting branch addresses.

Better (multi) platform support (platform separation by virtual "driver" methods), CMake as the main build platform (but still autoconf, as mentioned before), no IDE's, new ABI (all ABI guards have been removed), more virtual methods in the core classes (not yet done) and more to come...
 
 
#36 greg.ercolano
11:34 Jul 15, 2016
The other thing I could do is perhaps post a warning dialog
to force the user to first close the open editors, instead of
closing them automatically.

This could either be in place of the auto-close code, or could
be a GUI Settings option, e.g.

     On quitting fluid, any external editors still open should..
         a) Be closed automatically
         b) Warning dialog that asks user to close them first
         c) Not be closed.. just leave them open

But again, because of potential confusion with multiple fluid
sessions, it might be hard for the user to know which editors
need to be closed to affect the session being closed.

In all cases though, fluid would remove the temp files on quit.
So editors left open would now be editing a file that has been removed
in a directory that has been removed, so the editor wouldn't let them
save it without renaming it.. good feedback I guess to prevent data loss.

So I think making a GUI option as a separate RFE might be a good
thing to solve this. Want to treat that as a separate feature, just
so we can get this STR closed + committed.
here
 
 
#37 AlbrechtS
11:35 Jul 15, 2016
ACK.  
 
#38 greg.ercolano
11:41 Jul 15, 2016
..and regarding Comment #36, I suppose (b) could be the default
GUI Setting option: warn the user to close the editors before quitting
instead of closing them automatically.

That way no chance of data loss, and anyone that decides that's annoying
can change the GUI Settings to (a): close the editors.

And perhaps the GUI Settings could include a note about the various
implications of (a), (b) and (c).

With all these possibilities, it's again why I'd like to make that
a separate RFE/STR, just so this STR gets closed and the feature made
available to those using svn current.
 
 
#39 AlbrechtS
12:27 Jul 15, 2016
Sounds good, please open that other STR and go ahead. TIA.  
 
#40 greg.ercolano
12:54 Jul 17, 2016
Fixed in Subversion repository.

v7b patch applied, closed.

Will open a separate STR for the RFE described in Comments #33-38.
 
 
#41 greg.ercolano
14:55 Jul 18, 2016
Also applied v7b to the fltk-1.3-porting branch as r11818.

Just had to leave out the ide/* patches, as they don't apply
to this branch. No conflicts, just offsets, all checked out.

And "svn add"ed the ExternalCodeEditor*{.cxx,h} files this time ;)
 
 
#42 greg.ercolano
13:28 Aug 16, 2016
Fixed in Subversion repository.  
     

Return to Bugs & Features ]

 
 

Comments are owned by the poster. All other content is copyright 1998-2024 by Bill Spitzak and others. This project is hosted by The FLTK Team. Please report site problems to 'erco@seriss.com'.