Fix some reply issues

- Scroll to default selected message when sending reply - Correctly add initial msg text when replying for the first time - Some changes to make tags handling more reliable - Fix error in reply popup when reply cache is already removed - Increase reply cache time
23rd · Aug 28, 2020 · b31a0ae · b31a0ae
1 parent 0fb25b5
commit b31a0ae
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 15 deletions.
diff --git a/src/chatty/TwitchClient.java b/src/chatty/TwitchClient.java
@@ -859,6 +859,14 @@ private void sendMessage(String channel, String text, boolean allowCommandMessag
         }
     }
 
+    /**
+     * Check if the message should be sent as a reply.
+     * 
+     * @param channel The channel to send the message to (not null)
+     * @param text The text to send (not null)
+     * @return true if the message was handled by this method, false if it
+     * should be sent normally
+     */
     private boolean sendAsReply(String channel, String text) {
         boolean restricted = settings.getBoolean("mentionReplyRestricted");
         boolean doubleAt = text.startsWith("@@");
@@ -875,7 +883,8 @@ private boolean sendAsReply(String channel, String text) {
                     if (result.action != SelectReplyMessageResult.Action.SEND_NORMALLY) {
                         // Should not send normally, so return true
                         if (result.action == SelectReplyMessageResult.Action.REPLY) {
-                            sendReply(channel, actualMsg, username, result.atMsgId, null);
+                            // If changed to parent msg-id, atMsg will be null
+                            sendReply(channel, actualMsg, username, result.atMsgId, result.atMsg);
                         }
                         return true;
                     }
@@ -885,6 +894,15 @@ private boolean sendAsReply(String channel, String text) {
         return false;
     }
 
+    /**
+     * Send a reply.
+     * 
+     * @param channel The channel to send to (not null)
+     * @param text The text to send (not null)
+     * @param atUsername The username to address (not null)
+     * @param atMsgId The msg-id to use as reply thread parent (not null)
+     * @param atMsg The parent msg text (may be null)
+     */
     private void sendReply(String channel, String text, String atUsername, String atMsgId, String atMsg) {
         MsgTags tags = MsgTags.create("reply-parent-msg-id", atMsgId);
         if (c.sendSpamProtectedMessage(channel, text, false, tags)) {
@@ -2936,7 +2954,7 @@ public void onChannelMessage(User user, String text, boolean action, MsgTags tag
             }
             else {
                 g.printMessage(user, text, action, tags);
-                if (tags.isReply() && tags.hasReplyUserMsg()) {
+                if (tags.isReply() && tags.hasReplyUserMsg() && tags.hasId()) {
                     ReplyManager.addReply(tags.getReplyParentMsgId(), tags.getId(), String.format("<%s> %s", user.getName(), text), tags.getReplyUserMsg());
                 }
                 if (!action) {

diff --git a/src/chatty/TwitchConnection.java b/src/chatty/TwitchConnection.java
@@ -1408,7 +1408,7 @@ public void onChannelCommand(MsgTags tags, String nick,
                     if (tags.containsKey("followers-only")) {
                         channelStates.setFollowersOnly(channel, tags.get("followers-only"));
                     }
-                    if (!tags.isEmpty("room-id")) {
+                    if (tags.hasValue("room-id")) {
                         listener.onRoomId(channel, tags.get("room-id"));
                         if (Helper.isRegularChannel(channel)) {
                             // Set id for room (this should not run too often to

diff --git a/src/chatty/gui/components/SelectReplyMessage.java b/src/chatty/gui/components/SelectReplyMessage.java
@@ -163,11 +163,14 @@ else if (action == Action.CTRL_ENTER) {
         private void confirm() {
             User.TextMessage selected = list.getSelectedValue();
             if (selected != null) {
-                result = new SelectReplyMessageResult(selected.id);
+                result = new SelectReplyMessageResult(selected.id, selected.text);
                 if (continueThread.isSelected()) {
                     String parentMsgId = ReplyManager.getParentMsgId(selected.id);
                     if (parentMsgId != null) {
-                        result = new SelectReplyMessageResult(parentMsgId);
+                        // Overwrite result with parent msg-id if available
+                        // Msg should be null for this, since the selected.text
+                        // isn't the parent text
+                        result = new SelectReplyMessageResult(parentMsgId, null);
                     }
                 }
             }
@@ -177,9 +180,10 @@ private void confirm() {
         public SelectReplyMessageResult select() {
             if (list.getModel().getSize() == 0) {
                 // No messages available, so immediatelly return
-                return null;
+                return SEND_NORMALLY_RESULT;
             }
             list.setSelectedIndex(list.getModel().getSize() - 1);
+            list.ensureIndexIsVisible(list.getSelectedIndex());
             setLocationRelativeTo(getParent());
             setVisible(true);
             return result;
@@ -194,15 +198,18 @@ public enum Action {
         }
 
         public final String atMsgId;
+        public final String atMsg;
         public final Action action;
 
-        public SelectReplyMessageResult(String atMsgId) {
+        public SelectReplyMessageResult(String atMsgId, String atMsg) {
             this.atMsgId = atMsgId;
+            this.atMsg = atMsg;
             this.action = Action.REPLY;
         }
 
         public SelectReplyMessageResult(Action action) {
             this.atMsgId = null;
+            this.atMsg = null;
             this.action = action;
         }
 
@@ -213,6 +220,9 @@ public static void main(String[] args) {
         user.addMessage("abc", true, "1");
         user.addMessage("abc2", true, "2");
         user.addMessage("abc2", true, null);
+        for (int i=0;i<30;i++) {
+            user.addMessage("blah"+i, false, i+"msg-id");
+        }
         System.out.println(SelectReplyMessage.show(user));
         System.exit(0);
     }

diff --git a/src/chatty/gui/components/textpane/LinkController.java b/src/chatty/gui/components/textpane/LinkController.java
@@ -806,9 +806,14 @@ private static void makeReplacementPopupText(String replacedText, MyPopup p) {
     private static void makeReplyPopupText(String replyMsgId, MyPopup p) {
         List<Reply> replies = ReplyManager.getReplies(replyMsgId);
         StringBuilder b = new StringBuilder();
-        for (Reply reply : replies) {
-            b.append(StringUtil.addLinebreaks(Helper.htmlspecialchars_encode(reply.userMsg), 70, true));
-            b.append("<br />");
+        if (replies != null) {
+            for (Reply reply : replies) {
+                b.append(StringUtil.addLinebreaks(Helper.htmlspecialchars_encode(reply.userMsg), 70, true));
+                b.append("<br />");
+            }
+        }
+        else {
+            b.append("No reply data found (may have expired).");
         }
         p.setText(String.format("%sThread:<div style='text-align:left;font-weight:normal'>%s</div>",
                 POPUP_HTML_PREFIX, b.toString()));

diff --git a/src/chatty/util/ReplyManager.java b/src/chatty/util/ReplyManager.java
@@ -17,7 +17,7 @@
  */
 public class ReplyManager {
 
-    private static final long DELETE_TIME = TimeUnit.HOURS.toMillis(4);
+    private static final long DELETE_TIME = TimeUnit.HOURS.toMillis(12);
 //    private static final long DELETE_TIME = TimeUnit.MINUTES.toMillis(2);
 
     private static final Map<String, List<Reply>> data = new HashMap<>();

diff --git a/src/chatty/util/irc/IrcMsgTags.java b/src/chatty/util/irc/IrcMsgTags.java
@@ -101,8 +101,9 @@ public boolean isValueOf(String key, String... values) {
         return false;
     }
 
-    public boolean isEmpty(String key) {
-        return !tags.containsKey(key) || tags.get(key).isEmpty();
+    public boolean hasValue(String key) {
+        String value = tags.get(key);
+        return value != null && !value.isEmpty();
     }
 
     /**

diff --git a/src/chatty/util/irc/MsgTags.java b/src/chatty/util/irc/MsgTags.java
@@ -21,6 +21,10 @@ public String getId() {
         return get("id");
     }
 
+    public boolean hasId() {
+        return hasValue("id");
+    }
+
     public int getBits() {
         return getInteger("bits", 0);
     }
@@ -46,7 +50,7 @@ public boolean isFromPubSub() {
     }
 
     public boolean hasReplyUserMsg() {
-        return containsKey("reply-parent-msg-body") && containsKey("reply-parent-display-name");
+        return hasValue("reply-parent-msg-body") && hasValue("reply-parent-display-name");
     }
 
     public String getReplyUserMsg() {
@@ -59,7 +63,7 @@ public String getReplyUserMsg() {
     }
 
     public boolean isReply() {
-        return containsKey("reply-parent-msg-id");
+        return hasValue("reply-parent-msg-id");
     }
 
     public String getReplyParentMsgId() {