Re: [quickbook] Direct code import...

First, I need to say I'm not forcing this on anyone. I put in the svn 
repo as: a) I wanted to have it available someplace other than my one 
machine; b) I thought it might be something to spur conversations as to 
what literate programming support might look like. Hence I wasn't 
expecting it to go much further in its current form.

Note, below I haven't actually looked at the dev branch code to see hwo 
that works.. So my answers may be totally bogus. I.e. mostly I'm 
answering from the context of the trunk quickbook version. So please 
correct my ignorance as you see fit ;-)

On 10/29/2011 7:34 AM, Daniel James wrote:
> On 29 October 2011 03:48, Rene Rivera<[email protected]>  wrote:
>>
>> [import ../../../boost/predef/version_number.h]
>> [import ../../../boost/predef/architecture/.*\.h]
>
> I still don't like this much, but since it looks like this will be
> forced on me, here's a review:
>
> It would have been better if you'd branched from quickbook-dev, as
> there are some changes to import/include there that are very relevant.

OK. I was going under the plan that after the release you would merge 
that dev branch to trunk. And then I would work from that state. Mostly 
because it's hard enough to keep track of two quickbooks. I'd hate to 
add a third for myself. And I was in a hurry to start documenting, but 
not in a hurry to see this integrated into the main-line quickbook.

> There's the conflicting include source file thing that I implemented
> there but it should be pretty easy to adapt to your purposes. If you
> try building the example I pointed you to, or even just looked at
> 'gold' test output, it should be obvious what it does.

OK. I guess I missed, or forgot about, that pointing :-) I'll look again 
at the previous posts.

> I'd rather use 'include' than 'import', the idea being that 'include'
> is for including content, 'import' is for importing templates and
> macros. This should mean that 'include' wouldn't import the other
> snippets in the file. With your change it isn't clear what an 'import'
> does without looking at the imported file.

Well, there's at least one problem with using "include".. It would mean 
duplicating the file parsing that "import" already does. And if we 
restructure so that the grammar can be used, it would then complicate 
the "import" also since it has to now also ignore the escape comment 
parts that "include" is using. Which I guess means that both the file 
inclusion and file import should be combined into a comprehensive file 
reading and parsing mechanic. And then based on the template name doing 
the parsing we either do import like things, or we do include like things.

> 'quickbook-dev' has a 'call_code_snippet' function which is a lot
> easier to use than 'do_template_action'.

Didn't seem that hard to use do_template_action. Although I did spend 
longer than I expected trying to figure out the combination of values it 
wanted :-\ But I guess any modularity in this context is a good thing.

> I'd rather use explicit syntax to distinguish between paths and
> patterns. This change breaks the windows path support, which I think

Technically it doesn't break it. As long as you don't use a regex for 
the file it behaves as before.

> means that it can no longer be used to build some older versions of
> the maths documentation (I want to phase out support for windows
> paths, but try to keep quickbook backwards compatible, that's why I
> just added a warning for now). I also plan on implementing escapes for
> import, include, xinclude etc. eventually (1.7), which might interact
> badly with this, requiring double escapes, while an explicit syntax
> would indicate that escapes need to be treated differently.

OK. But would a more general concept of typed value 
quoting/escaping/parsing be better?

> I'd also use a version switch as well. I think things like this should
> be approached with some caution.

How would a version switch actually work here. I can only see it working 
with linear development. What I have is a branch.

> I'd rather use globs than regular expressions, it isn't that hard to
> implement a glob matcher (I think I might have one I wrote somewhere)
> and I find it nicer for matching paths. Would also not require adding
> an extra dependency.

I would rather not use globs. For various reasons: they are limited and 
likely people will want more control; writing a bunch of custom code for 
something that already exists seems foolish to me; regex are very well 
understood and documented whereas globs tend to be special to whomever 
implements it.

> I haven't tested it, but I don't think your code will work on windows.
> For example, the line:
>
>      std::string f = i->path().filename().native();
>
> should probably be something like:
>
>      std::string f = detail::path_to_generic(i->path().filename());

That would be true if 'f' was a full path. But it's only the file name 
part which can't have the characters that path_to_generic manipulates. 
Additionally this code is never executed in a backward compatibility 
context since it's only when one specifies a regex as the argument to 
the import.

> I think it'd be better to return 'std::set<include_search_return>'
> than turn 'include_search_return' into a stack. It took me a little
> while to work out what you were doing. Actually, I'd probably go for a
> vector and just sort it. I admit that 'include_search_return' is a
> really bad name, I wouldn't be against renaming it.

I was going for the minimal amount of code mucking about. Hence the 
double duty of the structure. If I was implementing this more generally 
I'd try to provide a more appropriate abstraction to file searching and 
handling.

> If there are matches at different points in the search path, they're
> ordered by absolute path which may vary on different computers. I
> think my preference would be to keep them in the order they appear in
> the search path, with 'filename_relative' as a secondary sort field in
> case of collisions. Or perhaps 'filename_relative' could be the
> primary sort field, and the include path order the secondary sort
> field.

Good point. Although the ordering within different search paths is 
likely to never come up. As it's more likely that if one is concerned 
about some specific ordering one would put the imports/inclusions in a 
specific order.. As I did in my use case.

> If you expand the '!' snippets in a separate loop after creating the
> other snippets, I think it would allow to you to expand code snippets
> from later in the file which should be useful.

Hm... Very good idea. That would allow for some very convenient document 
writing. As in allowing one to write a concise explanation of code at 
the top with code included from the actual program. Much like the 
example you point to below. But in a much improved organization as it 
doesn't reorder the code itself and hence not make the code flow less 
transparent.

But your idea is contradictory to your previous point about using 
"include" instead of "import". The above would be, according to your 
definition of the semantics, a combined import and include. Which is 
what I implemented. So perhaps we want to reconsider the semantics (and 
names) of "include" and "import".

> I wouldn't call this literate programming, as I think it annoys
> literate programming enthusiasts. See:
>
> http://www.perl.com/pub/tchrist/litprog.html

I guess it depends on whether one considers literate programming a 
general concept of having documentation and code interleaved vs. about 
some particular tool that implements the ideas ;-)


-- 
-- Grafik - Don't Assume Anything
-- Redshift Software, Inc. - http://redshift-software.com
-- rrivera/acm.org (msn) - grafik/redshift-software.com
-- 102708583/icq - grafikrobot/aim,yahoo,skype,efnet,gmail