Fix a couple of edge and node filtering bugs (#56)

* Fix FocusPackEdgeDirection::None not properly handled for show edge calculation

* More compact tests for .show_edge_builder

* Fix bug where dependencies would cause non-focus packs to be shown despite the fact that the edges would not be shown

* Make readable calculations for filters for dependents, dependencies, todos out, and todos

* Nicer switch between show edge on/off and in/out/inout/none/all!?

* Show todo types at the end of title

* Rename packs used in .filtered tests

* Fix in/out directionality bug

* Table-based tests for filtering

* No need to make empty focus_pack array a special case

* Test partial node input lists

* Added some more specs

* Add visual test for edge focus mode “in"

* Trim title length

* Updated cassettes

* Bump gem version

* Fix digram title tests

Author: shageman

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    visualize_packs (0.5.19)
+    visualize_packs (0.5.20)
       packs-specification
       parse_packwerk (>= 0.20.0)
       sorbet-runtime

diagram_examples.png

@@ -70,25 +70,31 @@ def self.code_owner(package)
   def self.diagram_title(options, max_todo_count)
     return "<<b>#{options.title}</b>>" if options.title
 
-    app_name = File.basename(Dir.pwd)
-    focus_edge_info = options.focus_pack && options.show_only_edges_to_focus_pack != FocusPackEdgeDirection::All ? "showing only edges to/from focus pack" : "showing all edges between visible packs"
-    focus_info = options.focus_pack ? "Focus on #{limited_sentence(options.focus_pack)} (#{focus_edge_info})" : "All packs"
-    skipped_info = 
-    [
-      options.show_legend ? nil : "hiding legend",
-      options.show_layers ? nil : "hiding layers",
-      options.show_dependencies ? nil : "hiding dependencies",
-      options.show_todos ? nil : "hiding todos",
-      EdgeTodoTypes.values.size == options.only_todo_types.size ? nil : "only #{limited_sentence(options.only_todo_types.map &:serialize)} todos",
-      options.show_privacy ? nil : "hiding privacy",
-      options.show_teams ? nil : "hiding teams",
-      options.show_visibility ? nil : "hiding visibility",
-      options.roll_nested_into_parent_packs ? "hiding nested packs" : nil,
-      options.show_nested_relationships ? nil : "hiding nested relationships",
-      options.exclude_packs.empty? ? nil : "excluding pack#{options.exclude_packs.size > 1 ? 's' : ''}: #{limited_sentence(options.exclude_packs)}",
+    focus_info = if options.focus_pack
+      "Focus on #{limited_sentence(options.focus_pack)} (Edge mode: #{options.show_only_edges_to_focus_pack.serialize})"
+    else 
+      "All packs"
+    end
+
+    hidden_aspects = [
+      options.show_legend ? nil : "legend",
+      options.show_layers ? nil : "layers",
+      options.show_dependencies ? nil : "dependencies",
+      options.show_todos ? nil : "todos",
+      options.show_privacy ? nil : "privacy",
+      options.show_teams ? nil : "teams",
+      options.show_visibility ? nil : "visibility",
+      options.roll_nested_into_parent_packs ? "nested packs" : nil,
+      options.show_nested_relationships ? nil : "nested relationships",
     ].compact.join(', ').strip
-    main_title = "#{app_name}: #{focus_info}#{skipped_info != '' ? ' - ' + skipped_info : ''}"
-    sub_title = ""
+    hidden_aspects_title = hidden_aspects != '' ? "Hiding #{hidden_aspects}" : nil
+
+    todo_types = EdgeTodoTypes.values.size == options.only_todo_types.size ? nil : "Only #{options.only_todo_types.map &:serialize} todos",
+
+    exclusions = options.exclude_packs.empty? ? nil : "Excluding pack#{options.exclude_packs.size > 1 ? 's' : ''}: #{limited_sentence(options.exclude_packs)}",
+
+    main_title = [focus_info, hidden_aspects_title, todo_types, exclusions].compact.join('. ')
+
     if options.show_todos && max_todo_count
       sub_title = "<br/><font point-size='12'>Widest todo edge is #{max_todo_count} todo#{max_todo_count > 1 ? 's' : ''}</font>"
     end
@@ -115,6 +121,8 @@ def self.show_edge_builder(options, all_package_names)
         case options.show_only_edges_to_focus_pack
         when FocusPackEdgeDirection::All then
           true
+        when FocusPackEdgeDirection::None then
+          match_packs?(start_node, options.focus_pack) && match_packs?(end_node, options.focus_pack)
         when FocusPackEdgeDirection::InOut then
           match_packs?(start_node, options.focus_pack) || match_packs?(end_node, options.focus_pack)
         when FocusPackEdgeDirection::In then
@@ -200,36 +208,32 @@ def self.filtered(packages, options)
     result = T.let([], T::Array[T.nilable(String)])
     result = packages.map { |pack| pack.name }
 
-    if focus_pack && !focus_pack.empty?
+    if focus_pack
       result = []
-      result += packages.map { |pack| pack.name }.select { |p| match_packs?(p, focus_pack) }
-      if options.show_dependencies
-        result += packages.select { |p| p.dependencies.any? { |d| match_packs?(d, focus_pack) }}.map { |pack| pack.name }
-      end
-      if options.show_todos && [FocusPackEdgeDirection::All, FocusPackEdgeDirection::In, FocusPackEdgeDirection::InOut].include?(options.show_only_edges_to_focus_pack)
-        result += packages.select do
-          |p| (p.violations || []).inject([]) do |res, todo|
-            res << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
-            res
-          end.any? { |v| match_packs?(v, focus_pack) }
-        end.map { |pack| pack.name }
+      focus_pack_name = packages.map { |pack| pack.name }.select { |p| match_packs?(p, focus_pack) }
+      result += focus_pack_name
+
+      dependents = options.show_dependencies ? dependents_on(packages, focus_pack_name) : []
+      dependencies = options.show_dependencies ? dependencies_of(packages, focus_pack_name) : []
+      todos_out = options.show_todos ? todos_out(packages, focus_pack_name, options) : []
+      todos_in = options.show_todos ? todos_in(packages, focus_pack_name, options) : []
+
+      case options.show_only_edges_to_focus_pack
+      when FocusPackEdgeDirection::All, FocusPackEdgeDirection::InOut then
+        result += dependents + dependencies + todos_out + todos_in
+      when FocusPackEdgeDirection::In then
+        result += dependents + todos_in
+      when FocusPackEdgeDirection::Out then
+        result += dependencies + todos_out 
+      when FocusPackEdgeDirection::None then
+        # nothing to do
       end
-      packages.map { |pack| pack.name }.select { |p| match_packs?(p, focus_pack) }.each do |p|
-        if options.show_dependencies
-          result += packages_by_name[p].dependencies
-        end
-        if options.show_todos && [FocusPackEdgeDirection::All, FocusPackEdgeDirection::Out, FocusPackEdgeDirection::InOut].include?(options.show_only_edges_to_focus_pack)
-          result += (packages_by_name[p].violations || []).inject([]) do |res, todo|
-            res << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
-            res
-          end
-        end
-      end
-      result = result.uniq
+
       parent_packs = result.inject([]) do |res, package_name|
         res << nested_packages[package_name]
         res
       end
+
       result = (result + parent_packs).uniq.compact
     end
 
@@ -318,4 +322,39 @@ def self.remove_nested_packs(packages, options)
   def self.match_packs?(pack, packs_name_with_wildcards)
     !packs_name_with_wildcards || packs_name_with_wildcards.any? {|p| File.fnmatch(p, pack)}
   end
+
+  sig { params(all_packages: T::Array[ParsePackwerk::Package], focus_packs_names: T::Array[String]).returns(T::Array[String]) }
+  def self.dependencies_of(all_packages, focus_packs_names)
+    focus_packs = all_packages.select { focus_packs_names.include?(_1.name)}
+    
+    focus_packs.inject([]) do |result, pack|
+      result += pack.dependencies
+      result
+    end.uniq
+  end
+
+  sig { params(all_packages: T::Array[ParsePackwerk::Package], focus_packs_names: T::Array[String]).returns(T::Array[String]) }
+  def self.dependents_on(all_packages, focus_packs_names)
+    all_packages.select { |pack| pack.dependencies.any? { focus_packs_names.include?(_1) }}.map &:name
+  end
+
+  sig { params(all_packages: T::Array[ParsePackwerk::Package], focus_packs_names: T::Array[String], options: Options).returns(T::Array[String]) }
+  def self.todos_in(all_packages, focus_packs_names, options)
+    all_packages.select do |p|
+      (p.violations || []).inject([]) do |res, todo|
+        res << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
+        res
+      end.any? { |v| focus_packs_names.include?(v) }
+    end.map { |pack| pack.name }
+  end
+
+  sig { params(all_packages: T::Array[ParsePackwerk::Package], focus_packs_names: T::Array[String], options: Options).returns(T::Array[String]) }
+  def self.todos_out(all_packages, focus_packs_names, options)
+    all_packages.inject([]) do |result, p|
+      focus_packs_names.include?(p.name) && (p.violations || []).each do |todo|
+        result << todo.to_package_name if options.only_todo_types.include?(EdgeTodoTypes.deserialize(todo.type))
+      end
+      result
+    end
+  end
 end

lib/visualize_packs.rb

@@ -31,6 +31,7 @@ test_names+=("only_layers"); test_params[${test_names[-1]}]="--no-dependency-arr
 test_names+=("no_to_all"); test_params[${test_names[-1]}]="--title='Hide everything' --no-layers --no-visibility --no-dependency-arrows --no-todo-arrows --no-privacy-boxes --no-teams --no-nesting-arrows"
 test_names+=("focussed_on_packs_ui"); test_params[${test_names[-1]}]="--focus-pack=packs/ui"
 test_names+=("focussed_on_packs_ui_focus_edges"); test_params[${test_names[-1]}]="--focus-pack=packs/ui --focus-pack-edge-mode=inout"
+test_names+=("focussed_on_packs_ui_focus_edges_in"); test_params[${test_names[-1]}]="--focus-pack=packs/ui --focus-pack-edge-mode=in"
 
 # Debugging...
 # echo "test_names: ${test_names[@]}"

spec/diagram_generation_helper.sh

@@ -1,7 +1,7 @@
 PATH
   remote: ../..
   specs:
-    visualize_packs (0.5.19)
+    visualize_packs (0.5.20)
       packs-specification
       parse_packwerk (>= 0.20.0)
       sorbet-runtime

spec/sample_app1/Gemfile.lock

@@ -5,7 +5,7 @@ digraph package_diagram {
     fontname="Helvetica,Arial,sans-serif"
     dpi=100
     layout=dot
-    label=<<b>sample_app1: Focus on packs/ui (showing all edges between visible packs)</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
+    label=<<b>Focus on packs/ui (Edge mode: all)</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
     fontsize=18
   ]
   node [

spec/sample_app1/test_output/focussed_on_packs_ui.dot

@@ -5,7 +5,7 @@ digraph package_diagram {
     fontname="Helvetica,Arial,sans-serif"
     dpi=100
     layout=dot
-    label=<<b>sample_app1: Focus on packs/ui (showing only edges to/from focus pack)</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
+    label=<<b>Focus on packs/ui (Edge mode: inout)</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
     fontsize=18
   ]
   node [

spec/sample_app1/test_output/focussed_on_packs_ui_focus_edges.dot

@@ -0,0 +1,99 @@
+digraph package_diagram {
+  rankdir=TD
+  graph [
+    labelloc="t"
+    fontname="Helvetica,Arial,sans-serif"
+    dpi=100
+    layout=dot
+    label=<<b>Focus on packs/ui (Edge mode: in)</b>>
+    fontsize=18
+  ]
+  node [
+    fontname="Helvetica,Arial,sans-serif"
+    fontsize=26.0
+    fontcolor=black
+    fillcolor=white
+    color=black
+    height=1.0
+    style=filled
+    shape=plain
+  ]
+    subgraph app {
+      shape=box
+      color=darkgrey
+      fillcolor=lightblue
+      style=filled
+        label="app"
+        cluster=true
+        rank = 0        "packs/ui" [
+            fontsize=18.0
+              URL="https://github.com/rubyatscale/visualize_packwerk/tree/main/spec/sample_app/packs/ui"
+
+              style=filled
+              fillcolor="#FFA9C5"
+            label= <
+                <table border='0' cellborder='1' cellspacing='0' cellpadding='4'>
+                  <tr> <td align='left'> packs/ui </td> </tr>
+                </table>
+              >
+          ]
+        
+    }
+    subgraph utilities {
+      shape=box
+      color=darkgrey
+      fillcolor=lightblue
+      style=filled
+        label="utilities"
+        cluster=true
+        rank = 1    }
+    subgraph NotInLayer {
+      shape=box
+      color=darkgrey
+      fillcolor=lightblue
+      style=filled
+        cluster=false
+    }
+    subgraph cluster_legend {
+      fontsize=16
+      label="Edges Styles and Arrow Heads"
+      A [ fontsize=12 shape=box label="package"]
+      B [ fontsize=12 shape=box label="package"]
+      A -> B [label="accepted dependency" color=darkgreen]
+      K [ fontsize=12 shape=box label="package"]
+      L [ fontsize=12 shape=box label="package"]
+      K -> L [label="nested package" color=purple]
+      M [ fontsize=12 shape=box label="package"]
+      N [ fontsize=12 shape=box label="package"]
+      M -> N [label="visibile to" color=blue]
+      C [ fontsize=12 shape=box label="package"]
+      D [ fontsize=12 shape=box label="package"]
+      C -> D [label="privacy todo" color=darkred style=dashed arrowhead=crow]
+      E [ fontsize=12 shape=box label="package"]
+      F [ fontsize=12 shape=box label="package"]
+      E -> F [label="architecture todo" color=darkred style=dashed arrowhead=obox]
+      G [ fontsize=12 shape=box label="package"]
+      H [ fontsize=12 shape=box label="package"]
+      G -> H [label="visibility todo" color=darkred style=dashed arrowhead=tee]
+      I [ fontsize=12 shape=box label="package"]
+      J [ fontsize=12 shape=box label="package"]
+      I -> J [label="dependency todo" color=darkred style=dashed arrowhead=odiamond]
+      LEGEND_NODE_1 [ label="" peripheries=0 height=0 width=0 style=invis ]
+      LEGEND_NODE_2 [ label="" peripheries=0 height=0 width=0 style=invis ]
+      LEGEND_NODE_1 -> LEGEND_NODE_2 [ style=invis ]
+    }
+  subgraph cluster_teams_legend {
+    fontsize=16
+    label="Team Colors"
+
+      "UI TeamUI Team" [
+        label="UI Team"
+        style=filled
+        fillcolor="#FFA9C5"
+        fontsize=12
+        shape=box
+      ]
+
+  }
+    LEGEND_NODE_2 -> "UI TeamUI Team" [style=invis]
+}

spec/sample_app1/test_output/focussed_on_packs_ui_focus_edges_in.dot

@@ -5,7 +5,7 @@ digraph package_diagram {
     fontname="Helvetica,Arial,sans-serif"
     dpi=100
     layout=dot
-    label=<<b>sample_app1: All packs - hiding dependencies</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
+    label=<<b>All packs. Hiding dependencies</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
     fontsize=18
   ]
   node [

spec/sample_app1/test_output/no_dependencies.dot

@@ -5,7 +5,7 @@ digraph package_diagram {
     fontname="Helvetica,Arial,sans-serif"
     dpi=100
     layout=dot
-    label=<<b>sample_app1: All packs - hiding layers</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
+    label=<<b>All packs. Hiding layers</b><br/><font point-size='12'>Widest todo edge is 5 todos</font>>
     fontsize=18
   ]
   node [