Made comment editing easier to use

In many cases the pre-comment of one move is displayed at the same
position in the move text as the post-comment of the previous move.
When this is the case, don't make a distinction between pre and post
comments in the "Edit Comments" dialog.

DroidFishApp/src/androidTest/java/org/petero/droidfish/gamelogic/GameTest.java

@@ -21,8 +21,10 @@ package org.petero.droidfish.gamelogic;
 import android.util.Pair;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 
 import org.petero.droidfish.PGNOptions;
+import org.petero.droidfish.gamelogic.Game.CommentInfo;
 
 import junit.framework.TestCase;
 
@@ -545,4 +547,170 @@ public class GameTest extends TestCase {
             assertEquals("Qc8# Qc7", GameTreeTest.getVariationsAsString(game.tree));
         }
     }
+
+    public final void testComments() throws ChessParseError {
+        PGNOptions options = new PGNOptions();
+        options.imp.variations = true;
+        options.imp.comments = true;
+        options.imp.nag = true;
+        {
+            Game game = new Game(null, new TimeControlData());
+            String pgn = "{a} 1. e4 {b} 1... e5 {c} ({g} 1... c6 {h} 2. Nf3 {i} 2... d5) " +
+                         "2. Nf3 {d} 2... Nc6 {e} 3. d3 {f} 3... Nf6 4. Nc3 d5 *";
+            boolean res = game.readPGN(pgn, options);
+            assertEquals(true, res);
+            Pair<CommentInfo,Boolean> p = game.getComments();
+//            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("", p.first.preComment);
+            assertEquals("a", p.first.postComment);
+
+            game.tree.goForward(0); // At "e4"
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("a", p.first.preComment);
+            assertEquals("b", p.first.postComment);
+
+            game.tree.goForward(0); // At "e5"
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("b", p.first.preComment);
+            assertEquals("c", p.first.postComment);
+
+            game.tree.goForward(0); // At "Nf3" in mainline
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("", p.first.preComment);
+            assertEquals("d", p.first.postComment);
+
+            game.tree.goForward(0); // At "Nc6" in mainline
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("d", p.first.preComment);
+            assertEquals("e", p.first.postComment);
+
+            game.tree.goForward(0); // At "d3" in mainline
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("e", p.first.preComment);
+            assertEquals("f", p.first.postComment);
+
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goForward(1); // At "c6" in variation
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("g", p.first.preComment);
+            assertEquals("h", p.first.postComment);
+
+            game.tree.goForward(1); // At "Nf3" in variation
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("h", p.first.preComment);
+            assertEquals("i", p.first.postComment);
+
+            game.tree.goForward(1); // At "d5" in variation
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("i", p.first.preComment);
+            assertEquals("", p.first.postComment);
+
+            game.tree.goBack();
+            game.tree.goBack(); // At "c6" in variation
+            game.moveVariation(-1); // At "c6" which is now mainline
+            p = game.getComments();
+            assertEquals(Boolean.TRUE, p.second);
+            assertEquals("b g", p.first.preComment);
+            assertEquals("h", p.first.postComment);
+
+            game.tree.goBack();
+            game.tree.goForward(1); // At "e5" in variation
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("", p.first.preComment);
+            assertEquals("c", p.first.postComment);
+
+            p.first.preComment = "x";
+            game.setComments(p.first);
+            p = game.getComments();
+            assertEquals(Boolean.FALSE, p.second);
+            assertEquals("x", p.first.preComment);
+            assertEquals("c", p.first.postComment);
+
+            game.moveVariation(-1); // Still at "e5", now mainline again
+            game.tree.goBack(); // At "e4"
+            p = game.getComments();
+            assertEquals(Boolean.TRUE, p.second);
+            assertEquals("a", p.first.preComment);
+            assertEquals("b g x", p.first.postComment);
+        }
+        {
+            Game game = new Game(null, new TimeControlData());
+            String pgn = "{a} 1. e4 (1. d4) *";
+            boolean res = game.readPGN(pgn, options);
+            assertEquals(true, res);
+            Pair<CommentInfo,Boolean> p = game.getComments();
+            assertEquals("", p.first.preComment);
+            assertEquals("a", p.first.postComment);
+        }
+        {
+            Game game = new Game(null, new TimeControlData());
+            String pgn = "1. e4 e5 (1... c6 2. Nf3 d5) 2. Nf3 Nc6 3. d3 Nf6 4. Nc3 d5 *";
+            boolean res = game.readPGN(pgn, options);
+            assertEquals(true, res);
+
+            CommentInfo info = game.getComments().first;
+            info.postComment = "a";
+            game.setComments(info);
+
+            game.tree.goForward(0);
+            game.tree.goForward(0);
+            info = game.getComments().first;
+            info.preComment = "b";
+            info.postComment = "c";
+            game.setComments(info);
+
+            game.tree.goForward(0);
+            game.tree.goForward(0);
+            info = game.getComments().first;
+            info.preComment = "d";
+            info.postComment = "e";
+            game.setComments(info);
+
+            game.tree.goForward(0);
+            info = game.getComments().first;
+            info.postComment = "f";
+            game.setComments(info);
+
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goBack();
+            game.tree.goForward(1); // At "c6" in variation
+            info = game.getComments().first;
+            info.preComment = "g";
+            info.postComment = "h";
+            game.setComments(info);
+
+            game.tree.goForward(0);
+            game.tree.goForward(0);
+            info = game.getComments().first;
+            info.preComment = "i";
+            info.postComment = "j";
+            game.setComments(info);
+
+            PGNOptions expOpts = new PGNOptions();
+            expOpts.exp.variations = true;
+            expOpts.exp.comments = true;
+            String exported = game.tree.toPGN(expOpts);
+            String[] split = exported.split("\n");
+            split = Arrays.stream(split).filter(e -> !e.startsWith("[") && e.length() > 0)
+                .toArray(String[]::new);
+            exported = String.join(" ", split);
+            String expected = "{a} 1. e4 {b} 1... e5 {c} ({g} 1... c6 {h} 2. Nf3 {i} 2... d5 {j}) " +
+                              "2. Nf3 {d} 2... Nc6 {e} 3. d3 {f} 3... Nf6 4. Nc3 d5 *";
+            assertEquals(expected, exported);
+        }
+    }
 }

