Re: babel perl issue

[Eric Schulte <schulte.eric@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

Achim Gratz <Stromeko@nexgo.de> writes:

> Eric Schulte writes:
>> I'm attaching two patches which implement this new require structure.
>> They move ob.el -> ob-core.el.  The new ob.el (which is now loaded by
>> every file which requires 'ob) does two things.
>
> That looks OK so far, just one nit: it would be much cleaner from the
> perspective of Git if you first renamed ob.el -> ob-core.el and then
> patched it.  That way git blame would continue to work across that
> commit in a mostly sensible manner.  Right now all lines in those two
> files are new things that sprang into existence with this commit as far
> as Git is concerned.
>

I will make this change (in fact I had done this originally, but then
didn't want to send too many patches so I compressed the commit
history).

>
>> 1. It defines every Babel defcustom (excluding the language-specific
>>    defcustoms).  Should defvars be moved here as well?
>
> Not necessarily, but if it cleans up the code, this would be worth
> doing.  Maybe as a second step, since none of it should be broken right
> now?
>

In fact I think this would make the code less readable, so I won't move
around any defvars.

>
>> 2. It loads the remainder of Babel, namely; ob-eval, ob-core, ob-comint,
>>    ob-exp, ob-keys, ob-table, ob-lob, ob-ref and ob-tangle.
>
> Which codepaths would directly activate ob-comint for instance and why
> is it appropriate to just (require 'ob-core) in this case?
>

Each language file will only need to (require 'ob), only a few of the
core Babel files would actually require ob-core.  Here's a diagram of
the require structure.

    # shell-script
    (echo "digraph {";
      for file in lisp/ob{,-eval,-core,-keys,-table,-ref,-comint,-exp,-lob,-tangle}.el;do
        name=$(basename $file .el)
        echo "\"$name\";"
        rgrep "require 'ob" $file|sed "s/^.*'/\"/;s/)/\" -> \"$name\"\;/;"
      done;echo "}";
    )|dot -Tpng > /tmp/babel-requires.png


[-- Attachment #2: babel-requires.png --]
[-- Type: image/png, Size: 251741 bytes --]

[-- Attachment #3: Type: text/plain, Size: 379 bytes --]


>
>> This allows for most of the language files to be simplified as they now
>> only need to require ob, rather than requiring the subset of the above
>> particular to their needs.
>
> Looks nicer to me, anyway.
>

Me too, I'll apply this after Bastien gets a chance to look it over.

Thanks for your help,

>
>
> Regards,
> Achim.

-- 
Eric Schulte
http://cs.unm.edu/~eschulte