On 14/03/2012 23:17, Marvin Humphrey wrote:
On Wed, Mar 14, 2012 at 1:02 PM, Nick Wellnhofer<[email protected]> wrote:
On 12/03/2012 00:41, Marvin Humphrey wrote:
Personally, I believe that these are all positive developments for the
project, so +1 to install the .cfh files instead of the autogenerated .h
files.
OK, so I would propose to install them in:
catdir ( $ARCH_DIR, 'Clownfish', '_cfh' );
I have a mild preference for naming that directory "_include" as opposed to
"_cfh" for a couple reasons:
1. That dir might wind up containing things other than .cfh files, such as
.h files that have been pound-included by .cfh files (in a literal C
section).
2. The string "cfh" is adequate as a file extension (which has to be short),
but in my opinion "_include" is more self-documenting as a directory name.
OK.
Then Clownfish has to be extended to read files from multiple source
directories. I'd propose to add an array of include directories to the
arguments of CFCHierarchy_init and use them to build the hierarchy.
+1 in principle.
I'd advocate for a slightly different implementation: strip the "source"
argument out of CFCHierarchy_new() and CFCHierarchy_init(), and add two new
functions:
/** Add a directory containing Clownfish source files to compile.
*/
CFCHierarchy_add_source_dir(CFCHierarchy *self, const char *dir);
/** Add an include directory containing interface-defining "header" files.
*/
CFCHierarchy_add_include_dir(CFCHierarchy *self, const char *dir);
This would mean to handle multiple source directories, too. I can't see
an immediate use case, but it shouldn't be too hard to implement once
multiple include dirs are supported.
If you want, feel free to add "include" and "source" labeled parameters to the
Perl binding for new(), since constructors with lots of labeled params are a
common Perl idiom:
my $hierarchy = Clownfish::CFC::Model::Hierarchy->new(
dest => $output_dir, # maybe rename to "output"?
source => \@source_dirs,
include => \@include_dirs,
);
Behind the scenes, though, have new() invoke add_source_dir() and
add_include_dir() before returning.
I have two reasons for making this suggestion:
* C functions with numerous arguments are not self-documenting and are hard
to use.
* It's not possible to map a Perl array to "const char**" automatically.
You typically have to allocate memory and build up the array manually in
the binding, which is messy, annoying, error-prone, and leaks memory when
an exception is thrown.
Yes, that makes sense.
Parts of the code in Lucy::Build can then be reused to build the extension.
This code could be factored out into a module like Clowfish::Build that is
installed together with Clownfish.
I think we probably want to stay out of the Clownfish:: first-level namespace
as much as possible, because that's where all our basic object types are going
to go. I realize I'm being a bit hypocritical here because I dumped CFC into
Clownfish::, but I figured that there would never be a collision.
Ah, good to know. I wondered about that already.
Would "Clownfish::CFC::Build" be appropriate? (We're using that privately
right now, but no big deal to vacate it.)
That's OK with me.
Nick