DroidFishApp/src/main/java/org/petero/droidfish/DroidFish.java

@@ -52,6 +52,7 @@ import org.petero.droidfish.engine.EngineUtil;
 import org.petero.droidfish.engine.UCIOptions;
 import org.petero.droidfish.gamelogic.DroidChessController;
 import org.petero.droidfish.gamelogic.ChessParseError;
+import org.petero.droidfish.gamelogic.Game;
 import org.petero.droidfish.gamelogic.Move;
 import org.petero.droidfish.gamelogic.Piece;
 import org.petero.droidfish.gamelogic.Position;
@@ -2807,7 +2808,7 @@ public class DroidFish extends Activity
         View content = View.inflate(DroidFish.this, R.layout.edit_comments, null);
         builder.setView(content);
 
-        DroidChessController.CommentInfo commInfo = ctrl.getComments();
+        Game.CommentInfo commInfo = ctrl.getComments();
 
         final TextView preComment, moveView, nag, postComment;
         preComment = content.findViewById(R.id.ed_comments_pre);
@@ -2829,11 +2830,10 @@ public class DroidFish extends Activity
             String post = postComment.getText().toString().trim();
             int nagVal = Node.strToNag(nag.getText().toString());
 
-            DroidChessController.CommentInfo commInfo1 = new DroidChessController.CommentInfo();
-            commInfo1.preComment = pre;
-            commInfo1.postComment = post;
-            commInfo1.nag = nagVal;
-            ctrl.setComments(commInfo1);
+            commInfo.preComment = pre;
+            commInfo.postComment = post;
+            commInfo.nag = nagVal;
+            ctrl.setComments(commInfo);
         });
 
         builder.show();

DroidFishApp/src/main/java/org/petero/droidfish/gamelogic/DroidChessController.java

@@ -43,6 +43,7 @@ import org.petero.droidfish.engine.DroidComputerPlayer;
 import org.petero.droidfish.engine.UCIOptions;
 import org.petero.droidfish.engine.DroidComputerPlayer.SearchRequest;
 import org.petero.droidfish.engine.DroidComputerPlayer.SearchType;
+import org.petero.droidfish.gamelogic.Game.CommentInfo;
 import org.petero.droidfish.gamelogic.Game.GameState;
 import org.petero.droidfish.gamelogic.GameTree.Node;
 
@@ -656,30 +657,20 @@ public class DroidChessController {
         updateGUI();
     }
 
-    /** Comments associated with a move. */
-    public static final class CommentInfo {
-        public String move;
-        public String preComment, postComment;
-        public int nag;
-    }
-
     /** Get comments associated with current position. */
     public final synchronized CommentInfo getComments() {
-        Node cur = game.tree.currentNode;
-        CommentInfo ret = new CommentInfo();
-        ret.move = cur.moveStrLocal;
-        ret.preComment = cur.preComment;
-        ret.postComment = cur.postComment;
-        ret.nag = cur.nag;
-        return ret;
+        Pair<CommentInfo,Boolean> p = game.getComments();
+        if (p.second) {
+            gameTextListener.clear();
+            updateGUI();
+        }
+        return p.first;
     }
 
