Merge lp:~julien-spautz/cable/message-history-refactor into lp:cable

[Julien Spautz]

Refactored the MessageHistory class and added more tests.

[Julien Spautz (julien-spautz)]

Forgot to change CMakeLists.txt. Don't merge yet.

[982c80311320c1b (alexander-wilms)]

One detail that caught my attention is that when you browser the history and then repeatedly press <down>, you won't end up with an empty text entry, but the last message is still shown.

[Julien Spautz (julien-spautz)]

You mean in trunk? Because it should be fixed in this branch.

[982c80311320c1b (alexander-wilms)]

Yes, trunk. I thought you only refactored it.

[lp:~julien-spautz/cable/message-history-refactor]

132. By Julien Spautz on 2013-09-13

fixed CMakeLists.txt

[982c80311320c1b (alexander-wilms)]

Works as expected and all tests pass

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-09-05 19:26:00 +0000
+++ CMakeLists.txt	2013-09-13 14:24:42 +0000
@@ -55,7 +55,7 @@
     src/Utils/EmbeddedAlert.vala
     src/Utils/Identity.vala
     src/Utils/Network.vala
-    src/Utils/RRStringList.vala
+    src/Utils/MessageHistory.vala
 
     src/Irc/Maki.vala
     src/Irc/Irc.vala
@@ -118,7 +118,7 @@
     tests/TestMessageHistory.vala
 
     src/Utils/Utils.vala
-    src/Utils/RRStringList.vala
+    src/Utils/MessageHistory.vala
 PACKAGES
     gtk+-3.0
     granite

=== added file 'src/Utils/MessageHistory.vala'
--- src/Utils/MessageHistory.vala	1970-01-01 00:00:00 +0000
+++ src/Utils/MessageHistory.vala	2013-09-13 14:24:42 +0000
@@ -0,0 +1,78 @@
+/***
+    Copyright (C) 2013 Cable Developers
+
+    This program or library is free software; you can redistribute it
+    and/or modify it under the terms of the GNU Lesser General Public
+    License as published by the Free Software Foundation; either
+    version 3 of the License, or (at your option) any later version.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+    Lesser General Public License for more details.
+
+    You should have received a copy of the GNU Lesser General
+    Public License along with this library; if not, write to the
+    Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301 USA.
+***/
+
+public class Cable.Utils.MessageHistory {
+
+    Gee.List <string> data;
+    int current;
+    
+    public MessageHistory () {
+        data = new Gee.ArrayList <string> ();
+        current = 0;
+    }
+
+    // data[data.size] is an imaginary, empty string, which is
+    // always the most recent message
+    public void append (string message) {
+        data.add (message);
+        current = data.size;
+    }
+    
+    public string get_current ()
+        requires (0 <= current <= data.size) {
+        if (current == data.size)
+            return "";
+        return data[current];
+    }
+    
+    public string get_older ()
+        requires (has_older ()) {
+        set_current_to_older ();
+        return get_current ();
+    }
+    
+    public string get_newer ()
+        requires (has_newer ()) {
+        set_current_to_newer ();
+        return get_current ();
+    }
+    
+    public bool has_older () {
+        return current > 0;
+    }
+    
+    public bool has_newer () {
+        return current < data.size;
+    }
+    
+    void set_current_to_older ()
+        requires (has_older ()) {
+        current--;
+    }
+    
+    void set_current_to_newer ()
+        requires (has_newer ()) {
+        current++;
+    }
+
+    public void modify_current (string value)
+        requires (0 <= current < data.size) {
+        data[current] = value;
+    }
+}

=== removed file 'src/Utils/RRStringList.vala'
--- src/Utils/RRStringList.vala	2013-08-31 15:19:12 +0000
+++ src/Utils/RRStringList.vala	1970-01-01 00:00:00 +0000
@@ -1,77 +0,0 @@
-/***
-    Copyright (C) 2013 Cable Developers
-
-    This program or library is free software; you can redistribute it
-    and/or modify it under the terms of the GNU Lesser General Public
-    License as published by the Free Software Foundation; either
-    version 3 of the License, or (at your option) any later version.
-
-    This library is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-    Lesser General Public License for more details.
-
-    You should have received a copy of the GNU Lesser General
-    Public License along with this library; if not, write to the
-    Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
-    Boston, MA 02110-1301 USA.
-***/
-
-
-public class Cable.Collections.RRStringList {
-
-    internal int? current_;
-    internal Gee.List <string> data;
-
-    public int capacity { get; set; }
-    public int? current_index { get { return current_; } }
-
-    public RRStringList.with_capacity (int capacity_value)
-        requires (capacity_value > 0) {
-        capacity = capacity_value;
-        data     = new Gee.ArrayList <string>();
-    }
-
-    public RRStringList () {
-        this.with_capacity(50); // default capacity
-    }
-
-    public void add(string value) {
-        if ( data.size >= this.capacity )
-            data.remove_at( 0 );
-        data.add(value);
-        current_ = data.size/*-1*/;
-    }
-
-    public bool contains(string value) {
-        return data.contains(value);
-    }
-
-    public string current() {
-        if ( current_ == null ) return "";
-        if ( current_ >= data.size ) current_ = data.size-1;
-        if ( current_ < 0 ) current_ = 0;
-        return data[current_];
-    }
-
-    public bool previous() {
-        if ( current_ == null ) return false;
-        if ( current_ <= 0    ) return false;
-        current_ = current_ - 1;
-        return true;
-    }
-
-    public bool next() {
-        if ( current_ == null ) return false;
-        if ( current_+1 >= data.size ) return false;
-        current_ = current_ + 1;
-        return true;
-    }
-
-    public bool set_current_value(string value) {
-        if ( !(value in data) ) return false;
-        current_ = data.index_of(value)+1;
-        return true;
-    }
-
-}

