Minor refactoring of include checks

5 files changed, 62 insertions, 48 deletions

diff --git a/src/MainWindow.h b/src/MainWindow.h
index bd32bdd..1dcffeb 100644
--- a/src/MainWindow.h
+++ b/src/MainWindow.h
@@ -76,7 +76,6 @@ private:
 	void refreshDocument();
 	void updateTemporalVariables();
 	bool fileChangedOnDisk();
-	bool includesChanged();
 	bool compileTopLevelDocument(bool reload);
 	bool compile(bool reload, bool procevents);
 	void compileCSG(bool procevents);
diff --git a/src/ModuleCache.cc b/src/ModuleCache.cc
index 0e443e8..83f7b7d 100644
--- a/src/ModuleCache.cc
+++ b/src/ModuleCache.cc
@@ -38,6 +38,9 @@ FileModule *ModuleCache::evaluate(const std::string &filename)
 	memset(&st, 0, sizeof(struct stat));
 	bool valid = (stat(filename.c_str(), &st) == 0);
 
+  // If file isn't there, just return and let the cache retain the old module
+	if (!valid) return NULL;
+	
 	std::string cache_id = str(boost::format("%x.%x") % st.st_mtime % st.st_size);
 
 	// Lookup in cache
@@ -45,13 +48,10 @@ FileModule *ModuleCache::evaluate(const std::string &filename)
 		lib_mod = &(*this->entries[filename].module);
 		if (this->entries[filename].cache_id == cache_id) {
 			shouldCompile = false;
-			
-			BOOST_FOREACH(const FileModule::IncludeContainer::value_type &include, lib_mod->includes) {
-				if (lib_mod->include_modified(include.second)) {
-					lib_mod = NULL;
-					shouldCompile = true;
-					break;
-				}
+
+			if (lib_mod->includesChanged()) {
+				lib_mod = NULL;
+				shouldCompile = true;
 			}
 		}
 	}
diff --git a/src/mainwin.cc b/src/mainwin.cc
index db161d7..7099053 100644
--- a/src/mainwin.cc
+++ b/src/mainwin.cc
@@ -1044,16 +1044,6 @@ bool MainWindow::fileChangedOnDisk()
 	return false;
 }
 