-    /** Set comments associated with current position. */
+    /** Set comments associated with current position. "commInfo" must be an object
+     *  (possibly modified) previously returned from getComments(). */
     public final synchronized void setComments(CommentInfo commInfo) {
-        Node cur = game.tree.currentNode;
-        cur.preComment = commInfo.preComment.replace('}', '\uff5d');
-        cur.postComment = commInfo.postComment.replace('}', '\uff5d');
-        cur.nag = commInfo.nag;
+        game.setComments(commInfo);
         gameTextListener.clear();
         updateGUI();
     }

DroidFishApp/src/main/java/org/petero/droidfish/gamelogic/Game.java

@@ -552,4 +552,67 @@ public class Game {
         }
         return ret;
     }
+
+    /** Comments associated with a move. */
+    public static final class CommentInfo {
+        private Node parent; // If non-null, use parent.postComment instead of
+        // node.preComment when updating comment data.
+        public String move;
+        public String preComment;
+        public String postComment;
+        public int nag;
+    }
+
+    /** Get comments associated with current position.
+     *  Return information about the comments in ret.first. ret.second is true
+     *  if the GUI needs to be updated because comments were coalesced. */
+    public Pair<CommentInfo,Boolean> getComments() {
+        Node cur = tree.currentNode;
+
+        // Move preComment to corresponding postComment of parent node if possible,
+        // i.e. if the comments would be next to each other in the move text area.
+        Node parent = cur.getParent();
+        if (parent != null && cur.getChildNo() != 0)
+            parent = null;
+        if (parent != null && parent.hasSibling() && parent.getChildNo() == 0)
+            parent = null;
+        boolean needUpdate = false;
+        if (parent != null && !cur.preComment.isEmpty()) {
+            if (!parent.postComment.isEmpty())
+                parent.postComment += ' ';
+            parent.postComment += cur.preComment;
+            cur.preComment = "";
+            needUpdate = true;
+        }
+        Node child = (cur.hasSibling() && cur.getChildNo() == 0) ? null : cur.getFirstChild();
+        if (child != null && !child.preComment.isEmpty()) {
+            if (!cur.postComment.isEmpty())
+                cur.postComment += ' ';
+            cur.postComment += child.preComment;
+            child.preComment = "";
+            needUpdate = true;
+        }
+
+        CommentInfo ret = new CommentInfo();
+        ret.parent = parent;
+        ret.move = cur.moveStrLocal;
+        ret.preComment = parent != null ? parent.postComment : cur.preComment;
+        ret.postComment = cur.postComment;
+        ret.nag = cur.nag;
+
+        return new Pair<>(ret, needUpdate);
+    }
+
+    /** Set comments associated with current position. "commInfo" must be an object
+     *  (possibly modified) previously returned from getComments(). */
+    public final void setComments(CommentInfo commInfo) {
+        Node cur = tree.currentNode;
+        String preComment = commInfo.preComment.replace('}', '\uff5d');
+        if (commInfo.parent != null)
+            commInfo.parent.postComment = preComment;
+        else
+            cur.preComment = preComment;
+        cur.postComment = commInfo.postComment.replace('}', '\uff5d');
+        cur.nag = commInfo.nag;
+    }
 }

DroidFishApp/src/main/java/org/petero/droidfish/gamelogic/GameTree.java

@@ -1075,6 +1075,14 @@ public class GameTree {
             return parent;
         }
 
+        public boolean hasSibling() {
+            return parent != null && parent.children.size() > 1;
+        }
+
+        public Node getFirstChild() {
+            return children.isEmpty() ? null : children.get(0);
+        }
+
         /** nodePos must represent the same position as this Node object. */
         private boolean verifyChildren(Position nodePos) {
             return verifyChildren(nodePos, null);

README.md

@@ -171,12 +171,6 @@ Tap and hold the move text area to open a menu with the following actions:
     * `+- ` : White has a decisive advantage
     * `-+ ` : Black has a decisive advantage
 
-  **Note!** When a game is exported in PGN format the *Before* comment for one
-  move is merged with the *After* comment for the previous move, if there is a
-  previous move adjacent to the current move in the PGN data. The *Before*
-  comment should therefore only be used when this is not the case, such as at
-  the start of the game or at the start of a variation.
-
 * *Add opening name*: Adds or updates the `ECO` and `Opening` PGN headers based
   on information from the ECO (Encyclopedia of Chess Openings) database and the
   main line in the current game.