Merge lp:~jamalta/gwibber/298898 into lp:gwibber/1.2

Proposed by Jamal Fanaian on 2009-07-10
Status: Rejected
Rejected by: Ken VanDine on 2012-05-09
Proposed branch: lp:~jamalta/gwibber/298898
Merge into: lp:gwibber/1.2
Diff against target: None lines
To merge this branch: bzr merge lp:~jamalta/gwibber/298898
Reviewer Review Type Date Requested Status
Alexander Sack (community) 2009-07-10 Needs Fixing on 2009-07-12
Review via email: mp+8543@code.launchpad.net
To post a comment you must log in.
Alexander Sack (asac) wrote :

The implementation is not yet complete imho; at least we should do two things:

1. fix the case when the user starts typing while he is at some pos < len ? I think when that happens we should reset the pos to len so the changed text gets saved as the new "current" (topmost) text.

2. the history buffer shouldn't grow indefinitely; please constraint it to 20-50 messages; consider to make the history buffer size configurable.

review: Needs Fixing
Jamal Fanaian (jamalta) wrote :

> The implementation is not yet complete imho; at least we should do two things:
>
> 1. fix the case when the user starts typing while he is at some pos < len ? I
> think when that happens we should reset the pos to len so the changed text
> gets saved as the new "current" (topmost) text.
>
> 2. the history buffer shouldn't grow indefinitely; please constraint it to
> 20-50 messages; consider to make the history buffer size configurable.

I will work on a fix for the 2nd issue.

Regarding the 1st issue, are you talking about the user typing something when they move up the buffer (pos < len) and then move again, thus losing what they typed? I think if the buffer gets switched to "current" (pos = len), it might confuse the user as they are now at the end of the buffer without realising it. I can think of one alternative, which is to save changes to the position when the user moves from it. This would end up working as it works when scrolling up and down the bash history. Which way of doing it do you think is better? I have implemented the first method, but I don't think it is the best way of doing things.

Thanks for your input. I really appreciate it, as it is very helpful.

lp:~jamalta/gwibber/298898 updated on 2009-07-13
348. By Jamal Fanaian on 2009-07-13

Added a max limit to the messages history. When a message is sent the limit will be checked and the list will be sliced if necessary.

Jamal Fanaian (jamalta) wrote :

> The implementation is not yet complete imho; at least we should do two things:
>
> 1. fix the case when the user starts typing while he is at some pos < len ? I
> think when that happens we should reset the pos to len so the changed text
> gets saved as the new "current" (topmost) text.
>
> 2. the history buffer shouldn't grow indefinitely; please constraint it to
> 20-50 messages; consider to make the history buffer size configurable.

The last commit added saving of the buffer whenever the position is moved, so if the message is changed it will not be lost. This, to me, seemed to be the best way to implement this. Please comment if you feel otherwise and I will resolve it.

Thanks again for your time.

lp:~jamalta/gwibber/298898 updated on 2009-07-13
349. By Jamal Fanaian on 2009-07-13

Changed messages in buffer will now be saved

350. By Jamal Fanaian on 2009-07-13

Removed feature to modify the message history

Unmerged revisions

350. By Jamal Fanaian on 2009-07-13

Removed feature to modify the message history

349. By Jamal Fanaian on 2009-07-13

Changed messages in buffer will now be saved

348. By Jamal Fanaian on 2009-07-13

Added a max limit to the messages history. When a message is sent the limit will be checked and the list will be sliced if necessary.

347. By Jamal Fanaian on 2009-07-09

Created a message history list that gets appended on message send. Added a key_press_event to the input editor to detect for Ctrl+Up and Ctrl+Down and move through the message history.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'gwibber/client.py'
2--- gwibber/client.py 2009-06-21 01:17:53 +0000
3+++ gwibber/client.py 2009-07-09 17:38:10 +0000
4@@ -106,6 +106,10 @@
5 self.indicator_items = {}
6 layout = gtk.VBox()
7
8+ self.messages_history = []
9+ self.messages_history_pos = 0
10+ self.messages_history_cur = None
11+
12 if self.preferences["facebook_beta"]:
13 from microblog import facebookstream
14 microblog.PROTOCOLS["facebook"] = facebookstream
15@@ -209,6 +213,7 @@
16 self.input.connect("populate-popup", self.on_input_context_menu)
17 self.input.connect("activate", self.on_input_activate)
18 self.input.connect("changed", self.on_input_change)
19+ self.input.connect("key_press_event", self.on_input_key_press)
20 self.input.set_max_length(MAX_MESSAGE_LENGTH)
21
22 self.cancel_button = gtk.Button(_("Cancel"))
23@@ -663,6 +668,33 @@
24 self.statusbar.push(1, _("Characters remaining: %s") % (
25 self.input.get_max_length() - i_textlen))
26
27+ def on_input_key_press(self, w, e):
28+ def set_input_history(i):
29+ # Append current text
30+ if self.messages_history_pos == len(self.messages_history):
31+ self.messages_history_cur = self.input.get_text()
32+
33+ self.messages_history_pos += i
34+
35+ if self.messages_history_pos < 0:
36+ self.messages_history_pos = 0
37+
38+ if self.messages_history_pos >= len(self.messages_history):
39+ self.messages_history_pos = len(self.messages_history)
40+ text = self.messages_history_cur
41+ else:
42+ text = self.messages_history[self.messages_history_pos]
43+
44+ self.input.set_text(text)
45+ self.input.set_position(-1)
46+
47+ if e.keyval == gtk.keysyms.Up and e.state & gtk.gdk.CONTROL_MASK:
48+ set_input_history(-1)
49+ return True
50+ elif e.keyval == gtk.keysyms.Down and e.state & gtk.gdk.CONTROL_MASK:
51+ set_input_history(1)
52+ return True
53+
54 def on_theme_change(self, *args):
55 for tab in self.tabs:
56 view = tab.get_child()
57@@ -983,6 +1015,10 @@
58
59 def on_input_activate(self, e):
60 text = self.input.get_text().strip()
61+
62+ self.messages_history.append(text)
63+ self.messages_history_pos = len(self.messages_history)
64+
65 if text:
66 # don't allow submission if text length is greater than allowed
67 if len(text.decode("utf-8")) > MAX_MESSAGE_LENGTH:

Subscribers

People subscribed via source and target branches