Merge lp:~oppifjellet/cable/message-history into lp:cable

[Daniele "OpenNingia" Simonetti]

Implemented message history

This branch fixes this bug https://bugs.launchpad.net/cable/+bug/1198635
Every channel now keeps an history of sent messages and commands that are navigable using arrow keys ( Up&Down )

For performance reasons the history capacity for each channel is 25 items. Old items are replaced with round robin logic.

[Eduard Gotwig (gotwig)]

sometimes I have to double press up or down, for it to work. Can this be fixed?

Thanks for your work.

[lp:~oppifjellet/cable/message-history]

126. By Daniele "OpenNingia" Simonetti on 2013-08-27

improved history navigation behavior

[Daniele "OpenNingia" Simonetti (oppifjellet)]

Revision 126 should fix the annoying behavior.
You're welcome, thank you for Cable.

[982c80311320c1b (alexander-wilms)]

It doesn't work as expected for me.
When I type
1 <Enter>
2 <Enter>
3 <Enter>

and then browse through history with the <up> key, the following messages appear in this order: 2, 1. When I then press the <down> key, those messages appear: 1, 2, 3

If I _instead_ (not after browing them with <up> and <down>) press the <down> key, 3 appears in the text box.

[Daniele "OpenNingia" Simonetti (oppifjellet)]

I admit it's a strange behavior. If it were you what's the best behavior?
Il giorno 30/ago/2013 00:57, "Alexander Wilms" <email address hidden>
ha scritto:

> It doesn't work as expected for me.
> When I type
> 1 <Enter>
> 2 <Enter>
> 3 <Enter>
>
> and then browse through history with the <up> key, the following messages
> appear in this order: 2, 1. When I then press the <down> key, those
> messages appear: 1, 2, 3
>
> If I _instead_ (not after browing them with <up> and <down>) press the
> <down> key, 3 appears in the text box.
> --
> https://code.launchpad.net/~oppifjellet/cable/message-history/+merge/182453
> You are the owner of lp:~oppifjellet/cable/message-history.
>

[Daniele "OpenNingia" Simonetti (oppifjellet)]

So I checked the implementation:

pressing <down><up> with the editbox empty ( or after sending a message ) is a shortcut to get the last typed message )

instead when navigating <up>/<down> will return the next and the previous message, with one exception:

. the history doesn't save duplicates

so if you write

1
2
3
2

the last "2" is ignored.

[982c80311320c1b (alexander-wilms)]

I think the most intuitive implementation (and IMHO the most simple one) would be this:

After typing:

1
2
3
2

Pressing <down> doesn't do anything. <Up> shows the last history entry (2) and every press of <up> shows the previous message. <Down> returns the next message.

Duplicates should not be skipped, since users don't know the algorithm behind it and at least I would expect it to list the messages in the order in which I pyed them.

[lp:~oppifjellet/cable/message-history]

127. By Daniele "OpenNingia" Simonetti on 2013-08-31

permit duplicates

[Daniele "OpenNingia" Simonetti (oppifjellet)]

Ok so, this revision implements your design. Duplicates are permitted, history default size has been increased to 50, and hitting <down> after typing an entry does nothing.

[982c80311320c1b (alexander-wilms)]

Great :)

[lp:~oppifjellet/cable/message-history]

128. By Daniele "OpenNingia" Simonetti on 2013-09-03

add unit test using GLib.Test

[Eduard Gotwig (gotwig)]

You shouldnt propose merge requests, which are based on other merge requests. Each merge request should be clean applied with it changes on trunk.

However, I am going to look today at the other merge request of you, which included this one.

Cheers,

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2013-08-06 15:32:12 +0000
+++ CMakeLists.txt	2013-09-03 20:19:34 +0000
@@ -55,6 +55,7 @@
     src/Utils/EmbeddedAlert.vala
     src/Utils/Identity.vala
     src/Utils/Network.vala
+    src/Utils/RRStringList.vala
 
     src/Irc/Maki.vala
     src/Irc/Irc.vala
@@ -90,7 +91,7 @@
     src/Dialogs/Preferences.vala
     src/Dialogs/ManageIdentity.vala
     src/Dialogs/ManageNetwork.vala
-    
+
     src/Tests/Tests.vala
     src/Tests/Utils.vala
 
@@ -109,9 +110,29 @@
     --verbose
     )
 
+vala_precompile(VALA_C_TESTS
+    tests/TestCase.vala
+    tests/TestMain.vala
+    tests/TestExample.vala
+    tests/TestMessageHistory.vala
+    src/Utils/RRStringList.vala
+PACKAGES
+    gtk+-3.0
+    granite
+    libnotify
+    unity
+OPTIONS
+    --target-glib=2.32
+    --vapidir=${CMAKE_CURRENT_SOURCE_DIR}/vapi/
+    --enable-experimental
+    --fatal-warnings
+    --verbose
+)
+
 add_subdirectory (po)
 add_subdirectory (data)
 
 add_executable(cable ${VALA_C})
+add_executable(cable_tests ${VALA_C_TESTS})
 
 install(TARGETS cable RUNTIME DESTINATION bin)
 
