Merge lp:~jtyr/terminator/terminator into lp:terminator/trunk

[Jiri Tyr]

I have noticed that the Broadcast is set to "group" by default. I believe that this is wrong and it should be set to "off" by default.

[Stephen Boddy (stephen-j-boddy)]

Hi Jiri, as the current maintainer and original contributor of that area of code I disagree that the current behaviour is wrong. The groups are there to facilitate flexible broadcast modes. If you don't need to broadcast there's not much (if any) need to use groups. By default new terminals do not get a new group, so there should be no negative use case.

It would take a convincing counter-argument to make me consider changing the current default behaviour. A patch making this configurable (including GUI and man pages changes too) is more likely to be accepted than a change in the default behaviour.

review: Disapprove

[lp:~jtyr/terminator/terminator]

1495. By Jiri Tyr on 2014-03-07

Configurable broadcast group

[Jiri Tyr (jtyr)]

Thanks for your comments. I have implemented the configurable broadcast group in the revision 1495. Please let me know if it looks fine or if I need to change something.

[Jiri Tyr (jtyr)]

Please could you review this change again? It's been one month since the commit and still no comment.

review: Needs Resubmitting

[Jiri Tyr (jtyr)]

Please can I get any feedback on this?

review: Needs Resubmitting

[Stephen Boddy (stephen-j-boddy)]

After all this time I finally merged this. Two minor differences,
1) I changed it from broadcast_group to broadcast_defaut.
2) I removed the line that causes the current mode to change. It felt unexpected when it happened, and I prefer the principle of least surprise. Change is picked up on next run.

review: Approve

=== modified file 'doc/terminator_config.5'
--- doc/terminator_config.5	2013-02-16 00:10:45 +0000
+++ doc/terminator_config.5	2014-03-07 22:51:55 +0000
@@ -61,6 +61,10 @@
 If this is set to "hidden", the tab bar will not be shown. Note that hiding the tab bar is very confusing and not recommended.
 Default value: \fBtop\fR
 .TP
+.B broadcast_group
+Defines broadcast behavior.  Can be any of: all, group, off.
+Default value: \fBgroup\fR
+.TP
 .B close_button_on_tab \fR(boolean)
 If set to True, tabs will have a close button on them.
 Default value: \fBTrue\fR

=== modified file 'terminatorlib/config.py'
--- terminatorlib/config.py	2014-01-24 22:29:07 +0000
+++ terminatorlib/config.py	2014-03-07 22:51:55 +0000
@@ -83,6 +83,7 @@
             'window_state'          : 'normal',
             'borderless'            : False,
             'tab_position'          : 'top',
+            'broadcast_group'       : 'group',
             'close_button_on_tab'   : True,
             'hide_tabbar'           : False,
             'scroll_tabbar'         : False,

=== modified file 'terminatorlib/preferences.glade'
--- terminatorlib/preferences.glade	2013-09-04 20:56:43 +0000
+++ terminatorlib/preferences.glade	2014-03-07 22:51:55 +0000
@@ -256,6 +256,23 @@
       </row>
     </data>
   </object>
+  <object class="GtkListStore" id="BroadcastGroupListStore">
+    <columns>
+      <!-- column-name position -->
+      <column type="gchararray"/>
+    </columns>
+    <data>
+      <row>
+        <col id="0" translatable="yes">All</col>
+      </row>
+      <row>
+        <col id="0" translatable="yes">Group</col>
+      </row>
+      <row>
+        <col id="0" translatable="yes">None</col>
+      </row>
+    </data>
+  </object>
   <object class="GtkListStore" id="WindowStateListStore">
     <columns>
       <!-- column-name state -->
@@ -531,6 +548,42 @@
                           </packing>
                         </child>
                         <child>
+                          <object class="GtkLabel" id="label41">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <property name="label" translatable="yes">Broadcast group</property>
+                          </object>
+                          <packing>
+                            <property name="top_attach">18</property>
+                            <property name="bottom_attach">19</property>
+                            <property name="x_options"></property>
+                            <property name="y_options"></property>
+                          </packing>
+                        </child>
+                        <child>
+                          <object class="GtkComboBox" id="broadcastgroup">
+                            <property name="visible">True</property>
+                            <property name="can_focus">False</property>
+                            <property name="model">BroadcastGroupListStore</property>
+                            <property name="active">0</property>
+                            <signal name="changed" handler="on_broadcastgroup_changed" swapped="no"/>
+                            <child>
+                              <object class="GtkCellRendererText" id="cellrenderertext18"/>
+                              <attributes>
+                                <attribute name="text">0</attribute>
+                              </attributes>
+                            </child>
+                          </object>
+                          <packing>
+                            <property name="left_attach">1</property>
+                            <property name="right_attach">2</property>
+                            <property name="top_attach">18</property>
+                            <property name="bottom_attach">19</property>
+                            <property name="x_options"></property>
+                            <property name="y_options">GTK_EXPAND</property>
+                          </packing>
+                        </child>
+                        <child>
                           <object class="GtkLabel" id="label6">
                             <property name="visible">True</property>
                             <property name="can_focus">False</property>

=== modified file 'terminatorlib/prefseditor.py'
--- terminatorlib/prefseditor.py	2014-01-24 22:29:07 +0000
+++ terminatorlib/prefseditor.py	2014-03-07 22:51:55 +0000
@@ -248,6 +248,16 @@
         else:
             active = 0
         widget.set_active(active)
+        # Broadcast group
+        option = self.config['broadcast_group']
+        widget = guiget('broadcastgroup')
+        if option == 'all':
+            active = 0
+        elif option == 'off':
+            active = 2
+        else:
+            active = 1
+        widget.set_active(active)
         # scroll_tabbar
         widget = guiget('scrolltabbarcheck')
         widget.set_active(self.config['scroll_tabbar'])
@@ -1007,6 +1017,18 @@
         self.config['tab_position'] = value
         self.config.save()
 
+    def on_broadcastgroup_changed(self, widget):
+        """Broadcast group changed"""
+        selected = widget.get_active()
+        if selected == 0:
+            value = 'all'
+        elif selected == 2:
+            value = 'off'
+        else:
+            value = 'group'
+        self.config['broadcast_group'] = value
+        self.config.save()
+
     def on_winstatecombo_changed(self, widget):
         """Window state changed"""
         selected = widget.get_active()

=== modified file 'terminatorlib/terminator.py'
--- terminatorlib/terminator.py	2014-01-24 22:29:54 +0000
+++ terminatorlib/terminator.py	2014-03-07 22:51:55 +0000
@@ -56,10 +56,10 @@
             self.terminals = []
         if not self.groups:
             self.groups = []
-        if self.groupsend == None:
-            self.groupsend = self.groupsend_type['group']
         if not self.config:
             self.config = Config()
+        if self.groupsend == None:
+            self.groupsend = self.groupsend_type[self.config['broadcast_group']]
         if not self.keybindings:
             self.keybindings = Keybindings()
             self.keybindings.configure(self.config['keybindings'])
@@ -370,6 +370,9 @@
             if maker.isinstance(child, 'Notebook'):
                 child.configure()
 
+        # Set broadcasting group
+        self.groupsend = self.groupsend_type[self.config['broadcast_group']]
+
     def create_group(self, name):
         """Create a new group"""
         if name not in self.groups: