From ed833506a9d152776f251684facf9b7d7154a39d Mon Sep 17 00:00:00 2001
From: Andreas Longva <andreas.b.longva@gmail.com>
Date: Sun, 11 Apr 2021 17:29:33 +0200
Subject: [PATCH] Fix warnings, refactor code

---
 nalgebra-macros/src/lib.rs     | 40 ++++++++++++++++++++--------------
 nalgebra-macros/tests/tests.rs | 10 +++++----
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/nalgebra-macros/src/lib.rs b/nalgebra-macros/src/lib.rs
index 10127094..3a75b471 100644
--- a/nalgebra-macros/src/lib.rs
+++ b/nalgebra-macros/src/lib.rs
@@ -34,33 +34,41 @@ impl Matrix {
 }
 
 type MatrixRowSyntax = Punctuated<Expr, Token![,]>;
-type MatrixSyntax = Punctuated<MatrixRowSyntax, Token![;]>;
 
 impl Parse for Matrix {
     fn parse(input: ParseStream) -> Result<Self> {
-        let span = input.span();
-        // TODO: Handle empty matrix case
-        let ast = MatrixSyntax::parse_separated_nonempty_with(input,
-            |input| MatrixRowSyntax::parse_separated_nonempty(input))?;
-        let ncols = ast.first().map(|row| row.len())
-            .unwrap_or(0);
-
         let mut rows = Vec::new();
+        let mut ncols = None;
 
-        for row in ast {
-            if row.len() != ncols {
-                // TODO: Is this the correct span?
-                // Currently it returns the span corresponding to the first element in the macro
-                // invocation, but it would be nice if it returned the span of the first element
-                // in the first row that has an unexpected number of columns
-                return Err(Error::new(span, "Unexpected number of columns. TODO"))
+        while !input.is_empty() {
+            let row_span = input.span();
+            let row = MatrixRowSyntax::parse_separated_nonempty(input)?;
+
+            if let Some(ncols) = ncols {
+                if row.len() != ncols {
+                    let row_idx = rows.len();
+                    let error_msg = format!(
+                        "Unexpected number of entries in row {}. Expected {}, found {} entries.",
+                        row_idx,
+                        ncols,
+                        row.len());
+                    return Err(Error::new(row_span, error_msg));
+                }
+            } else {
+                ncols = Some(row.len());
             }
             rows.push(row.into_iter().collect());
+
+            // We've just read a row, so if there are more tokens, there must be a semi-colon,
+            // otherwise the input is malformed
+            if !input.is_empty() {
+                input.parse::<Token![;]>()?;
+            }
         }
 
         Ok(Self {
             rows,
-            ncols
+            ncols: ncols.unwrap_or(0)
         })
     }
 }
diff --git a/nalgebra-macros/tests/tests.rs b/nalgebra-macros/tests/tests.rs
index 5c87ad87..ddbcc1ed 100644
--- a/nalgebra-macros/tests/tests.rs
+++ b/nalgebra-macros/tests/tests.rs
@@ -1,8 +1,10 @@
 use nalgebra_macros::matrix;
+use nalgebra::Matrix3x2;
 
 #[test]
-fn basic_usage() {
-    matrix![ 1, 3;
-             4, 5*3;
-             3, 3];
+fn matrix_basic_construction() {
+    let matrix: Matrix3x2<_> = matrix![ 1, 2;
+                                        3, 4;
+                                        5, 6 ];
+    assert_eq!(matrix, Matrix3x2::new(1, 2, 3, 4, 5, 6));
 }
\ No newline at end of file