fix code quality. if user presses Cancel in Status change dialog do not set the new status; add comments to code, make it cleaner (use 'SEPARATOR' and not '' for string for combobox; HIG (add ... to finish menuitems that need it and add a missing mnemonic)

src/dialogs.py

@@ -239,16 +239,17 @@ class ChangeStatusMessageDialog:
 		self.window.show_all()
 
 	def run(self):
-		'''Wait for OK button to be pressed and return status messsage'''
+		'''Wait for OK or Cancel button to be pressed and return status messsage
+		(None if users pressed Cancel or x button of WM'''
 		rep = self.window.run()
 		if rep == gtk.RESPONSE_OK:
 			beg, end = self.message_buffer.get_bounds()
-			message = self.message_buffer.get_text(beg, end, 0).decode('utf-8').strip()
+			message = self.message_buffer.get_text(beg, end).decode('utf-8').strip()
 			msg = helpers.to_one_line(message)
 			if self.show:
 				gajim.config.set('last_status_msg_' + self.show, msg)
 		else:
-			message = -1
+			message = None # user pressed Cancel button or X wm button
 		self.window.destroy()
 		return message
 
@@ -361,7 +362,7 @@ _('Contact names must be of the form "user@servername".')).get_response()
 		message_buffer = self.xml.get_widget('message_textview').get_buffer()
 		start_iter = message_buffer.get_start_iter()
 		end_iter = message_buffer.get_end_iter()
-		message = message_buffer.get_text(start_iter, end_iter, 0).decode('utf-8')
+		message = message_buffer.get_text(start_iter, end_iter).decode('utf-8')
 		group = self.group_comboboxentry.child.get_text().decode('utf-8')
 		self.plugin.roster.req_sub(self, jid, message, self.account,
 			group = group, pseudo = nickname)

src/roster_window.py

@@ -1008,7 +1008,8 @@ class RosterWindow:
 		show = gajim.SHOW_LIST[gajim.connections[account].connected]
 		dlg = dialogs.ChangeStatusMessageDialog(self.plugin, show)
 		message = dlg.run()
-		self.send_status(account, show, message)
+		if message is not None: # None if user pressed Cancel
+			self.send_status(account, show, message)
 
 	def mk_menu_account(self, event, iter):
 		'''Make account's popup menu'''
@@ -1421,9 +1422,10 @@ _('If "%s" accepts this request you will know his status.') %jid)
 				return
 			dlg = dialogs.ChangeStatusMessageDialog(self.plugin)
 			message = dlg.run()
-			for acct in accounts:
+			if message is not None: # None if user pressed Cancel
-				show = gajim.SHOW_LIST[gajim.connections[acct].connected]
+				for acct in accounts:
-				self.send_status(acct, show, message)
+					show = gajim.SHOW_LIST[gajim.connections[acct].connected]
+					self.send_status(acct, show, message)
 		return
 		if status == 'invisible':
 			bug_user = False
@@ -2266,7 +2268,7 @@ _('If "%s" accepts this request you will know his status.') %jid)
 		gtkgui_helpers.set_unset_urgency_hint(self.window, self.nb_unread)
 
 	def iter_is_separator(self, model, iter):
-		if not model[iter][0]:
+		if model[iter][0] == 'SEPARATOR':
 			return True
 		return False
 
@@ -2312,7 +2314,7 @@ _('If "%s" accepts this request you will know his status.') %jid)
 			self.transports_state_images[transport] = self.load_iconset(
 				folder + '/16x16/')
 
-		liststore = gtk.ListStore(str, gtk.Image, str)
+		liststore = gtk.ListStore(str, gtk.Image, str) # uf_show, img, show
 		self.status_combobox = self.xml.get_widget('status_combobox')
 
 		cell = cell_renderer_image.CellRendererImage()
@@ -2329,15 +2331,15 @@ _('If "%s" accepts this request you will know his status.') %jid)
 		for show in ['online', 'chat', 'away', 'xa', 'dnd', 'invisible']:
 			uf_show = helpers.get_uf_show(show)
 			liststore.append([uf_show, self.jabber_state_images[show], show])
-		# Add a Separator
+		# Add a Separator (self.iter_is_separator() checks on string SEPARATOR)
-		liststore.append(['', None, ''])
+		liststore.append(['SEPARATOR', None, ''])
 
 		path = os.path.join(gajim.DATA_DIR, 'pixmaps', 'rename.png')
 		img = gtk.Image()
 		img.set_from_file(path)
-		liststore.append([_('Change Status Message'), img, ''])
+		liststore.append([_('_Change Status Message...'), img, ''])
-		# Add a Separator
+		# Add a Separator (self.iter_is_separator() checks on string SEPARATOR)
-		liststore.append(['', None, ''])
+		liststore.append(['SEPARATOR', None, ''])
 
 		uf_show = helpers.get_uf_show('offline')
 		liststore.append([uf_show, self.jabber_state_images['offline'],

src/systray.py

@@ -141,7 +141,7 @@ class Systray:
 		item = gtk.SeparatorMenuItem()
 		sub_menu.append(item)
 
-		item = gtk.ImageMenuItem(_('_Change Status Message'))
+		item = gtk.ImageMenuItem(_('_Change Status Message...'))
 		path = os.path.join(gajim.DATA_DIR, 'pixmaps', 'rename.png')
 		img = gtk.Image()
 		img.set_from_file(path)
@@ -315,10 +315,11 @@ class Systray:
 	def on_change_status_message_activate(self, widget):
 		dlg = dialogs.ChangeStatusMessageDialog(self.plugin)
 		message = dlg.run()
-		accounts = gajim.connections.keys()
+		if message is not None: # None if user press Cancel
-		for acct in accounts:
+			accounts = gajim.connections.keys()
-			show = gajim.SHOW_LIST[gajim.connections[acct].connected]
+			for acct in accounts:
-			self.plugin.roster.send_status(acct, show, message)
+				show = gajim.SHOW_LIST[gajim.connections[acct].connected]
+				self.plugin.roster.send_status(acct, show, message)
 
 	def show_tooltip(self, widget):
 		position = widget.window.get_origin()