Merge pull request #1509 from mgreter/bugfix/importer-c-api-path-handling

Improve importer C-API and path handling

Author: mgreter

include/sass_functions.h

@@ -57,7 +57,7 @@ ADDAPI void ADDCALL sass_delete_importer (Sass_Importer_Entry cb);
 ADDAPI Sass_Import_List ADDCALL sass_make_import_list (size_t length);
 // Creator for a single import entry returned by the custom importer inside the list
 ADDAPI Sass_Import_Entry ADDCALL sass_make_import_entry (const char* path, char* source, char* srcmap);
-ADDAPI Sass_Import_Entry ADDCALL sass_make_import (const char* path, const char* base, char* source, char* srcmap);
+ADDAPI Sass_Import_Entry ADDCALL sass_make_import (const char* imp_path, const char* abs_base, char* source, char* srcmap);
 // set error message to abort import and to print out a message (path from existing object is used in output)
 ADDAPI Sass_Import_Entry ADDCALL sass_import_set_error(Sass_Import_Entry import, const char* message, size_t line, size_t col);
 
@@ -67,8 +67,8 @@ ADDAPI void ADDCALL sass_import_set_list_entry (Sass_Import_List list, size_t id
 ADDAPI Sass_Import_Entry ADDCALL sass_import_get_list_entry (Sass_Import_List list, size_t idx);
 
 // Getters for import entry
-ADDAPI const char* ADDCALL sass_import_get_path (Sass_Import_Entry);
-ADDAPI const char* ADDCALL sass_import_get_base (Sass_Import_Entry);
+ADDAPI const char* ADDCALL sass_import_get_imp_path (Sass_Import_Entry);
+ADDAPI const char* ADDCALL sass_import_get_abs_path (Sass_Import_Entry);
 ADDAPI const char* ADDCALL sass_import_get_source (Sass_Import_Entry);
 ADDAPI const char* ADDCALL sass_import_get_srcmap (Sass_Import_Entry);
 // Explicit functions to take ownership of these items

src/ast.cpp

@@ -1924,6 +1924,7 @@ namespace Sass {
     // some final cosmetics
     if (res == "-0.0") res.erase(0, 1);
     else if (res == "-0") res.erase(0, 1);
+    else if (res == "") res = "0";
 
     // add unit now
     res += unit();

src/context.cpp

@@ -411,15 +411,17 @@ namespace Sass {
   }
 
 
-  std::vector<std::string> Context::get_included_files(size_t skip)
+  // for data context we want to start after "stdin"
+  // we probably always want to skip the header includes?
+  std::vector<std::string> Context::get_included_files(bool skip, size_t headers)
   {
+      // create a copy of the vector for manupulations
       std::vector<std::string> includes = included_files;
       if (includes.size() == 0) return includes;
-      std::sort( includes.begin() + skip, includes.end() );
-      includes.erase( includes.begin(), includes.begin() + skip );
+      if (skip) { includes.erase( includes.begin(), includes.begin() + 1 + headers); }
+      else { includes.erase( includes.begin() + 1, includes.begin() + 1 + headers); }
       includes.erase( std::unique( includes.begin(), includes.end() ), includes.end() );
-      // the skip solution seems more robust, as we may have real files named stdin
-      // includes.erase( std::remove( includes.begin(), includes.end(), "stdin" ), includes.end() );
+      std::sort( includes.begin() + (skip ? 0 : 1), includes.end() );
       return includes;
   }
 

src/context.hpp

@@ -110,8 +110,8 @@ namespace Sass {
     Block* parse_string();
     void add_source(std::string, std::string, char*);
 
-    std::string add_file(const std::string& file);
-    std::string add_file(const std::string& base, const std::string& file, ParserState pstate);
+    std::string add_file(const std::string& imp_path);
+    std::string add_file(const std::string& imp_path, const std::string& abs_path, ParserState pstate);
 
 
     // allow to optionally overwrite the input path
@@ -122,7 +122,7 @@ namespace Sass {
     char* compile_block(Block* root);
     char* generate_source_map();
 
-    std::vector<std::string> get_included_files(size_t skip = 0);
+    std::vector<std::string> get_included_files(bool skip = false, size_t headers = 0);
 
   private:
     void collect_plugin_paths(const char* paths_str);

src/functions.cpp

@@ -1575,7 +1575,7 @@ namespace Sass {
 
       Arguments* args = SASS_MEMORY_NEW(ctx.mem, Arguments, pstate);
       std::string full_name(name + "[f]");
-      Definition* def = static_cast<Definition*>((d_env)[full_name]);
+      Definition* def = d_env.has(full_name) ? static_cast<Definition*>((d_env)[full_name]) : 0;
       Parameters* params = def ? def->parameters() : 0;
       size_t param_size = params ? params->length() : 0;
       for (size_t i = 0, L = arglist->length(); i < L; ++i) {

src/parser.cpp

@@ -329,18 +329,25 @@ namespace Sass {
 
   bool Parser::do_import(const std::string& import_path, Import* imp, std::vector<Sass_Importer_Entry> importers, bool only_one)
   {
+    size_t i = 0;
     bool has_import = false;
     std::string load_path = unquote(import_path);
-    for (auto importer : importers) {
+    for (Sass_Importer_Entry& importer : importers) {
       // int priority = sass_importer_get_priority(importer);
       Sass_Importer_Fn fn = sass_importer_get_function(importer);
       if (Sass_Import_List includes =
           fn(load_path.c_str(), importer, ctx.c_compiler)
       ) {
         Sass_Import_List list = includes;
-        while (*includes) {
+        while (*includes) { ++i;
+          std::string uniq_path = load_path;
+          if (!only_one && i) {
+            std::stringstream pathstrm;
+            pathstrm << uniq_path << ":" << i;
+            uniq_path = pathstrm.str();
+          }
           Sass_Import_Entry include = *includes;
-          const char *file = sass_import_get_path(include);
+          const char *abs_path = sass_import_get_abs_path(include);
           char* source = sass_import_take_source(include);
           size_t line = sass_import_get_error_line(include);
           size_t column = sass_import_get_error_column(include);
@@ -349,15 +356,15 @@ namespace Sass {
             if (line == std::string::npos && column == std::string::npos) error(message, pstate);
             else error(message, ParserState(message, source, Position(line, column)));
           } else if (source) {
-            if (file) {
-              ctx.add_source(load_path, file, source);
-              imp->files().push_back(load_path);
+            if (abs_path) {
+              ctx.add_source(uniq_path, abs_path, source);
+              imp->files().push_back(uniq_path);
             } else {
-              ctx.add_source(load_path, load_path, source);
-              imp->files().push_back(load_path);
+              ctx.add_source(uniq_path, uniq_path, source);
+              imp->files().push_back(uniq_path);
             }
-          } else if(file) {
-            import_single_file(imp, file);
+          } else if(abs_path) {
+            import_single_file(imp, abs_path);
           }
           ++includes;
         }

src/sass_context.cpp

@@ -511,21 +511,24 @@ extern "C" {
 
       // maybe skip some entries of included files
       // we do not include stdin for data contexts
-      size_t skip = 0;
+      bool skip = false;
 
       // dispatch to the correct render function
       if (c_ctx->type == SASS_CONTEXT_FILE) {
         root = cpp_ctx->parse_file();
       } else if (c_ctx->type == SASS_CONTEXT_DATA) {
         root = cpp_ctx->parse_string();
-        skip = 1; // skip first entry of includes
+        skip = true; // skip first entry of includes
       }
 
-      // skip all prefixed files?
-      skip += cpp_ctx->head_imports;
+      // skip all prefixed files? (ToDo: check srcmap)
+      // IMO source-maps should point to headers already
+      // therefore don't skip it for now. re-enable or
+      // remove completely once this is tested
+      size_t headers = cpp_ctx->head_imports;
 
       // copy the included files on to the context (dont forget to free)
-      if (root) copy_strings(cpp_ctx->get_included_files(skip), &c_ctx->included_files);
+      if (root) copy_strings(cpp_ctx->get_included_files(skip, headers), &c_ctx->included_files);
 
       // return parsed block
       return root;

src/sass_functions.cpp

@@ -38,8 +38,8 @@ extern "C" {
 
   // External import entry
   struct Sass_Import {
-    char* path;
-    char* base;
+    char* imp_path; // path as found in the import statement
+    char *abs_path; // path after importer has resolved it
     char* source;
     char* srcmap;
     // error handling
@@ -92,12 +92,12 @@ extern "C" {
 
   // Creator for a single import entry returned by the custom importer inside the list
   // We take ownership of the memory for source and srcmap (freed when context is destroyd)
-  Sass_Import_Entry ADDCALL sass_make_import(const char* path, const char* base, char* source, char* srcmap)
+  Sass_Import_Entry ADDCALL sass_make_import(const char* imp_path, const char* abs_path, char* source, char* srcmap)
   {
     Sass_Import* v = (Sass_Import*) calloc(1, sizeof(Sass_Import));
     if (v == 0) return 0;
-    v->path = path ? sass_strdup(path) : 0;
-    v->base = base ? sass_strdup(base) : 0;
+    v->imp_path = imp_path ? sass_strdup(imp_path) : 0;
+    v->abs_path = abs_path ? sass_strdup(abs_path) : 0;
     v->source = source;
     v->srcmap = srcmap;
     v->error = 0;
@@ -142,17 +142,17 @@ extern "C" {
   // Just in case we have some stray import structs
   void ADDCALL sass_delete_import(Sass_Import_Entry import)
   {
-    free(import->path);
-    free(import->base);
+    free(import->imp_path);
+    free(import->abs_path);
     free(import->source);
     free(import->srcmap);
     free(import->error);
     free(import);
   }
 
   // Getter for import entry
-  const char* ADDCALL sass_import_get_path(Sass_Import_Entry entry) { return entry->path; }
-  const char* ADDCALL sass_import_get_base(Sass_Import_Entry entry) { return entry->base; }
+  const char* ADDCALL sass_import_get_imp_path(Sass_Import_Entry entry) { return entry->imp_path; }
+  const char* ADDCALL sass_import_get_abs_path(Sass_Import_Entry entry) { return entry->abs_path; }
   const char* ADDCALL sass_import_get_source(Sass_Import_Entry entry) { return entry->source; }
   const char* ADDCALL sass_import_get_srcmap(Sass_Import_Entry entry) { return entry->srcmap; }
 

src/sass_interface.cpp

@@ -135,7 +135,7 @@ extern "C" {
       c_ctx->error_message = 0;
       c_ctx->error_status = 0;
 
-      copy_strings(cpp_ctx.get_included_files(1), &c_ctx->included_files, 1);
+      copy_strings(cpp_ctx.get_included_files(true), &c_ctx->included_files, 1);
     }
     catch (Error_Invalid& e) {
       std::stringstream msg_stream;
@@ -227,7 +227,7 @@ extern "C" {
       c_ctx->error_message = 0;
       c_ctx->error_status = 0;
 
-      copy_strings(cpp_ctx.get_included_files(), &c_ctx->included_files);
+      copy_strings(cpp_ctx.get_included_files(false), &c_ctx->included_files);
     }
     catch (Error_Invalid& e) {
       std::stringstream msg_stream;