Achim Gratz 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