refactor: Address code review feedback for Scriban HTML formatter

- Fix CustomTemplateLoader to avoid using internal Scriban API
- Replace Console.WriteLine with proper exception handling
- Remove duplicate PrepareConfigurationData method
- Improve error handling in GetCssStyles() and GetJavaScriptCode()
- Use Debug.WriteLine for non-critical warnings instead of empty catch blocks

These changes improve code robustness, eliminate code duplication, and follow better error handling practices.

Author: jbrinkman

src/DotNetApiDiff/Reporting/CustomTemplateLoader.cs

@@ -5,38 +5,25 @@
 namespace DotNetApiDiff.Reporting;
 
 /// <summary>
-/// Custom template loader for Scriban that loads templates from a dictionary
+/// Custom template loader for Scriban that loads templates from embedded resources
 /// </summary>
 public class CustomTemplateLoader : ITemplateLoader
 {
-    private readonly Dictionary<string, Template> _templates;
-
-    public CustomTemplateLoader(Dictionary<string, Template> templates)
-    {
-        _templates = templates;
-    }
-
     public string GetPath(TemplateContext context, SourceSpan callerSpan, string templateName)
     {
         return templateName;
     }
 
     public string Load(TemplateContext context, SourceSpan callerSpan, string templatePath)
     {
-        if (_templates.TryGetValue(templatePath, out var template))
-        {
-            // Get the source code of the template
-            return template.Page.Body.ToString();
-        }
-
-        // Try loading from embedded resources directly
         try
         {
+            // Load template source directly from embedded resources
             return EmbeddedTemplateLoader.LoadTemplate($"{templatePath}.scriban");
         }
-        catch
+        catch (Exception ex)
         {
-            throw new InvalidOperationException($"Template '{templatePath}' not found");
+            throw new InvalidOperationException($"Template '{templatePath}' not found: {ex.Message}", ex);
         }
     }
 

src/DotNetApiDiff/Reporting/HtmlFormatterScriban.cs

@@ -35,10 +35,7 @@ public HtmlFormatterScriban()
         }
         catch (Exception ex)
         {
-            // Fallback to basic template if templates can't be loaded
-            Console.WriteLine($"Warning: Could not load templates, using fallback: {ex.Message}");
-            _mainTemplate = Template.Parse(GetFallbackTemplate());
-            _partialTemplates = new Dictionary<string, Template>();
+            throw new InvalidOperationException("Failed to initialize Scriban HTML formatter templates. Ensure template resources are properly embedded.", ex);
         }
     }
 
@@ -72,7 +69,7 @@ public string Format(ComparisonResult result)
         context.PushGlobal(scriptObject);
 
         // Set up template loader for includes
-        context.TemplateLoader = new CustomTemplateLoader(_partialTemplates);
+        context.TemplateLoader = new CustomTemplateLoader();
 
         return _mainTemplate.Render(context);
     }
@@ -137,7 +134,7 @@ private object PrepareResultData(ComparisonResult result)
                 modified_count = result.Summary.ModifiedCount,
                 breaking_changes_count = result.Summary.BreakingChangesCount
             },
-            configuration = PrepareConfigurationData(result.Configuration),
+            configuration = PrepareConfigData(result.Configuration),
             breaking_changes = PrepareBreakingChangesData(result.Differences.Where(d => d.IsBreakingChange))
         };
     }
@@ -251,46 +248,6 @@ private object[] PrepareBreakingChangesData(IEnumerable<ApiDifference> breakingC
             }).ToArray();
     }
 
-    private object PrepareConfigurationData(DotNetApiDiff.Models.Configuration.ComparisonConfiguration config)
-    {
-        return new
-        {
-            filters = new
-            {
-                include_internals = config.Filters.IncludeInternals,
-                include_compiler_generated = config.Filters.IncludeCompilerGenerated,
-                include_namespaces = config.Filters.IncludeNamespaces,
-                exclude_namespaces = config.Filters.ExcludeNamespaces,
-                include_types = config.Filters.IncludeTypes,
-                exclude_types = config.Filters.ExcludeTypes
-            },
-            mappings = new
-            {
-                auto_map_same_name_types = config.Mappings.AutoMapSameNameTypes,
-                ignore_case = config.Mappings.IgnoreCase,
-                type_mappings = config.Mappings.TypeMappings.Select(kvp => new { key = kvp.Key, value = kvp.Value }),
-                namespace_mappings = config.Mappings.NamespaceMappings.Select(kvp => new { key = kvp.Key, value = kvp.Value })
-            },
-            exclusions = new
-            {
-                excluded_types = config.Exclusions.ExcludedTypes,
-                excluded_members = config.Exclusions.ExcludedMembers,
-                excluded_type_patterns = config.Exclusions.ExcludedTypePatterns,
-                excluded_member_patterns = config.Exclusions.ExcludedMemberPatterns
-            },
-            breaking_change_rules = new
-            {
-                treat_type_removal_as_breaking = config.BreakingChangeRules.TreatTypeRemovalAsBreaking,
-                treat_member_removal_as_breaking = config.BreakingChangeRules.TreatMemberRemovalAsBreaking,
-                treat_signature_change_as_breaking = config.BreakingChangeRules.TreatSignatureChangeAsBreaking,
-                treat_reduced_accessibility_as_breaking = config.BreakingChangeRules.TreatReducedAccessibilityAsBreaking
-            },
-            output_format = config.OutputFormat,
-            fail_on_breaking_changes = config.FailOnBreakingChanges,
-            output_path = config.OutputPath
-        };
-    }
-
     private string RenderPartial(string templateName, object data)
     {
         if (_partialTemplates.TryGetValue(templateName, out var template))
@@ -313,8 +270,9 @@ private string GetCssStyles()
         {
             return EmbeddedTemplateLoader.LoadStyles();
         }
-        catch
+        catch (Exception ex)
         {
+            System.Diagnostics.Debug.WriteLine($"Warning: Could not load CSS styles, using fallback: {ex.Message}");
             return GetFallbackStyles();
         }
     }
@@ -325,8 +283,9 @@ private string GetJavaScriptCode()
         {
             return EmbeddedTemplateLoader.LoadScripts();
         }
-        catch
+        catch (Exception ex)
         {
+            System.Diagnostics.Debug.WriteLine($"Warning: Could not load JavaScript, using fallback: {ex.Message}");
             return GetFallbackJavaScript();
         }
     }