=== added file 'src/Utils/RRStringList.vala'
--- src/Utils/RRStringList.vala	1970-01-01 00:00:00 +0000
+++ src/Utils/RRStringList.vala	2013-09-03 20:19:34 +0000
@@ -0,0 +1,77 @@
+/***
+    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-22 18:40:53 +0000
+++ src/Widgets/Room.vala	2013-09-03 20:19:34 +0000
@@ -51,6 +51,8 @@
     internal Gtk.Entry text_entry;
     Utils.AutoScrolled left_scrolled;
 
+    internal Cable.Collections.RRStringList history_set;
+
     Gtk.Box user_box;
 
     public Widgets.Topic topic_bar;
@@ -62,17 +64,21 @@
     public signal void rejoin ();
 
     public string channel { get; set; }
-    
+
     public string nick {
         get { return pseudo_label.label; }
         set { pseudo_label.label = value; }
     }
-    
+
     public string entry {
         get { return text_entry.text; }
         set { text_entry.text = value; }
     }
 
+    public Cable.Collections.RRStringList history {
+        get { return history_set; }
+    }
+
     State _state;
     public State state {
         get { return _state; }
@@ -104,7 +110,7 @@
             show_all ();
         }
     }
-    
+
     private Gee.Set <string> nicks {
         owned get {
             var _nicks = new Gee.HashSet <string> ();
@@ -119,6 +125,8 @@
     public Room (string channel, string nick="") {
         this.channel = channel;
 
+        history_set = new Cable.Collections.RRStringList();
+
         init_layout ();
         connect_callbacks ();
     }
@@ -227,7 +235,7 @@
                     return true;
                 } else {
                     text_entry.buffer.delete_text (text_entry.cursor_position - word.length, word.length);
-                    
+
                     if( text_entry.text != "" ){
                         text_entry.insert_at_cursor (completion[0] + " ");
                     } else {
@@ -236,12 +244,26 @@
                     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;
+            }
             return false;
         });
-    
+
         text_entry.activate.connect (() => {
-            if (text_entry.text != "")
+            if (text_entry.text != "") {
+
+                // permit duplicates
+                history.add(text_entry.text);
                 send_message (text_entry.text); // signal
+            }
         });
 
         action_join.activate.connect (() => {
@@ -281,7 +303,7 @@
         requires (!has_user (new_name))
         ensures (!has_user (old_name))
         ensures (has_user (new_name)) {
-        
+
         if (old_name == null) {
             add_user (new_name, type, true);
             return;
@@ -300,7 +322,7 @@
 
         var user = new User (name);
         user.selectable = false;
-            
+
         switch (type) {
             case UserType.OPERATOR:
                 operators.add (user);
@@ -324,7 +346,7 @@
     public bool remove_user (string name, string message, bool silent = false)
         requires (has_user (name))
         ensures (!has_user (name)) {
-        
+
         Granite.Widgets.SourceList.ExpandableItem[] lists = {voiced, operators, regular};
         foreach (var list in lists) {
             foreach (var child in list.children) {
 
=== added directory 'tests'
=== added file 'tests/TestCase.vala'
--- tests/TestCase.vala	1970-01-01 00:00:00 +0000
+++ tests/TestCase.vala	2013-09-03 20:19:34 +0000
@@ -0,0 +1,58 @@
+public abstract class Gee.TestCase : Object {
+
+	private GLib.TestSuite suite;
+	private Adaptor[] adaptors = new Adaptor[0];
+
+	public delegate void TestMethod ();
+
+	public TestCase (string name) {
+		this.suite = new GLib.TestSuite (name);
+	}
+
+	public void add_test (string name, owned TestMethod test) {
+		var adaptor = new Adaptor (name, (owned)test, this);
+		this.adaptors += adaptor;
+
+		this.suite.add (new GLib.TestCase (adaptor.name,
+		                                   adaptor.set_up,
+		                                   adaptor.run,
+		                                   adaptor.tear_down ));
+	}
+
+	public virtual void set_up () {
+	}
+
+	public virtual void tear_down () {
+	}
+
+	public GLib.TestSuite get_suite () {
+		return this.suite;
+	}
+
+	private class Adaptor {
+
+		public string name { get; private set; }
+		private TestMethod test;
+		private TestCase test_case;
+
+		public Adaptor (string name,
+		                owned TestMethod test,
+		                TestCase test_case) {
+			this.name = name;
+			this.test = (owned)test;
+			this.test_case = test_case;
+		}
+
+		public void set_up (void* fixture) {
+			this.test_case.set_up ();
+		}
+
+		public void run (void* fixture) {
+			this.test ();
+		}
+
+		public void tear_down (void* fixture) {
+			this.test_case.tear_down ();
+		}
+	}
+}
 \ No newline at end of file
 
=== added file 'tests/TestExample.vala'
--- tests/TestExample.vala	1970-01-01 00:00:00 +0000
+++ tests/TestExample.vala	2013-09-03 20:19:34 +0000
@@ -0,0 +1,22 @@
+class TestExample : Gee.TestCase {
+
+  public TestExample() {
+    // assign a name for this class
+    base("TestExample");
+    // add test methods
+    add_test("test_example", test_example);
+   }
+
+   public override void set_up () {
+     // setup your test
+   }
+
+   public void test_example() {
+     // add your expressions
+     // assert(expression);
+   }
+
+   public override void tear_down () {
+     // tear down your test
+   }
+}
 \ No newline at end of file
 
=== added file 'tests/TestMain.vala'
--- tests/TestMain.vala	1970-01-01 00:00:00 +0000
+++ tests/TestMain.vala	2013-09-03 20:19:34 +0000
@@ -0,0 +1,7 @@
+public static int main(string[] args) {
+  Test.init (ref args);
+  // add any of your test cases here
+  TestSuite.get_root().add_suite(new TestExample().get_suite());
+  TestSuite.get_root().add_suite(new TestMessageHistory().get_suite());
+  return Test.run ();
+}
 \ No newline at end of file
 
=== added file 'tests/TestMessageHistory.vala'
--- tests/TestMessageHistory.vala	1970-01-01 00:00:00 +0000
+++ tests/TestMessageHistory.vala	2013-09-03 20:19:34 +0000
@@ -0,0 +1,41 @@
+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