=== modified file 'src/Widgets/Room.vala'
--- src/Widgets/Room.vala	2013-08-31 15:19:12 +0000
+++ src/Widgets/Room.vala	2013-09-13 14:24:42 +0000
@@ -51,8 +51,6 @@
     internal Gtk.Entry text_entry;
     Utils.AutoScrolled left_scrolled;
 
-    internal Cable.Collections.RRStringList history_set;
-
     Gtk.Box user_box;
 
     public Widgets.Topic topic_bar;
@@ -75,9 +73,7 @@
         set { text_entry.text = value; }
     }
 
-    public Cable.Collections.RRStringList history {
-        get { return history_set; }
-    }
+    public Utils.MessageHistory history { get; private set; }
 
     State _state;
     public State state {
@@ -125,7 +121,7 @@
     public Room (string channel, string nick="") {
         this.channel = channel;
 
-        history_set = new Cable.Collections.RRStringList();
+        history = new Utils.MessageHistory ();
 
         init_layout ();
         connect_callbacks ();
@@ -243,26 +239,23 @@
                     }
                     return true;
                 }
-            }
-            else if ( ev.keyval == Gdk.Key.Up ) {
-                if ( history.previous() )
-                    text_entry.text = history.current();
-                return true;
-            }
-            else if ( ev.keyval == Gdk.Key.Down ) {
-                if ( history.next() )
-                    text_entry.text = history.current();
-                return true;
-            }
+            } else if (ev.keyval == Gdk.Key.Up) {
+                if (history.has_older ())
+                    text_entry.text = history.get_older ();
+                return true;
+            } else if (ev.keyval == Gdk.Key.Down) {
+                if (history.has_newer ())
+                    text_entry.text = history.get_newer ();
+                return true;
+            }
+
             return false;
         });
 
         text_entry.activate.connect (() => {
             if (text_entry.text != "") {
-
-                // permit duplicates
-                history.add(text_entry.text);
-                send_message (text_entry.text); // signal
+                history.append (text_entry.text);
+                send_message (text_entry.text);
             }
         });
 

=== modified file 'tests/TestMessageHistory.vala'
--- tests/TestMessageHistory.vala	2013-09-03 20:19:11 +0000
+++ tests/TestMessageHistory.vala	2013-09-13 14:24:42 +0000
@@ -1,41 +1,95 @@
 class TestMessageHistory : Gee.TestCase {
 
-  public TestMessageHistory() {
-    // assign a name for this class
-    base("TestMessageHistory");
-    // add test methods
-    add_test("[History] Navigate history", test_navigate);
-    add_test("[History] Limited capacity", test_capacity);
-   }
-
-   public override void set_up () {
-     // setup your test
-   }
-
-   public void test_navigate() {
-     var history = new Cable.Collections.RRStringList();
-     history.add("one");
-     history.add("two");
-     history.add("three");
-     assert(history.current() == "three");
-     assert(history.next() == false);
-     assert(history.current() == "three");
-     assert(history.previous() == true);
-     assert(history.current() == "two");
-   }
-
-   public void test_capacity() {
-     var history = new Cable.Collections.RRStringList.with_capacity(2);
-     history.add("one");
-     history.add("two");
-     history.add("three");
-     assert(history.current() == "three");
-     assert(history.previous() == true);
-     assert(history.previous() == false);
-     assert(history.current() == "two");
-   }
-
-   public override void tear_down () {
-     // tear down your test
-   }
-}
\ No newline at end of file
+    public TestMessageHistory () {
+        base("Message History");
+
+        add_test("Initial State", test_initial_state);
+        add_test("First Insert", test_first_insert);
+        add_test("Second Insert", test_second_insert);
+        add_test("Navigate Back", test_navigate_back);
+        add_test("Navigate Forth", test_navigate_forth);
+        add_test("Reset After Insert", test_reset_after_insert);
+        add_test("Modify Current", test_modify_current);
+    }
+
+    public override void set_up () {
+        // setup your test
+    }
+
+    public void test_initial_state () {
+        var history = new Cable.Utils.MessageHistory ();
+
+        assert (history.has_older () == false);
+        assert (history.has_newer () == false);
+        assert (history.get_current () == "");
+    }
+
+    public void test_first_insert () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+
+        assert (history.get_current () == "");
+        assert (history.has_newer () == false);
+        assert (history.has_older () == true);
+    }
+    
+    public void test_second_insert () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+        history.append ("two");
+        
+        assert (history.get_current () == "");
+        assert (history.has_newer () == false);
+        assert (history.has_older () == true);
+    }
+    
+    public void test_navigate_back () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+        
+        assert (history.get_older () == "one");
+        assert (history.get_current () == "one");
+        assert (history.has_newer () == true);
+    }
+    
+    public void test_navigate_forth () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+        history.get_older ();
+        
+        assert (history.get_current () == "one");
+        assert (history.get_newer () == "");
+        assert (history.has_older () == true);
+        assert (history.has_newer () == false);
+    }
+    
+    public void test_reset_after_insert () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+        history.get_older ();
+        history.append ("two");
+        
+        assert (history.get_current () == "");
+        assert (history.has_newer () == false);
+        assert (history.has_older () == true);
+    }
+    
+    public void test_modify_current () {
+        var history = new Cable.Utils.MessageHistory ();
+        history.append ("one");
+        history.get_older ();
+        
+        assert (history.get_current () == "one");
+        assert (history.has_newer () == true);
+        assert (history.has_older () == false);
+        
+        history.modify_current ("modified");
+        assert (history.get_current () == "modified");
+        assert (history.has_newer () == true);
+        assert (history.has_older () == false);
+    }
+
+    public override void tear_down () {
+        // tear down your test
+    }
+}