Refactor and simplify initial tagspecs (#110)

crates/djls-templates/src/tagspecs.rs

@@ -1,5 +1,5 @@
 use anyhow::Result;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use std::fs;
 use std::path::Path;
@@ -27,38 +27,41 @@ impl TagSpecs {
     }
 
     /// Load specs from a TOML file, looking under the specified table path
-    fn load_from_toml(path: &Path, table_path: &[&str]) -> Result<Self, anyhow::Error> {
+    fn load_from_toml(path: &Path, table_path: &[&str]) -> Result<Self, TagSpecError> {
         let content = fs::read_to_string(path)?;
         let value: Value = toml::from_str(&content)?;
 
-        // Navigate to the specified table
-        let table = table_path
+        let start_node = table_path
             .iter()
-            .try_fold(&value, |current, &key| {
-                current
-                    .get(key)
-                    .ok_or_else(|| anyhow::anyhow!("Missing table: {}", key))
-            })
-            .unwrap_or(&value);
+            .try_fold(&value, |current, &key| current.get(key));
 
         let mut specs = HashMap::new();
-        TagSpec::extract_specs(table, None, &mut specs)
-            .map_err(|e| TagSpecError::Extract(e.to_string()))?;
+
+        if let Some(node) = start_node {
+            let initial_prefix = if table_path.is_empty() {
+                None
+            } else {
+                Some(table_path.join("."))
+            };
+            TagSpec::extract_specs(node, initial_prefix.as_deref(), &mut specs)
+                .map_err(TagSpecError::Extract)?;
+        }
+
         Ok(TagSpecs(specs))
     }
 
     /// Load specs from a user's project directory
     pub fn load_user_specs(project_root: &Path) -> Result<Self, anyhow::Error> {
-        // List of config files to try, in priority order
         let config_files = ["djls.toml", ".djls.toml", "pyproject.toml"];
 
         for &file in &config_files {
             let path = project_root.join(file);
             if path.exists() {
-                return match file {
+                let result = match file {
                     "pyproject.toml" => Self::load_from_toml(&path, &["tool", "djls", "tagspecs"]),
-                    _ => Self::load_from_toml(&path, &["tagspecs"]), // Root level for other files
+                    _ => Self::load_from_toml(&path, &["tagspecs"]),
                 };
+                return result.map_err(anyhow::Error::from);
             }
         }
         Ok(Self::default())
@@ -72,8 +75,8 @@ impl TagSpecs {
         for entry in fs::read_dir(&specs_dir)? {
             let entry = entry?;
             let path = entry.path();
-            if path.extension().and_then(|ext| ext.to_str()) == Some("toml") {
-                let file_specs = Self::load_from_toml(&path, &[])?;
+            if path.is_file() && path.extension().and_then(|ext| ext.to_str()) == Some("toml") {
+                let file_specs = Self::load_from_toml(&path, &["tagspecs"])?;
                 specs.extend(file_specs.0);
             }
         }
@@ -95,80 +98,85 @@ impl TagSpecs {
     }
 }
 
-#[derive(Debug, Clone, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
 pub struct TagSpec {
-    #[serde(rename = "type")]
-    pub tag_type: TagType,
-    pub closing: Option<String>,
+    pub end: Option<EndTag>,
     #[serde(default)]
-    pub branches: Option<Vec<String>>,
-    pub args: Option<Vec<ArgSpec>>,
+    pub intermediates: Option<Vec<String>>,
 }
 
 impl TagSpec {
+    /// Recursive extraction: Check if node is spec, otherwise recurse if table.
     fn extract_specs(
         value: &Value,
-        prefix: Option<&str>,
+        prefix: Option<&str>, // Path *to* this value node
         specs: &mut HashMap<String, TagSpec>,
     ) -> Result<(), String> {
-        // Try to deserialize as a tag spec first
-        match TagSpec::deserialize(value.clone()) {
-            Ok(tag_spec) => {
-                let name = prefix.map_or_else(String::new, |p| {
-                    p.split('.').last().unwrap_or(p).to_string()
-                });
-                specs.insert(name, tag_spec);
+        // Check if the current node *itself* represents a TagSpec definition
+        // We can be more specific: check if it's a table containing 'end' or 'intermediates'
+        let mut is_spec_node = false;
+        if let Some(table) = value.as_table() {
+            if table.contains_key("end") || table.contains_key("intermediates") {
+                // Looks like a spec, try to deserialize
+                match TagSpec::deserialize(value.clone()) {
+                    Ok(tag_spec) => {
+                        // It is a TagSpec. Get name from prefix.
+                        if let Some(p) = prefix {
+                            if let Some(name) = p.split('.').next_back().filter(|s| !s.is_empty()) {
+                                specs.insert(name.to_string(), tag_spec);
+                                is_spec_node = true;
+                            } else {
+                                return Err(format!(
+                                    "Invalid prefix '{}' resulted in empty tag name component.",
+                                    p
+                                ));
+                            }
+                        } else {
+                            return Err("Cannot determine tag name for TagSpec: prefix is None."
+                                .to_string());
+                        }
+                    }
+                    Err(e) => {
+                        // Looked like a spec but failed to deserialize. This is an error.
+                        return Err(format!(
+                            "Failed to deserialize potential TagSpec at prefix '{}': {}",
+                            prefix.unwrap_or("<root>"),
+                            e
+                        ));
+                    }
+                }
             }
-            Err(_) => {
-                // Not a tag spec, try recursing into any table values
-                for (key, value) in value.as_table().iter().flat_map(|t| t.iter()) {
+        }
+
+        // If the node was successfully processed as a spec, DO NOT recurse into its fields.
+        // Otherwise, if it's a table, recurse into its children.
+        if !is_spec_node {
+            if let Some(table) = value.as_table() {
+                for (key, inner_value) in table.iter() {
                     let new_prefix = match prefix {
                         None => key.clone(),
                         Some(p) => format!("{}.{}", p, key),
                     };
-                    Self::extract_specs(value, Some(&new_prefix), specs)?;
+                    Self::extract_specs(inner_value, Some(&new_prefix), specs)?;
                 }
             }
         }
+
         Ok(())
     }
 }
 
-#[derive(Clone, Debug, Deserialize, PartialEq)]
-#[serde(rename_all = "lowercase")]
-pub enum TagType {
-    Container,
-    Inclusion,
-    Single,
-}
-
-#[derive(Clone, Debug, Deserialize)]
-pub struct ArgSpec {
-    pub name: String,
-    pub required: bool,
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
+pub struct EndTag {
+    pub tag: String,
     #[serde(default)]
-    pub allowed_values: Option<Vec<String>>,
-    #[serde(default)]
-    pub is_kwarg: bool,
-}
-
-impl ArgSpec {
-    pub fn is_placeholder(arg: &str) -> bool {
-        arg.starts_with('{') && arg.ends_with('}')
-    }
-
-    pub fn get_placeholder_name(arg: &str) -> Option<&str> {
-        if Self::is_placeholder(arg) {
-            Some(&arg[1..arg.len() - 1])
-        } else {
-            None
-        }
-    }
+    pub optional: bool,
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::fs;
 
     #[test]
     fn test_can_load_builtins() -> Result<(), anyhow::Error> {
@@ -176,6 +184,8 @@ mod tests {
 
         assert!(!specs.0.is_empty(), "Should have loaded at least one spec");
 
+        assert!(specs.get("if").is_some(), "'if' tag should be present");
+
         for name in specs.0.keys() {
             assert!(!name.is_empty(), "Tag name should not be empty");
         }
@@ -190,29 +200,34 @@ mod tests {
             "autoescape",
             "block",
             "comment",
-            "cycle",
-            "debug",
-            "extends",
             "filter",
             "for",
-            "firstof",
             "if",
-            "include",
-            "load",
-            "now",
+            "ifchanged",
             "spaceless",
-            "templatetag",
-            "url",
             "verbatim",
             "with",
+            "cache",
+            "localize",
+            "blocktranslate",
+            "localtime",
+            "timezone",
         ];
         let missing_tags = [
             "csrf_token",
-            "ifchanged",
+            "cycle",
+            "debug",
+            "extends",
+            "firstof",
+            "include",
+            "load",
             "lorem",
+            "now",
             "querystring", // 5.1
             "regroup",
             "resetcycle",
+            "templatetag",
+            "url",
             "widthratio",
         ];
 
@@ -237,33 +252,44 @@ mod tests {
         let root = dir.path();
 
         let pyproject_content = r#"
-[tool.djls.template.tags.mytag]
-type = "container"
-closing = "endmytag"
-branches = ["mybranch"]
-args = [{ name = "myarg", required = true }]
+[tool.djls.tagspecs.mytag]
+end = { tag = "endmytag" }
+intermediates = ["mybranch"]
+
+[tool.djls.tagspecs.anothertag]
+end = { tag = "endanothertag", optional = true }
 "#;
         fs::write(root.join("pyproject.toml"), pyproject_content)?;
 
+        // Load all (built-in + user)
         let specs = TagSpecs::load_all(root)?;
 
-        let if_tag = specs.get("if").expect("if tag should be present");
-        assert_eq!(if_tag.tag_type, TagType::Container);
+        assert!(specs.get("if").is_some(), "'if' tag should be present");
 
         let my_tag = specs.get("mytag").expect("mytag should be present");
-        assert_eq!(my_tag.tag_type, TagType::Container);
-        assert_eq!(my_tag.closing, Some("endmytag".to_string()));
+        assert_eq!(
+            my_tag.end,
+            Some(EndTag {
+                tag: "endmytag".to_string(),
+                optional: false
+            })
+        );
+        assert_eq!(my_tag.intermediates, Some(vec!["mybranch".to_string()]));
 
-        let branches = my_tag
-            .branches
-            .as_ref()
-            .expect("mytag should have branches");
-        assert!(branches.iter().any(|b| b == "mybranch"));
-
-        let args = my_tag.args.as_ref().expect("mytag should have args");
-        let arg = &args[0];
-        assert_eq!(arg.name, "myarg");
-        assert!(arg.required);
+        let another_tag = specs
+            .get("anothertag")
+            .expect("anothertag should be present");
+        assert_eq!(
+            another_tag.end,
+            Some(EndTag {
+                tag: "endanothertag".to_string(),
+                optional: true
+            })
+        );
+        assert!(
+            another_tag.intermediates.is_none(),
+            "anothertag should have no intermediates"
+        );
 
         dir.close()?;
         Ok(())
@@ -274,36 +300,45 @@ args = [{ name = "myarg", required = true }]
         let dir = tempfile::tempdir()?;
         let root = dir.path();
 
+        // djls.toml has higher priority
         let djls_content = r#"
-[mytag1]
-type = "container"
-closing = "endmytag1"
+[tagspecs.mytag1]
+end = { tag = "endmytag1_from_djls" }
 "#;
         fs::write(root.join("djls.toml"), djls_content)?;
 
+        // pyproject.toml has lower priority
         let pyproject_content = r#"
-[tool.djls.template.tags]
-mytag2.type = "container"
-mytag2.closing = "endmytag2"
+[tool.djls.tagspecs.mytag1]
+end = { tag = "endmytag1_from_pyproject" }
+
+[tool.djls.tagspecs.mytag2]
+end = { tag = "endmytag2_from_pyproject" }
 "#;
         fs::write(root.join("pyproject.toml"), pyproject_content)?;
 
         let specs = TagSpecs::load_user_specs(root)?;
 
-        assert!(specs.get("mytag1").is_some(), "mytag1 should be present");
+        let tag1 = specs.get("mytag1").expect("mytag1 should be present");
+        assert_eq!(tag1.end.as_ref().unwrap().tag, "endmytag1_from_djls");
+
+        // Should not find mytag2 because djls.toml was found first
         assert!(
             specs.get("mytag2").is_none(),
-            "mytag2 should not be present"
+            "mytag2 should not be present when djls.toml exists"
         );
 
+        // Remove djls.toml, now pyproject.toml should be used
         fs::remove_file(root.join("djls.toml"))?;
         let specs = TagSpecs::load_user_specs(root)?;
 
+        let tag1 = specs.get("mytag1").expect("mytag1 should be present now");
+        assert_eq!(tag1.end.as_ref().unwrap().tag, "endmytag1_from_pyproject");
+
         assert!(
-            specs.get("mytag1").is_none(),
-            "mytag1 should not be present"
+            specs.get("mytag2").is_some(),
+            "mytag2 should be present when only pyproject.toml exists"
         );
-        assert!(specs.get("mytag2").is_some(), "mytag2 should be present");
 
         dir.close()?;
         Ok(())

crates/djls-templates/tagspecs/README.md

@@ -1,51 +1,28 @@
 # TagSpecs
 
+Tag Specifications (TagSpecs) define how template tags are structured, helping the language server understand template syntax for features like block completion and diagnostics.
+
 ## Schema
 
 Tag Specifications (TagSpecs) define how tags are parsed and understood. They allow the parser to handle custom tags without hard-coding them.
 
 ```toml
-[package.module.path.tag_name]  # Path where tag is registered, e.g., django.template.defaulttags
-type = "container" | "inclusion" | "single"
-closing = "closing_tag_name"        # For block tags that require a closing tag
-branches = ["branch_tag_name", ...] # For block tags that support branches
-
-# Arguments can be positional (matched by order) or keyword (matched by name)
-args = [
-    # Positional argument (position inferred from array index)
-    { name = "setting", required = true, allowed_values = ["on", "off"] },
-    # Keyword argument
-    { name = "key", required = false, is_kwarg = true }
-]
+[path.to.tag_name]  # Path where tag is registered, e.g., django.template.defaulttags
+end = { tag = "end_tag_name", optional = false } # Optional: Defines the closing tag
+intermediates = ["intermediate_tag_name", ...]   # Optional: Defines intermediate tags (like else, elif)
 ```
 
-The `name` field in args should match the internal name used in Django's node implementation. For example, the `autoescape` tag's argument is stored as `setting` in Django's `AutoEscapeControlNode`.
+The `end` table defines the closing tag for a block tag.
+- `tag`: The name of the closing tag (e.g., "endif").
+- `optional`: Whether the closing tag is optional (defaults to `false`).
 
-## Tag Types
+The `intermediates` array lists tags that can appear between the opening and closing tags (e.g., "else", "elif" for an "if" tag).
 
-- `container`: Tags that wrap content and require a closing tag
-
-  ```django
-  {% if condition %}content{% endif %}
-  {% for item in items %}content{% endfor %}
-  ```
-
-- `inclusion`: Tags that include or extend templates.
-
-  ```django
-  {% extends "base.html" %}
-  {% include "partial.html" %}
-  ```
-
-- `single`: Single tags that don't wrap content
-
-  ```django
-  {% csrf_token %}
-  ```
+The tag name itself (e.g., `if`, `for`, `my_custom_tag`) is derived from the last segment of the TOML table path defining the spec.
 
 ## Configuration
 
-- **Built-in TagSpecs**: The parser includes TagSpecs for Django's built-in tags and popular third-party tags.
+- **Built-in TagSpecs**: The parser includes TagSpecs for Django's built-in tags and popular third-party tags. These are provided by `djls-templates` automatically; users do not need to define them. The examples below show the format, but you only need to create files for your *own* custom tags or to override built-in behavior.
 - **User-defined TagSpecs**: Users can expand or override TagSpecs via `pyproject.toml` or `djls.toml` files in their project, allowing custom tags and configurations to be seamlessly integrated.
 
 ## Examples
@@ -53,37 +30,30 @@ The `name` field in args should match the internal name used in Django's node im
 ### If Tag
 
 ```toml
-[django.template.defaulttags.if]
-type = "container"
-closing = "endif"
-branches = ["elif", "else"]
-args = [{ name = "condition", required = true }]
+[tagspecs.django.template.defaulttags.if]
+end = { tag = "endif" }
+intermediates = ["elif", "else"]
 ```
 
-### Include Tag
+### For Tag
 
 ```toml
-[django.template.defaulttags.includes]
-type = "inclusion"
-args = [{ name = "template_name", required = true }]
+[tagspecs.django.template.defaulttags.for]
+end = { tag = "endfor" }
+intermediates = ["empty"]
 ```
 
 ### Autoescape Tag
 
 ```toml
-[django.template.defaulttags.autoescape]
-type = "container"
-closing = "endautoescape"
-args = [{ name = "setting", required = true, allowed_values = ["on", "off"] }]
+[tagspecs.django.template.defaulttags.autoescape]
+end = { tag = "endautoescape" }
 ```
 
-### Custom Tag with Kwargs
+### Custom Tag
 
 ```toml
-[my_module.templatetags.my_tags.my_custom_tag]
-type = "single"
-args = [
-    { name = "arg1", required = true },
-    { name = "kwarg1", required = false, is_kwarg = true }
-]
+[tagspecs.my_module.templatetags.my_tags.my_custom_tag]
+end = { tag = "endmycustomtag", optional = true }
+intermediates = ["myintermediate"]
 ```

crates/djls-templates/tagspecs/django.toml

@@ -1,104 +1,48 @@
-[django.template.defaulttags.autoescape]
-args = [{ name = "setting", required = true, allowed_values = ["on", "off"] }]
-closing = "endautoescape"
-type = "container"
+[tagspecs.django.template.defaulttags.autoescape]
+end = { tag = "endautoescape" }
 
-[django.template.defaulttags.block]
-closing = "endblock"
-type = "container"
+[tagspecs.django.template.defaulttags.block]
+end = { tag = "endblock" }
 
-[django.template.defaulttags.comment]
-closing = "endcomment"
-type = "container"
+[tagspecs.django.template.defaulttags.comment]
+end = { tag = "endcomment" }
 
-[django.template.defaulttags.cycle]
-args = [
-  { name = "cyclevars", required = true },
-  { name = "variable_name", required = false, is_kwarg = true },
-]
-type = "single"
+[tagspecs.django.template.defaulttags.filter]
+end = { tag = "endfilter" }
 
-[django.template.defaulttags.debug]
-type = "single"
+[tagspecs.django.template.defaulttags.for]
+end = { tag = "endfor" }
+intermediates = [ "empty" ]
 
-[django.template.defaulttags.extends]
-args = [{ name = "parent_name", required = true }]
-type = "inclusion"
+[tagspecs.django.template.defaulttags.if]
+end = { tag = "endif" }
+intermediates = [ "elif", "else" ]
 
-[django.template.defaulttags.for]
-args = [
-  { name = "{item}", required = true },
-  { name = "in", required = true },
-  { name = "{iterable}", required = true },
-]
-branches = ["empty"]
-closing = "endfor"
-type = "container"
+[tagspecs.django.template.defaulttags.ifchanged]
+end = { tag = "endifchanged" }
+intermediates = [ "else" ]
 
-[django.template.defaulttags.filter]
-args = [{ name = "filter_expr", required = true }]
-closing = "endfilter"
-type = "container"
+[tagspecs.django.template.defaulttags.spaceless]
+end = { tag = "endspaceless" }
 
-[django.template.defaulttags.firstof]
-args = [{ name = "variables", required = true }]
-type = "single"
+[tagspecs.django.template.defaulttags.verbatim]
+end = { tag = "endverbatim" }
 
-[django.template.defaulttags.if]
-args = [{ name = "condition", required = true }]
-branches = ["elif", "else"]
-closing = "endif"
-type = "container"
+[tagspecs.django.template.defaulttags.with]
+end = { tag = "endwith" }
 
-[django.template.defaulttags.include]
-args = [
-{ name = "template", required = true },
-{ name = "with", required = false, is_kwarg = true },
-{ name = "only", required = false, is_kwarg = true },
-]
-type = "inclusion"
+[tagspecs.django.templatetags.cache.cache]
+end = { tag = "endcache" }
 
-[django.template.defaulttags.load]
-args = [{ name = "library", required = true }]
-type = "single"
+[tagspecs.django.templatetags.i10n.localize]
+end = { tag = "endlocalize" }
 
-[django.template.defaulttags.now]
-args = [{ name = "format_string", required = true }]
-type = "single"
+[tagspecs.django.templatetags.i18n.blocktranslate]
+end = { tag = "endblocktranslate" }
+intermediates = [ "plural" ]
 
-[django.template.defaulttags.spaceless]
-closing = "endspaceless"
-type = "container"
+[tagspecs.django.templatetags.tz.localtime]
+end = { tag = "endlocaltime" }
 
-[django.template.defaulttags.templatetag]
-type = "single"
-
-[[django.template.defaulttags.templatetag.args]]
-allowed_values = [
-  "openblock",
-  "closeblock",
-  "openvariable",
-  "closevariable",
-  "openbrace",
-  "closebrace",
-  "opencomment",
-  "closecomment",
-]
-name = "tagtype"
-required = true
-
-[django.template.defaulttags.url]
-args = [
-{ name = "view_name", required = true },
-{ name = "asvar", required = false, is_kwarg = true },
-]
-type = "single"
-
-[django.template.defaulttags.verbatim]
-closing = "endverbatim"
-type = "container"
-
-[django.template.defaulttags.with]
-args = [{ name = "extra_context", required = true }]
-closing = "endwith"
-type = "container"
+[tagspecs.django.templatetags.tz.timezone]
+end = { tag = "endtimezone" }