From fc3e65a897fba9daa4664ed46d7bc49076d9c0f2 Mon Sep 17 00:00:00 2001
From: John May <john@nextmovesoftware.com>
Date: Tue, 9 Aug 2016 14:55:09 +0100
Subject: [PATCH] Don't reset atom labels when we read atom values and allow
 trailing ';' in the atom label field.

---
 .../openscience/cdk/smiles/CxSmilesParser.java   |  7 ++++++-
 .../org/openscience/cdk/smiles/CxSmilesTest.java | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/storage/smiles/src/main/java/org/openscience/cdk/smiles/CxSmilesParser.java b/storage/smiles/src/main/java/org/openscience/cdk/smiles/CxSmilesParser.java
index 73496ebed51..6480a3615d8 100644
--- a/storage/smiles/src/main/java/org/openscience/cdk/smiles/CxSmilesParser.java
+++ b/storage/smiles/src/main/java/org/openscience/cdk/smiles/CxSmilesParser.java
@@ -73,6 +73,7 @@ private static boolean processAtomLabels(final CharIter iter, final Map<Integer,
 
             char c = iter.next();
             if (c == '$') {
+                iter.nextIf(','); // optional
                 // end of atom label
                 return true;
             } else {
@@ -410,10 +411,14 @@ static int processCx(final String str, final CxSmilesState state) {
             switch (iter.next()) {
                 case '$': // atom labels and values
                     // dest is atom labels by default
-                    Map<Integer, String> dest = state.atomLabels = new TreeMap<>();
+                    Map<Integer, String> dest;
+
                     // check for atom values
                     if (iter.nextIf("_AV:"))
                         dest = state.atomValues = new TreeMap<>();
+                    else
+                        dest = state.atomLabels = new TreeMap<>();
+
                     if (!processAtomLabels(iter, dest))
                         return -1;
                     break;
diff --git a/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesTest.java b/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesTest.java
index 534fe33fc5e..9b46157e5f8 100644
--- a/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesTest.java
+++ b/storage/smiles/src/test/java/org/openscience/cdk/smiles/CxSmilesTest.java
@@ -27,6 +27,7 @@
 import org.junit.Test;
 import org.openscience.cdk.CDKConstants;
 import org.openscience.cdk.exception.InvalidSmilesException;
+import org.openscience.cdk.interfaces.IAtom;
 import org.openscience.cdk.interfaces.IAtomContainer;
 import org.openscience.cdk.interfaces.IPseudoAtom;
 import org.openscience.cdk.interfaces.IReaction;
@@ -37,6 +38,7 @@
 
 import javax.vecmath.Point2d;
 import javax.vecmath.Point3d;
+import java.util.ArrayList;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -200,5 +202,19 @@ public void atomValues() throws InvalidSmilesException {
         IReaction rxn = smipar.parseReactionSmiles("C1=CC(=CC=C1)C(CC(N)=O)=O.*C>C1(=CC=CC=C1)N.C*.C1=CC(=CC=C1)C=2C=C(C3=C(N2)C=CC=C3)O.C*.C*> |$;;;;;;;;;;;;R22;;;;;;;;;;;;;;;;;;;;;;;;;;;;;R22$,f:0.1,2.3,4.5.6,m:13:0.1.2.3.4.5,21:14.15.16.17.18.19,40:23.24.25.26.27.28,42:29.30.31.32.33.34.35.36.37.38|");
     }
 
+    @Test public void trailingAtomLabelSemiColonAndAtomValues() throws Exception {
+        IAtomContainer mol = smipar.parseSmiles("[H]C1=C([H])N2C(=O)C(=C([O-])[N+](CC3=CN=C(Cl)S3)=C2C(C)=C1[H])C1=CC(*)=CC=C1.** |$;;;;;;;;;;;;;;;;;;;;;;;;;;R;;;;RA;$,$_AV:;;;;;;;;;;;;;;;;;;;;;;;;2;;;4;5;6;;$,c:1,18,22,29,31,t:7,12,14,26,m:31:29.28.27.25.24.23|");
+        List<Sgroup> sgroups = mol.getProperty(CDKConstants.CTAB_SGROUPS);
+        assertThat(mol.getAtom(26), is(instanceOf(IPseudoAtom.class)));
+        assertThat(mol.getAtom(30), is(instanceOf(IPseudoAtom.class)));
+        assertThat(((IPseudoAtom)mol.getAtom(26)).getLabel(), is("R"));
+        assertThat(((IPseudoAtom)mol.getAtom(30)).getLabel(), is("RA"));
+        assertThat(mol.getAtom(24).getProperty(CDKConstants.COMMENT), CoreMatchers.<Object>is("2"));
+        assertThat(mol.getAtom(27).getProperty(CDKConstants.COMMENT), CoreMatchers.<Object>is("4"));
+        assertThat(mol.getAtom(28).getProperty(CDKConstants.COMMENT), CoreMatchers.<Object>is("5"));
+        assertThat(mol.getAtom(29).getProperty(CDKConstants.COMMENT), CoreMatchers.<Object>is("6"));
+        assertThat(sgroups.size(), is(1));
+    }
+
 
 }