From 1106708a6de2c52cb3c12bb8a9649b8d38f1d8db Mon Sep 17 00:00:00 2001
From: "Salvador E. Tropea" <salvador@inti.gob.ar>
Date: Thu, 6 Oct 2022 08:58:12 -0300
Subject: [PATCH] [Fixed][Position] Wrong side classification

- When the side column wasn't the last column

Closes #313
---
 CHANGELOG.md                     |  2 ++
 kibot/out_position.py            | 28 ++++++++++------------------
 tests/test_plot/test_position.py |  4 ++++
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 87d6d12d..192e8291 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Schematic v6: Problems when creating a variant of a sub-sheet that was edited
   as a standalone sheet (#307)
 - iBoM: Name displayed in the HTML when using filters and/or variants.
+- Position: Components wrongly separated by side when the side column wasn't
+  the last column (#313)
 
 ### Changed
 - Diff:
diff --git a/kibot/out_position.py b/kibot/out_position.py
index 3bd2b11f..9c4cf076 100644
--- a/kibot/out_position.py
+++ b/kibot/out_position.py
@@ -100,7 +100,7 @@ class PositionOptions(VariantOptions):
             self.columns = new_columns
         self._expand_ext = 'pos' if self.format == 'ASCII' else 'csv'
 
-    def _do_position_plot_ascii(self, output_dir, columns, modulesStr, maxSizes):
+    def _do_position_plot_ascii(self, output_dir, columns, modulesStr, maxSizes, modules_side):
         topf = None
         botf = None
         bothf = None
@@ -139,13 +139,8 @@ class PositionOptions(VariantOptions):
         # Account for the "# " at the start of the comment column
         maxSizes[0] = maxSizes[0] + 2
 
-        for m in modulesStr:
-            file = bothf
-            if file is None:
-                if m[-1] == "top":
-                    file = topf
-                else:
-                    file = botf
+        for (m, is_bottom) in zip(modulesStr, modules_side):
+            file = bothf if bothf is not None else (botf if is_bottom else topf)
             for idx, col in enumerate(m):
                 if idx > 0:
                     file.write("   ")
@@ -162,7 +157,7 @@ class PositionOptions(VariantOptions):
         if bothf is not None:
             bothf.close()
 
-    def _do_position_plot_csv(self, output_dir, columns, modulesStr):
+    def _do_position_plot_csv(self, output_dir, columns, modulesStr, modules_side):
         topf = None
         botf = None
         bothf = None
@@ -181,13 +176,8 @@ class PositionOptions(VariantOptions):
             f.write(",".join(columns))
             f.write("\n")
 
-        for m in modulesStr:
-            file = bothf
-            if file is None:
-                if m[-1] == "top":
-                    file = topf
-                else:
-                    file = botf
+        for (m, is_bottom) in zip(modulesStr, modules_side):
+            file = bothf if bothf is not None else (botf if is_bottom else topf)
             file.write(",".join('{}'.format(e) for e in m))
             file.write("\n")
 
@@ -235,6 +225,7 @@ class PositionOptions(VariantOptions):
         # Format all strings
         comps_hash = self.get_refs_hash()
         modules = []
+        modules_side = []
         is_pure_smd, is_not_virtual = self.get_attr_tests()
         quote_char = '"' if self.format == 'CSV' else ''
         x_origin = 0.0
@@ -292,6 +283,7 @@ class PositionOptions(VariantOptions):
                     elif k == 'Side':
                         row.append("bottom" if is_bottom else "top")
                 modules.append(row)
+                modules_side.append(is_bottom)
         # Find max width for all columns
         maxlengths = []
         for col, name in enumerate(columns):
@@ -301,9 +293,9 @@ class PositionOptions(VariantOptions):
             maxlengths.append(max_l)
         # Note: the parser already checked the format is ASCII or CSV
         if self.format == 'ASCII':
-            self._do_position_plot_ascii(output_dir, columns, modules, maxlengths)
+            self._do_position_plot_ascii(output_dir, columns, modules, maxlengths, modules_side)
         else:  # if self.format == 'CSV':
-            self._do_position_plot_csv(output_dir, columns, modules)
+            self._do_position_plot_csv(output_dir, columns, modules, modules_side)
 
 
 @output_class
diff --git a/tests/test_plot/test_position.py b/tests/test_plot/test_position.py
index 2eb6ff5a..852ba955 100644
--- a/tests/test_plot/test_position.py
+++ b/tests/test_plot/test_position.py
@@ -140,6 +140,10 @@ def test_position_csv_cols(test_dir):
     ctx.expect_out_file(pos_top)
     ctx.expect_out_file(pos_bot)
     assert ctx.search_in_file(pos_top, ["Ref,Value,Center X"]) is not None
+    ctx.search_in_file(pos_top, ['^"R1",'])
+    ctx.search_not_in_file(pos_top, ['^"R2",', '^"R3",'])
+    ctx.search_in_file(pos_bot, ['^"R2",'])
+    ctx.search_not_in_file(pos_bot, ['^"R1",', '^"R3",'])
     ctx.clean_up()