Editable: improve column parsing. Fixes #982 & #979

Also replace jQuery each functions

docs/example-widget-editable.html

@@ -124,6 +124,7 @@ td.editable_updated {
 		<ul>
 			<li>This widget can not be applied to the original plugin and requires jQuery 1.7+ and a browser that supports <a href="http://caniuse.com/#feat=contenteditable"><code>contenteditable</code> attributes</a> (almost all modern browsers).</li>
 			<li><span class="label warning">Important</span>: In Internet Explorer, please wrap the cell content with a DIV or SPAN as <a href="http://msdn.microsoft.com/en-us/library/ie/ms533690(v=vs.85).aspx">it is not possible to make table cells directly contenteditable</a>. Wrapping the content in the markup is much more efficient than using javascript to do it for you (especially in IE).<br><br></li>
+			<li>In <span class="version">v2.23.0</span>, the <code>editable_columns</code> option will now accept a string with both ranges and single columns, e.g. <code>'1,3-5,7'</code>.</li>
 			<li>In <span class="version">v2.22.2</span>,
 				<ul>
 					<li><kbd>Shift</kbd>+<kbd>Enter</kbd> can now be used to start a new line even if <code>editable_enterToAccept</code> is <code>true</code>.</li>
@@ -232,10 +233,11 @@ td.editable_updated {
 
 					<tr id="editable-columns">
 						<td><a href="#" class="permalink">editable_columns</a></td>
-						<td>Contains an array (or range string) of columns numbers you want to have editable content (zero-based index).
+						<td>Contains an array (or range string) of columns numbers you want to have editable content (zero-based index) (<span class="version updated">v2.23.0</span>).
 							<div class="collapsible">
 							<ul>
-								<li>In tablesorter core <span class="version">v2.14.2</span>, this widget was updated to allow passing a range string in this option, i.e. <code>"0-2"</code> instead of <code>[0,1,2]</code>.</li>
+								<li>In <span class="version">v2.23.0</span>, this option will now accept both range &amp; comma separated values, e.g. <code>'1,3-5,7'</code>.</li>
+								<li>In tablesorter core <span class="version">v2.14.2</span>, this widget was updated to allow passing a range string in this option, i.e. <code>'0-2'</code> instead of <code>[0,1,2]</code>.</li>
 								<li><code>contenteditable="true"</code> is added to cells within these columns.</li>
 							</ul>
 							Default value: <code>[]</code> (empty array)

js/widgets/widget-editable.js

@@ -45,38 +45,43 @@
 		},
 
 		getColumns : function( c, wo ) {
-			var indx, tmp,
+			var list, indx, range, len, tmp,
-				colIndex = [],
+				editableColumns = wo.editable_columns,
 				cols = [];
-			if ( !wo.editable_columnsArray && $.type( wo.editable_columns ) === 'string' && wo.editable_columns.indexOf( '-' ) >= 0 ) {
+			if ( typeof editableColumns === 'string' ) {
-				// editable_columns can contain a range string ( i.e. '2-4' )
+				// editable_columns can contain a range string, or comma separated values (e.g. '1,2-4,7')
-				tmp = wo.editable_columns.split( /\s*-\s*/ );
+				list = editableColumns.replace( /\s+/, '' ).split( /,/ );
-				indx = parseInt( tmp[ 0 ], 10 ) || 0;
+				len = list.length - 1;
-				tmp = parseInt( tmp[ 1 ], 10 ) || ( c.columns - 1 );
+				while ( len >= 0 ) {
-				if ( tmp > c.columns ) {
+					if ( list[ len ].indexOf( '-' ) >= 0 ) {
-					tmp = c.columns - 1;
+						range = list[ len ].split( '-' );
+						indx = parseInt( range[ 0 ], 10 ) || 0;
+						range = parseInt( range[ 1 ], 10) || c.columns - 1;
+						if ( indx > range ) {
+							// in case someone does '5-3'
+							tmp = indx; indx = range; range = tmp;
 						}
-				for ( ; indx <= tmp; indx++ ) {
+						for ( ; indx <= range; indx++ ) {
-					colIndex.push( indx );
 							cols.push( 'td:nth-child(' + ( indx + 1 ) + ')' );
 						}
-			} else if ( $.isArray( wo.editable_columns ) ) {
+					} else {
-				$.each( wo.editable_columnsArray || wo.editable_columns, function( i, col ) {
+						cols.push( 'td:nth-child(' + ( ( parseInt( list[ len ], 10 ) || 0 ) + 1 ) + ')' );
-					if ( col < c.columns ) {
+					}
-						colIndex.push( col );
+					len--;
-						cols.push( 'td:nth-child(' + ( col + 1 ) + ')' );
+				}
+			} else if ( $.isArray( editableColumns ) ) {
+				len = editableColumns.length;
+				for ( indx = 0; indx < len; indx++ ) {
+					if ( editableColumns[ indx ] < c.columns ) {
+						cols.push( 'td:nth-child(' + ( editableColumns[ indx ] + 1 ) + ')' );
 					}
-				});
 				}
-			if ( !wo.editable_columnsArray ) {
-				wo.editable_columnsArray = colIndex;
-				wo.editable_columnsArray.sort(function(a, b){ return a - b; });
 			}
 			return cols;
 		},
 
 		update: function( c, wo ) {
-			var $t,
+			var $t, $cells, cellIndex, cellLen, $editable, editableIndex, editableLen,
 				tmp = $( '<div>' ).wrapInner( wo.editable_wrapContent ).children().length || $.isFunction( wo.editable_wrapContent ),
 				cols = tse.getColumns( c, wo ).join( ',' );
 
@@ -86,24 +91,28 @@
 
 			// IE does not allow making TR/TH/TD cells directly editable ( issue #404 )
 			// so add a div or span inside ( it's faster than using wrapInner() )
-			c.$tbodies.find( cols ).not( '.' + wo.editable_noEdit ).each( function() {
+			$cells = c.$tbodies.find( cols ).not( '.' + wo.editable_noEdit );
+			cellLen = $cells.length;
+			for ( cellIndex = 0; cellIndex < cellLen; cellIndex++ ) {
+				/*jshint loopfunc:true */
 				// test for children, if they exist, then make the children editable
-				$t = $( this );
+				$t = $cells.eq( cellIndex );
-
 				if ( tmp && $t.children( 'div, span' ).length === 0 ) {
 					$t.wrapInner( wo.editable_wrapContent );
 				}
-				if ( $t.children( 'div, span' ).length ) {
+				$editable = $t.children( 'div, span' ).not( '.' + wo.editable_noEdit );
+				editableLen = $editable.length;
+				if ( editableLen ) {
 					// make div/span children content editable
-					$t.children( 'div, span' ).not( '.' + wo.editable_noEdit ).each( function() {
+					for ( editableIndex = 0; editableIndex < editableLen; editableIndex++ ) {
-						var $this = $( this );
+						var $this = $editable.eq( editableIndex );
 						if ( wo.editable_trimContent ) {
 							$this.html( function( i, txt ) {
 								return $.trim( txt );
 							});
 						}
 						$this.prop( 'contenteditable', true );
-					});
+					}
 				} else {
 					if ( wo.editable_trimContent ) {
 						$t.html( function( i, txt ) {
@@ -112,7 +121,7 @@
 					}
 					$t.prop( 'contenteditable', true );
 				}
-			});
+			}
 		},
 
 		bindEvents: function( c, wo ) {