release 0.9.9

Compute shader with suffixes support and some small fix

.gitattributes

@@ -1 +1,2 @@
-* text eol=lf
+* text eol=lf
+*.png binary

CHANGELOG.md

@@ -4,6 +4,26 @@ All notable changes to the "vscode-mc-shader" extension will be documented in th
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 
+## [0.9.9]
+
+### Added
+
+- Support for mod world folders, outside the standard world{-1,0,1}.
+- Support for compute shader files ending in \_a to \_z.
+
+### Fixed
+
+- Crash when running with eglot as LSP client.
+- Extension icon client not displaying (encoding issue).
+
+## [0.9.8]
+
+### Fixed
+
+- NVIDIA diagnostics line offset off-by-one due to confusion with erroneous (non-proper) GLSL files resulting in both -1 and -2 offsets appearing to be valid when only the former is.
+- Non-toplevel files being treated as toplevel files when they have .fsh/.vsh/etc and not imported into a valid toplevel file.
+- Fix issue in the depth-first-search iterator when a file is imported twice into another file with a different include in between.
+
 ## [0.9.7]
 
 ### Fixed

README.md

@@ -1,7 +1,7 @@
 # Minecraft GLSL Shaders Language Server
 ## mcshader-lsp
 
-[![Marketplace Version](https://vsmarketplacebadge.apphb.com/version/strum355.vscode-mc-shader.svg)](https://marketplace.visualstudio.com/items?itemName=strum355.vscode-mc-shader) [![Installs](https://vsmarketplacebadge.apphb.com/installs/strum355.vscode-mc-shader.svg)](https://marketplace.visualstudio.com/items?itemName=strum355.vscode-mc-shader)
+[![Marketplace Version](https://img.shields.io/visual-studio-marketplace/v/strum355.vscode-mc-shader.svg)](https://marketplace.visualstudio.com/items?itemName=strum355.vscode-mc-shader) [![Installs](https://img.shields.io/visual-studio-marketplace/i/strum355.vscode-mc-shader.svg)](https://marketplace.visualstudio.com/items?itemName=strum355.vscode-mc-shader)
 [![license](https://img.shields.io/github/license/Strum355/vscode-mc-shader.svg)](https://github.com/Strum355/mcshader-lsp)
 [![Issues](https://img.shields.io/github/issues-raw/Strum355/mcshader-lsp.svg)](https://github.com/Strum355/mcshader-lsp/issues)
 [![Build Status](https://img.shields.io/drone/build/Strum355/mcshader-lsp)](https://cloud.drone.io/Strum355/mcshader-lsp)
@@ -12,7 +12,7 @@ Currently supported editors:
 
 - [Visual Studio Code](https://code.visualstudio.com/) with `vscode-mc-shader`
 
-<img src="https://github.com/Strum355/mcshader-lsp/raw/master/logo.png" width="20%" height="20%">
+<img src="https://github.com/Strum355/mcshader-lsp/raw/rust-rewrite/logo.png" width="20%" height="20%">
 
 ## Features
 

client/src/extension.ts

@@ -25,7 +25,7 @@ export class Extension {
 
   readonly package: {
     version: string
-  } = vscode.extensions.getExtension(this.extensionID)!.packageJSON;
+  } = vscode.extensions.getExtension(this.extensionID)!.packageJSON
 
   public get context(): vscode.ExtensionContext {
     return this.extensionContext

package-lock.json

@@ -1,11 +1,12 @@
 {
   "name": "vscode-mc-shader",
-  "version": "0.9.7",
+  "version": "0.9.8",
   "lockfileVersion": 2,
   "requires": true,
   "packages": {
     "": {
-      "version": "0.9.7",
+      "name": "vscode-mc-shader",
+      "version": "0.9.8",
       "hasInstallScript": true,
       "license": "MIT",
       "devDependencies": {

package.json

@@ -2,11 +2,11 @@
   "name": "vscode-mc-shader",
   "displayName": "Minecraft GLSL Shaders",
   "description": "A Visual Studio Code extension for linting/etc Minecraft GLSL Shaders",
-  "version": "0.9.7",
+  "version": "0.9.9",
   "publisher": "Strum355",
   "author": "Noah Santschi-Cooney (Strum355)",
   "license": "MIT",
-  "icon": "logo-mini.png",
+  "icon": "logo-min.png",
   "repository": {
     "url": "https://github.com/Strum355/mcshader-lsp"
   },

server/Cargo.lock

@@ -656,7 +656,7 @@ dependencies = [
 
 [[package]]
 name = "logging"
-version = "0.9.7"
+version = "0.9.9"
 dependencies = [
  "lazy_static",
  "rand 0.8.5",
@@ -668,7 +668,7 @@ dependencies = [
 
 [[package]]
 name = "logging_macro"
-version = "0.9.7"
+version = "0.9.9"
 dependencies = [
  "quote",
  "syn",
@@ -703,7 +703,7 @@ checksum = "a3e378b66a060d48947b590737b30a1be76706c8dd7b8ba0f2fe3989c68a853f"
 
 [[package]]
 name = "mcshader-lsp"
-version = "0.9.7"
+version = "0.9.9"
 dependencies = [
  "anyhow",
  "fs_extra",

server/logging/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "logging"
-version = "0.9.7"
+version = "0.9.9"
 authors = ["Noah Santschi-Cooney <noah@santschi-cooney.ch>"]
 edition = "2021"
 

server/logging_macro/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "logging_macro"
-version = "0.9.7"
+version = "0.9.9"
 authors = ["Noah Santschi-Cooney <noah@santschi-cooney.ch>"]
 edition = "2021"
 

server/main/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mcshader-lsp"
-version = "0.9.7"
+version = "0.9.9"
 authors = ["Noah Santschi-Cooney <noah@santschi-cooney.ch>"]
 edition = "2021"
 

server/main/src/dfs.rs

@@ -63,25 +63,22 @@ impl<'a> Iterator for Dfs<'a> {
                 touch: 1,
             });
 
-            let mut children: Vec<NodeIndex> = self.graph.child_node_indexes(child).collect();
+            let mut children: Vec<_> = self
+                .graph
+                .get_all_child_positions(child)
+                .collect();
+            children.reverse();
 
             if !children.is_empty() {
-                // sort by line number in parent
-                children.sort_by(|x, y| {
-                    let graph = &self.graph.graph;
-                    let edge1 = graph.edge_weight(graph.find_edge(child, *x).unwrap()).unwrap();
-                    let edge2 = graph.edge_weight(graph.find_edge(child, *y).unwrap()).unwrap();
 
-                    edge2.line.cmp(&edge1.line)
+                let child_indexes: Vec<_> = children.iter().map(|c| c.0).collect();
-                });
+                match self.check_for_cycle(&child_indexes) {
-
-                match self.check_for_cycle(&children) {
                     Ok(_) => {}
                     Err(e) => return Some(Err(e)),
                 };
 
                 for child in children {
-                    self.stack.push(child);
+                    self.stack.push(child.0);
                 }
             } else {
                 self.reset_path_to_branch();
@@ -314,10 +311,10 @@ mod dfs_test {
             //     \ /  \
             //      6 - 7
 
-            assert!(is_cyclic_directed(&graph.graph));
-
             let next = dfs.next().unwrap();
             assert_that!(next, err());
+
+            assert!(is_cyclic_directed(&graph.graph));
         }
         {
             let mut graph = CachedStableGraph::new();

server/main/src/diagnostics_parser.rs

@@ -1,4 +1,4 @@
-use std::{collections::HashMap, lazy::OnceCell, path::Path};
+use std::{collections::HashMap, cell::OnceCell, path::Path};
 
 use regex::Regex;
 use rust_lsp::lsp_types::{Diagnostic, DiagnosticSeverity, Position, Range};
@@ -40,7 +40,7 @@ impl<'a, T: opengl::ShaderValidator + ?Sized> DiagnosticsParser<'a, T> {
     fn get_line_offset(&self) -> u32 {
         *self.line_offset.get_or_init(|| match self.vendor_querier.vendor().as_str() {
             "ATI Technologies" => 0,
-            _ => 2,
+            _ => 1,
         })
     }
 

server/main/src/graph.rs

@@ -59,9 +59,9 @@ impl CachedStableGraph {
         PathBuf::from_str(&self.graph[node]).unwrap()
     }
 
-    /// returns an iterator over all the `IncludePosition`'s between a parent and its child for all the positions
+    /// Returns an iterator over all the `IncludePosition`'s between a parent and its child for all the positions
     /// that the child may be imported into the parent, in order of import.
-    pub fn get_edge_metas(&self, parent: NodeIndex, child: NodeIndex) -> impl Iterator<Item = IncludePosition> + '_ {
+    pub fn get_child_positions(&self, parent: NodeIndex, child: NodeIndex) -> impl Iterator<Item = IncludePosition> + '_ {
         let mut edges = self
             .graph
             .edges(parent)
@@ -77,6 +77,18 @@ impl CachedStableGraph {
         edges.into_iter()
     }
 
+    /// Returns an iterator over all the `(NodeIndex, IncludePosition)` tuples between a node and all its children, in order
+    /// of import.
+    pub fn get_all_child_positions(&self, node: NodeIndex) -> impl Iterator<Item = (NodeIndex, IncludePosition)> + '_ {
+        let mut edges = self.graph.edges(node).map(|edge| {
+            let child = self.graph.edge_endpoints(edge.id()).unwrap().1;
+            (child, self.graph[edge.id()])
+        })
+        .collect::<Vec<_>>();
+        edges.sort_by(|x, y| x.1.line.cmp(&y.1.line));
+        edges.into_iter()
+    }
+
     pub fn add_node(&mut self, name: &Path) -> NodeIndex {
         if let Some(idx) = self.cache.get(name) {
             return *idx;
@@ -99,14 +111,6 @@ impl CachedStableGraph {
             .and_then(|edge| self.graph.remove_edge(edge));
     }
 
-    pub fn child_node_metas(&self, node: NodeIndex) -> impl Iterator<Item = (PathBuf, IncludePosition)> + '_ {
-        self.graph.neighbors(node).map(move |n| {
-            let edge = self.graph.find_edge(node, n).unwrap();
-            let edge_meta = self.graph.edge_weight(edge).unwrap();
-            return (self.reverse_index.get(&n).unwrap().clone(), *edge_meta);
-        })
-    }
-
     pub fn child_node_indexes(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + '_ {
         self.graph.neighbors(node)
     }
@@ -236,9 +240,9 @@ mod graph_test {
         //    / \
         //   1   1
 
-        assert_eq!(2, graph.get_edge_metas(idx0, idx1).count());
+        assert_eq!(2, graph.get_child_positions(idx0, idx1).count());
 
-        let mut edge_metas = graph.get_edge_metas(idx0, idx1);
+        let mut edge_metas = graph.get_child_positions(idx0, idx1);
         assert_eq!(Some(IncludePosition { line: 2, start: 0, end: 0 }), edge_metas.next());
         assert_eq!(Some(IncludePosition { line: 4, start: 0, end: 0 }), edge_metas.next());
     }

server/main/src/main.rs

@@ -7,6 +7,7 @@ use rust_lsp::lsp::*;
 use rust_lsp::lsp_types::{notification::*, *};
 
 use petgraph::stable_graph::NodeIndex;
+use path_slash::PathExt;
 
 use serde::Deserialize;
 use serde_json::{from_value, Value};
@@ -58,8 +59,79 @@ mod url_norm;
 #[cfg(test)]
 mod test;
 
+pub fn is_top_level(path: &Path) -> bool {
+    let path = path.to_slash().unwrap();
+    if !RE_WORLD_FOLDER.is_match(&path) {
+        return false;
+    }
+    let parts: Vec<&str> = path.split("/").collect();
+    let len = parts.len();
+    (len == 3 || len == 2) && TOPLEVEL_FILES.contains(parts[len - 1])
+}
+
 lazy_static! {
     static ref RE_INCLUDE: Regex = Regex::new(r#"^(?:\s)*?(?:#include) "(.+)"\r?"#).unwrap();
+    static ref RE_WORLD_FOLDER: Regex = Regex::new(r#"^shaders(/world-?\d+)?"#).unwrap();
+    static ref TOPLEVEL_FILES: HashSet<String> = {
+        let mut set = HashSet::with_capacity(1716);
+        for ext in ["fsh", "vsh", "gsh", "csh"] {
+            set.insert(format!("composite.{}", ext));
+            set.insert(format!("deferred.{}", ext));
+            set.insert(format!("prepare.{}", ext));
+            set.insert(format!("shadowcomp.{}", ext));
+            for i in 1..=99 {
+                set.insert(format!("composite{}.{}", i, ext));
+                set.insert(format!("deferred{}.{}", i, ext));
+                set.insert(format!("prepare{}.{}", i, ext));
+                set.insert(format!("shadowcomp{}.{}", i, ext));
+            }
+            set.insert(format!("composite_pre.{}", ext));
+            set.insert(format!("deferred_pre.{}", ext));
+            set.insert(format!("final.{}", ext));
+            set.insert(format!("gbuffers_armor_glint.{}", ext));
+            set.insert(format!("gbuffers_basic.{}", ext));
+            set.insert(format!("gbuffers_beaconbeam.{}", ext));
+            set.insert(format!("gbuffers_block.{}", ext));
+            set.insert(format!("gbuffers_clouds.{}", ext));
+            set.insert(format!("gbuffers_damagedblock.{}", ext));
+            set.insert(format!("gbuffers_entities.{}", ext));
+            set.insert(format!("gbuffers_entities_glowing.{}", ext));
+            set.insert(format!("gbuffers_hand.{}", ext));
+            set.insert(format!("gbuffers_hand_water.{}", ext));
+            set.insert(format!("gbuffers_item.{}", ext));
+            set.insert(format!("gbuffers_line.{}", ext));
+            set.insert(format!("gbuffers_skybasic.{}", ext));
+            set.insert(format!("gbuffers_skytextured.{}", ext));
+            set.insert(format!("gbuffers_spidereyes.{}", ext));
+            set.insert(format!("gbuffers_terrain.{}", ext));
+            set.insert(format!("gbuffers_terrain_cutout.{}", ext));
+            set.insert(format!("gbuffers_terrain_cutout_mip.{}", ext));
+            set.insert(format!("gbuffers_terrain_solid.{}", ext));
+            set.insert(format!("gbuffers_textured.{}", ext));
+            set.insert(format!("gbuffers_textured_lit.{}", ext));
+            set.insert(format!("gbuffers_water.{}", ext));
+            set.insert(format!("gbuffers_weather.{}", ext));
+            set.insert(format!("shadow.{}", ext));
+            set.insert(format!("shadow_cutout.{}", ext));
+            set.insert(format!("shadow_solid.{}", ext));
+        }
+        let base_char_num = 'a' as u8;
+        for suffix_num in 0u8..=25u8 {
+            let suffix_char = (base_char_num + suffix_num) as char;
+            set.insert(format!("composite_{}.csh", suffix_char));
+            set.insert(format!("deferred_{}.csh", suffix_char));
+            set.insert(format!("prepare_{}.csh", suffix_char));
+            set.insert(format!("shadowcomp_{}.csh", suffix_char));
+            for i in 1..=99 {
+                let total_suffix = format!("{}_{}", i, suffix_char);
+                set.insert(format!("composite{}.csh", total_suffix));
+                set.insert(format!("deferred{}.csh", total_suffix));
+                set.insert(format!("prepare{}.csh", total_suffix));
+                set.insert(format!("shadowcomp{}.csh", total_suffix));
+            }
+        }
+        set
+    };
 }
 
 fn main() {
@@ -156,7 +228,7 @@ impl MinecraftShaderLanguageServer {
         }
     }
 
-    pub fn gen_initial_graph(&self) {
+    fn build_initial_graph(&self) {
         info!("generating graph for current root"; "root" => self.root.to_str().unwrap());
 
         // filter directories and files not ending in any of the 3 extensions
@@ -251,7 +323,9 @@ impl MinecraftShaderLanguageServer {
             Some(n) => n,
         };
 
-        let prev_children: HashSet<_> = HashSet::from_iter(self.graph.borrow().child_node_metas(idx));
+        let prev_children: HashSet<_> = HashSet::from_iter(self.graph.borrow().get_all_child_positions(idx).map(|tup| {
+            (self.graph.borrow().get_node(tup.0), tup.1)
+        }));
         let new_children: HashSet<_> = includes.iter().cloned().collect();
 
         let to_be_added = new_children.difference(&prev_children);
@@ -339,6 +413,13 @@ impl MinecraftShaderLanguageServer {
                     return Ok(diagnostics);
                 }
             };
+
+            if !is_top_level(root_path.strip_prefix(&self.root).unwrap()) {
+                warn!("got a non-valid toplevel file"; "root_ancestor" => root_path.to_str().unwrap(), "stripped" => root_path.strip_prefix(&self.root).unwrap().to_str().unwrap());
+                back_fill(&all_sources, &mut diagnostics);
+                return Ok(diagnostics);
+            }
+
             let tree_type = if ext == "fsh" {
                 TreeType::Fragment
             } else if ext == "vsh" {
@@ -348,12 +429,7 @@ impl MinecraftShaderLanguageServer {
             } else if ext == "csh" {
                 TreeType::Compute
             } else {
-                warn!(
+                unreachable!();
-                    "got a non fsh|vsh|gsh|csh as a file root ancestor, skipping lint";
-                    "extension" => ext, "root_ancestor" => root_path.to_str().unwrap()
-                );
-                back_fill(&all_sources, &mut diagnostics);
-                return Ok(diagnostics);
             };
 
             let stdout = match self.compile_shader_source(&view, tree_type, &root_path) {
@@ -385,6 +461,12 @@ impl MinecraftShaderLanguageServer {
                     Some(ext) => ext.to_str().unwrap(),
                     None => continue,
                 };
+
+                if !is_top_level(root_path.strip_prefix(&self.root).unwrap()) {
+                    warn!("got a non-valid toplevel file"; "root_ancestor" => root_path.to_str().unwrap(), "stripped" => root_path.strip_prefix(&self.root).unwrap().to_str().unwrap());
+                    continue;
+                }
+
                 let tree_type = if ext == "fsh" {
                     TreeType::Fragment
                 } else if ext == "vsh" {
@@ -394,11 +476,7 @@ impl MinecraftShaderLanguageServer {
                 } else if ext == "csh" {
                     TreeType::Compute
                 } else {
-                    warn!(
+                    unreachable!();
-                        "got a non fsh|vsh|gsh|csh as a file root ancestor, skipping lint";
-                        "extension" => ext, "root_ancestor" => root_path.to_str().unwrap()
-                    );
-                    continue;
                 };
 
                 let sources = self.load_sources(&nodes)?;
@@ -564,7 +642,8 @@ impl LanguageServerHandling for MinecraftShaderLanguageServer {
 
             self.root = root;
 
-            self.gen_initial_graph();
+
+            self.build_initial_graph();
 
             self.set_status("ready", "Project initialized", "$(check)");
         });
@@ -587,14 +666,16 @@ impl LanguageServerHandling for MinecraftShaderLanguageServer {
                 log_level: String,
             }
 
-            let config: Configuration = from_value(params.settings.as_object().unwrap().get("mcglsl").unwrap().to_owned()).unwrap();
+            if let Some(settings) = params.settings.as_object().unwrap().get("mcglsl") {
+                let config: Configuration = from_value(settings.to_owned()).unwrap();
 
-            info!("got updated configuration"; "config" => params.settings.as_object().unwrap().get("mcglsl").unwrap().to_string());
+                info!("got updated configuration"; "config" => params.settings.as_object().unwrap().get("mcglsl").unwrap().to_string());
 
-            configuration::handle_log_level_change(config.log_level, |level| {
+                configuration::handle_log_level_change(config.log_level, |level| {
-                self.log_guard = None; // set to None so Drop is invoked
+                    self.log_guard = None; // set to None so Drop is invoked
-                self.log_guard = Some(logging::set_logger_with_level(level));
+                    self.log_guard = Some(logging::set_logger_with_level(level));
-            })
+                })
+            }
         });
     }
 
@@ -825,7 +906,7 @@ impl LanguageServerHandling for MinecraftShaderLanguageServer {
                 .child_node_indexes(node)
                 .filter_map::<Vec<DocumentLink>, _>(|child| {
                     let graph = self.graph.borrow();
-                    graph.get_edge_metas(node, child).map(|value| {
+                    graph.get_child_positions(node, child).map(|value| {
                         let path = graph.get_node(child);
                         let url = match Url::from_file_path(&path) {
                             Ok(url) => url,

server/main/src/merge_views.rs

@@ -8,6 +8,7 @@ use std::{
 use core::slice::Iter;
 
 use petgraph::stable_graph::NodeIndex;
+use slog_scope::debug;
 
 use crate::graph::CachedStableGraph;
 use crate::source_mapper::SourceMapper;
@@ -88,13 +89,7 @@ impl<'a> MergeViewBuilder<'a> {
         // );
 
         // last_offset_set.insert((first, None), version_char_offsets.1);
-        self.last_offset_set.insert(
+        self.set_last_offset_for_tuple(None, first, 0);
-            FilialTuple {
-                child: first,
-                parent: None,
-            },
-            0,
-        );
 
         // stack to keep track of the depth first traversal
         let mut stack = VecDeque::<NodeIndex>::new();
@@ -102,13 +97,8 @@ impl<'a> MergeViewBuilder<'a> {
         self.create_merge_views(&mut merge_list, &mut extra_lines, &mut stack);
 
         // now we add a view of the remainder of the root file
-        let offset = *self
+
-            .last_offset_set
+        let offset = self.get_last_offset_for_tuple(None, first).unwrap();
-            .get(&FilialTuple {
-                child: first,
-                parent: None,
-            })
-            .unwrap();
 
         let len = first_source.len();
         merge_list.push_back(&first_source[min(offset, len)..]);
@@ -135,8 +125,8 @@ impl<'a> MergeViewBuilder<'a> {
                 .parent_child_edge_iterator
                 .entry(*n)
                 .or_insert_with(|| {
-                    let edge_metas = self.graph.get_edge_metas(parent, child);
+                    let child_positions = self.graph.get_child_positions(parent, child);
-                    Box::new(edge_metas)
+                    Box::new(child_positions)
                 })
                 .next()
                 .unwrap();
@@ -147,15 +137,16 @@ impl<'a> MergeViewBuilder<'a> {
             let (char_for_line, char_following_line) = self.char_offset_for_line(edge.line, parent_source);
 
             let offset = *self
-                .last_offset_set
+                .set_last_offset_for_tuple(stack.back().copied(), parent, char_following_line)
-                .insert(
-                    FilialTuple {
-                        child: parent,
-                        parent: stack.back().copied(),
-                    },
-                    char_following_line,
-                )
                 .get_or_insert(0);
+
+            debug!("creating view to start child file";
+                "parent" => parent_path.to_str().unwrap(), "child" => child_path.to_str().unwrap(),
+                "grandparent" => stack.back().copied().map(|g| self.graph.get_node(g).to_str().unwrap().to_string()), // self.graph.get_node().to_str().unwrap(),
+                "last_parent_offset" => offset, "line" => edge.line, "char_for_line" => char_for_line,
+                "char_following_line" => char_following_line,
+            );
+
             merge_list.push_back(&parent_source[offset..char_for_line]);
             self.add_opening_line_directive(&child_path, child, merge_list, extra_lines);
 
@@ -173,13 +164,7 @@ impl<'a> MergeViewBuilder<'a> {
                             }
                         };
                         merge_list.push_back(&child_source[..offset]);
-                        self.last_offset_set.insert(
+                        self.set_last_offset_for_tuple(Some(parent), child, 0);
-                            FilialTuple {
-                                child,
-                                parent: Some(parent),
-                            },
-                            0,
-                        );
                         // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
                         self.add_closing_line_directive(edge.line + 2, &parent_path, parent, merge_list, extra_lines);
                         // if the next pair's parent is not the current pair's parent, we need to bubble up
@@ -193,29 +178,17 @@ impl<'a> MergeViewBuilder<'a> {
                     self.create_merge_views(merge_list, extra_lines, stack);
                     stack.pop_back();
 
-                    let offset = *self
+                    let offset = self.get_last_offset_for_tuple(Some(parent), child).unwrap();
-                        .last_offset_set
-                        .get(&FilialTuple {
-                            child,
-                            parent: Some(parent),
-                        })
-                        .unwrap();
                     let child_source = self.sources.get(&child_path).unwrap();
                     // this evaluates to false once the file contents have been exhausted aka offset = child_source.len() + 1
                     let end_offset = match child_source.ends_with('\n') {
-                        true => 1,  /* child_source.len()-1 */
+                        true => 1,
-                        false => 0, /* child_source.len() */
+                        false => 0,
                     };
                     if offset < child_source.len() - end_offset {
                         // if ends in \n\n, we want to exclude the last \n for some reason. Ask optilad
-                        merge_list.push_back(&child_source[offset../* std::cmp::max( */child_source.len()-end_offset/* , offset) */]);
+                        merge_list.push_back(&child_source[offset..child_source.len() - end_offset]);
-                        self.last_offset_set.insert(
+                        self.set_last_offset_for_tuple(Some(parent), child, 0);
-                            FilialTuple {
-                                child,
-                                parent: Some(parent),
-                            },
-                            0,
-                        );
                     }
 
                     // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
@@ -234,13 +207,7 @@ impl<'a> MergeViewBuilder<'a> {
                         false => child_source.len(),
                     };
                     merge_list.push_back(&child_source[..offset]);
-                    self.last_offset_set.insert(
+                    self.set_last_offset_for_tuple(Some(parent), child, 0);
-                        FilialTuple {
-                            child,
-                            parent: Some(parent),
-                        },
-                        0,
-                    );
                     // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
                     self.add_closing_line_directive(edge.line + 2, &parent_path, parent, merge_list, extra_lines);
                 }
@@ -248,6 +215,18 @@ impl<'a> MergeViewBuilder<'a> {
         }
     }
 
+    fn set_last_offset_for_tuple(&mut self, parent: Option<NodeIndex>, child: NodeIndex, offset: usize) -> Option<usize> {
+        debug!("inserting last offset";
+            "parent" => parent.map(|p| self.graph.get_node(p).to_str().unwrap().to_string()),
+            "child" => self.graph.get_node(child).to_str().unwrap().to_string(),
+            "offset" => offset);
+        self.last_offset_set.insert(FilialTuple { child, parent }, offset)
+    }
+
+    fn get_last_offset_for_tuple(&self, parent: Option<NodeIndex>, child: NodeIndex) -> Option<usize> {
+        self.last_offset_set.get(&FilialTuple { child, parent }).copied()
+    }
+
     // returns the character offset + 1 of the end of line number `line` and the character
     // offset + 1 for the end of the line after the previous one
     fn char_offset_for_line(&self, line_num: usize, source: &str) -> (usize, usize) {