STR #3136: Patch to add an option to the Fl_Native_file_Chooser and Fl_File_Chooser to allow selection of either a file or a directory

STR #3136

Application:	FLTK Library
Status:	5 - New
Priority:	1 - Request for Enhancement, e.g. asking for a feature
Scope:	2 - Specific to an operating system
Subsystem:	Core Library
Summary:	Patch to add an option to the Fl_Native_file_Chooser and Fl_File_Chooser to allow selection of either a file or a directory
Version:	1.3-feature
Created By:	Kennyyy
Assigned To:	Unassigned
Fix Version:	Unassigned
Update Notification:	Receive EMails Don't Receive EMails

Trouble Report Files:

Post File

#1	Kennyyy 17:20 Sep 21, 2014

File_Chooser_Or_Directory.patch
6k

Trouble Report Comments:

Post Text

#1	Kennyyy 17:20 Sep 21, 2014

The patch allow the user to select either a file or a directory (or both in case of FLTK_MULTI_FILE type) in the same file chooser. The implementation affects FL_File_Chooser and FL_Native_File_Chooser but is only effective when the FLTK file chooser is called (i.e. only on Linux). Vincent

#2	AlbrechtS 04:35 Sep 24, 2014

Thanks for the patch. I'm not so much involved in file chooser issues, maybe someone else can comment on that (and I seem to recall that this has been discussed in fltk.general or somewhere else). However, two comments from me: (1) Adding the new member variable 'int options_;' to class Fl_File_Chooser breaks the ABI (this can be done with our ABI macros or in FLTK 1.4 or higher) (2) Removal of '#include' of system headers is being discussed in fltk.coredev. We should probably remove all three system headers instead of only stdio.h. Question to OP: why did you remove '#include <stdio.h>' in your patch? Was there a specific reason, maybe incompatibility, or just because it was not needed?

#3	Kennyyy 05:26 Sep 28, 2014

Hi Albrecht, yes this have been discussed on fltk.general (discussion is here: https://groups.google.com/d/topic/fltkgeneral/YhdpxJeTwbc/discussion). Regarding your remarks: (1) Well I guess you are right. I originally wanted to add the new "option" as a type() of FL_CHOOSER which would not have required adding the options_ variable. But, as suggested by Greg, since the "option" is supported only on some platform (and here, only on linux), it's more correct to set it as an option (as opposed to a type) since these ones could be platform dependent (and inefficient on some platforms). I don't know if there is a better trade of (between option and type). (2) For the stdio removal, I did it at the time I removed the ones I actually added myself in other files for development purpose. I did notice that this one was useless as well and so remove it as well. No incompatibility at all. I also remember having read something in a thread about some useless stdio inclusion that could get read off, so I let the removal in my patch. But this patch is certainly the place to remove this useless header. Sorry about that. For the other system headers, if I remember well, at least some of them are actually needed.

#4	AlbrechtS 07:37 Sep 28, 2014

Thanks for the feedback. Re (1): There's nothing wrong with the option_ approach, it's just that we need to take care not to break the ABI. We can handle it though with ABI macros [1], if we want. Re (2): We decided in fltk.coredev that we don't want to remove the system header #include's in fltk 1.3.x, so this should not be done in this patch. You may wait for more comments or post an updated patch to help the developer who will do it to test and apply it. An additional small, compileable test program that can be used to verify the patch might speed up its application... [1] use this to protect your new code from breaking the ABI (in this case this may result in NOT compiling the entire code): #if FLTK_ABI_VERSION >= 10303 ... ABI breaking code here ... #endif

#5	ianmacarthur 02:05 Oct 03, 2014

As discussed on the list, it doens't look like we can make this fly consistently on all platforms right now. In the meantime, I think it would be interesting to try and float this as an "add-on" widget, so that users who needed this functionality, and were on a platform where it is available, could have access to it. Thoughts?