From 0f0f12cd80b82b1dae8093eeb28b44341f18241f Mon Sep 17 00:00:00 2001
From: Marcin Wojdyr <wojdyr@gmail.com>
Date: Fri, 25 Jun 2021 11:35:08 +0200
Subject: [PATCH] parse triplets such as "h/2+k/2, -h/2+k/2, l"

---
 include/gemmi/symmetry.hpp | 60 +++++++++++++++++++++++++-------------
 tests/test_symmetry.py     |  3 ++
 2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/include/gemmi/symmetry.hpp b/include/gemmi/symmetry.hpp
index c068fb1db..497f5dde3 100644
--- a/include/gemmi/symmetry.hpp
+++ b/include/gemmi/symmetry.hpp
@@ -10,7 +10,7 @@
 
 #include <cstdint>
 #include <cstdlib>    // for strtol
-#include <cstring>    // for memchr, strchr, strlen
+#include <cstring>    // for memchr, strchr
 #include <array>
 #include <algorithm>  // for count, sort, remove
 #include <functional> // for hash
@@ -206,6 +206,16 @@ inline Op Op::inverse() const {
 
 // TRIPLET -> OP
 
+inline int interpret_miller_character(char c, const std::string& s) {
+  static const signed char values[] =
+    //a  b  c  d  e  f  g  h  i  j  k  l  m  n  o  p  q  r  s  t  u  v  w  x  y  z
+    { 1, 2, 3, 0, 0, 0, 0, 1, 0, 0, 2, 3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 3 };
+  size_t idx = size_t((c | 0x20) - 'a');  // "|0x20" = to lower
+  if (idx >= sizeof(values) || values[idx] == 0)
+    fail("unexpected character '", c, "' in: ", s);
+  return values[idx] - 1;
+}
+
 inline std::array<int, 4> parse_triplet_part(const std::string& s) {
   std::array<int, 4> r = { 0, 0, 0, 0 };
   int num = Op::DEN;
@@ -217,30 +227,38 @@ inline std::array<int, 4> parse_triplet_part(const std::string& s) {
     }
     if (num == 0)
       fail("wrong or unsupported triplet format: " + s);
-    bool is_shift = false;
+    int r_idx;
+    int den = 1;
     if (*c >= '0' && *c <= '9') {
+      // syntax examples in this branch: "1", "-1/2", "+2*x", "1/2 * b"
       char* endptr;
       num *= std::strtol(c, &endptr, 10);
-      if (*endptr == '/') {
-        int den = std::strtol(endptr + 1, &endptr, 10);
-        if (den < 1 || Op::DEN % den != 0)
-          fail("Wrong denominator " + std::to_string(den) + " in: " + s);
-        num /= den;
+      if (*endptr == '/')
+        den = std::strtol(endptr + 1, &endptr, 10);
+      if (*endptr == '*') {
+        c = impl::skip_blank(endptr + 1);
+        r_idx = interpret_miller_character(*c, s);
+        ++c;
+      } else {
+        c = endptr;
+        r_idx = 3;
+      }
+    } else {
+      // syntax examples in this branch: "x", "+a", "-k/3"
+      r_idx = interpret_miller_character(*c, s);
+      c = impl::skip_blank(++c);
+      if (*c == '/') {
+        char* endptr;
+        den = std::strtol(c + 1, &endptr, 10);
+        c = endptr;
       }
-      is_shift = (*endptr != '*');
-      c = (is_shift ? endptr - 1 : impl::skip_blank(endptr + 1));
     }
-    if (is_shift)
-      r[3] += num;
-    else if (std::memchr("xXhHaA", *c, 6))
-      r[0] += num;
-    else if (std::memchr("yYkKbB", *c, 6))
-      r[1] += num;
-    else if (std::memchr("zZlLcC", *c, 6))
-      r[2] += num;
-    else
-      fail(std::string("unexpected character '") + *c + "' in: " + s);
-    ++c;
+    if (den != 1) {
+      if (den <= 0 || Op::DEN % den != 0)
+        fail("Wrong denominator " + std::to_string(den) + " in: " + s);
+      num /= den;
+    }
+    r[r_idx] += num;
     num = 0;
   }
   if (num != 0)
@@ -730,7 +748,7 @@ inline Op hall_matrix_symbol(const char* start, const char* end,
 // Parses either short (0 0 1) or long notation (x,y,z+1/12)
 // but without multpliers (such as 1/2x) to keep things simple for now.
 inline Op parse_hall_change_of_basis(const char* start, const char* end) {
-  if (memchr(start, ',', end - start) != nullptr) // long symbol
+  if (std::memchr(start, ',', end - start) != nullptr) // long symbol
     return parse_triplet(std::string(start, end));
   // short symbol (0 0 1)
   Op cob = Op::identity();
diff --git a/tests/test_symmetry.py b/tests/test_symmetry.py
index 94720c605..1d6f669e1 100644
--- a/tests/test_symmetry.py
+++ b/tests/test_symmetry.py
@@ -102,6 +102,9 @@ def test_invert(self):
         self.assertEqual(gemmi.Op(expected_inv), inv)
         op = gemmi.Op('1/2*x+1/2*y,-1/2*x+1/2*y,z')
         self.assertEqual(op.inverse().triplet(), 'x-y,x+y,z')
+        # check also alternative writing
+        op2 = gemmi.Op('x/2+y/2,-a/2+k/2,z')
+        self.assertEqual(op, op2)
 
     def test_generators_from_hall(self):
         # first test on example matrices from