-bool MainWindow::includesChanged()
-{
-	if (this->root_module) {
-		BOOST_FOREACH(const FileModule::IncludeContainer::value_type &item, this->root_module->includes) {
-			if (this->root_module->include_modified(item.second)) return true;
-		}
-	}
-	return false;
-}
-
 /*!
 	If reload is true, does a timestamp check on the document and tries to reload it.
 	Otherwise, just reparses the current document and any dependencies, updates the 
@@ -1065,7 +1055,9 @@ bool MainWindow::compileTopLevelDocument(bool reload)
 {
 	bool shouldcompiletoplevel = !reload;
 
-	if (includesChanged()) shouldcompiletoplevel = true;
+	if (this->root_module && this->root_module->includesChanged()) {
+		shouldcompiletoplevel = true;
+	}
 
 	if (reload) {	
 		// Refresh files if it has changed on disk
diff --git a/src/module.cc b/src/module.cc
index aa2fad3..046d0c4 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -226,6 +226,28 @@ void FileModule::registerInclude(const std::string &localpath,
 	this->includes[localpath] = inc;
 }
 
+bool FileModule::includesChanged() const
+{
+	BOOST_FOREACH(const FileModule::IncludeContainer::value_type &item, this->includes) {
+		if (include_modified(item.second)) return true;
+	}
+	return false;
+}
+
+bool FileModule::include_modified(const IncludeFile &inc) const
+{
+	struct stat st;
+	memset(&st, 0, sizeof(struct stat));
+
+	fs::path fullpath = find_valid_path(this->path, inc.filename);
+	bool valid = !fullpath.empty() ? (stat(boosty::stringy(fullpath).c_str(), &st) == 0) : false;
+	
+	if (valid && !inc.valid) return true; // Detect appearance of file but not removal
+	if (valid && st.st_mtime > inc.mtime) return true;
+	
+	return false;
+}
+
 /*!
 	Check if any dependencies have been modified and recompile them.
 	Returns true if anything was recompiled.
@@ -236,12 +258,13 @@ bool FileModule::handleDependencies()
 	this->is_handling_dependencies = true;
 
 	bool changed = false;
-	// Iterating manually since we want to modify the container while iterating
-
 
 	// If a lib in usedlibs was previously missing, we need to relocate it
 	// by searching the applicable paths. We can identify a previously missing module
 	// as it will have a relative path.
+
+	// Iterating manually since we want to modify the container while iterating
+	std::vector<std::pair<std::string, FileModule*> > modified_modules;
 	FileModule::ModuleContainer::iterator iter = this->usedlibs.begin();
 	while (iter != this->usedlibs.end()) {
 		FileModule::ModuleContainer::iterator curr = iter++;
@@ -256,17 +279,28 @@ bool FileModule::handleDependencies()
 			if (!fullpath.empty()) filename = boosty::stringy(fullpath);
 		}
 
-		curr->second = ModuleCache::instance()->evaluate(filename);
-		if (curr->second != oldmodule) {
+		FileModule *newmodule = ModuleCache::instance()->evaluate(filename);
+		// Detect appearance but not removal of files
+		if (newmodule && oldmodule != newmodule) {
 			changed = true;
 #ifdef DEBUG
-			PRINTB_NOCACHE("  %s: %p", filename % curr->second);
+			PRINTB_NOCACHE("  %s: %p", filename % newmodule);
 #endif
 		}
-		if (!curr->second && !wasmissing) {
-			PRINTB_NOCACHE("WARNING: Failed to compile library '%s'.", filename);
+		if (newmodule) {
+			modified_modules.push_back(std::make_pair(filename, newmodule));
+			this->usedlibs.erase(curr);
+		}
+		else {
+			// Only print warning if we're not part of an automatic reload
+			if (!oldmodule && !wasmissing) {
+				PRINTB_NOCACHE("WARNING: Failed to compile library '%s'.", filename);
+			}
 		}
 	}
+	BOOST_FOREACH(const FileModule::ModuleContainer::value_type &mod, modified_modules) {
+		this->usedlibs[mod.first] = mod.second;
+	}
 
 	this->is_handling_dependencies = false;
 	return changed;
@@ -288,17 +322,3 @@ AbstractNode *FileModule::instantiate(const Context *ctx, const ModuleInstantiat
 
 	return node;
 }
-
-bool FileModule::include_modified(IncludeFile inc)
-{
-	struct stat st;
-	memset(&st, 0, sizeof(struct stat));
-
-	fs::path fullpath = find_valid_path(this->path, inc.filename);
-	bool valid = !fullpath.empty() ? (stat(boosty::stringy(fullpath).c_str(), &st) == 0) : false;
-	
-	if (valid != inc.valid) return true;
-	if (valid && st.st_mtime > inc.mtime) return true;
-	
-	return false;
-}
diff --git a/src/module.h b/src/module.h
index 96e5649..e28677a 100644
--- a/src/module.h
+++ b/src/module.h
@@ -78,12 +78,6 @@ public:
 	LocalScope scope;
 };
 
-struct IncludeFile {
-	std::string filename;
-	bool valid;
-	time_t mtime;
-};
-
 // FIXME: A FileModule doesn't have definition arguments, so we shouldn't really
 // inherit from a Module
 class FileModule : public Module
@@ -95,15 +89,24 @@ public:
 	void setModulePath(const std::string &path) { this->path = path; }
 	const std::string &modulePath() const { return this->path; }
 	void registerInclude(const std::string &localpath, const std::string &fullpath);
+	bool includesChanged() const;
 	bool handleDependencies();
 	virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx = NULL) const;
+	bool hasIncludes() const { return !this->includes.empty(); }
+	bool usesLibraries() const { return !this->usedlibs.empty(); }
 
 	typedef boost::unordered_map<std::string, class FileModule*> ModuleContainer;
 	ModuleContainer usedlibs;
+private:
+	struct IncludeFile {
+		std::string filename;
+		bool valid;
+		time_t mtime;
+	};
+
+	bool include_modified(const IncludeFile &inc) const;
 	typedef boost::unordered_map<std::string, struct IncludeFile> IncludeContainer;
 	IncludeContainer includes;
-	bool include_modified(struct IncludeFile inc);
-private:
 	bool is_handling_dependencies;
 	std::string path;
 };