Merge pull request #2777 from storytouch/fix2772

Fix 2772. Skipping line marker when applying attribs to a range

src/static/js/AttributeManager.js

@@ -4,23 +4,23 @@ var _ = require('./underscore');
 
 var lineMarkerAttribute = 'lmkr';
 
-// If one of these attributes are set to the first character of a 
+// If one of these attributes are set to the first character of a
 // line it is considered as a line attribute marker i.e. attributes
-// set on this marker are applied to the whole line. 
+// set on this marker are applied to the whole line.
 // The list attribute is only maintained for compatibility reasons
 var lineAttributes = [lineMarkerAttribute,'list'];
 
 /*
-  The Attribute manager builds changesets based on a document 
+  The Attribute manager builds changesets based on a document
   representation for setting and removing range or line-based attributes.
-  
+
   @param rep the document representation to be used
-  @param applyChangesetCallback this callback will be called 
+  @param applyChangesetCallback this callback will be called
     once a changeset has been built.
-    
+
-    
+
-  A document representation contains 
+  A document representation contains
-  - an array `alines` containing 1 attributes string for each line 
+  - an array `alines` containing 1 attributes string for each line
   - an Attribute pool `apool`
   - a SkipList `lines` containing the text lines of the document.
 */
@@ -30,7 +30,7 @@ var AttributeManager = function(rep, applyChangesetCallback)
   this.rep = rep;
   this.applyChangesetCallback = applyChangesetCallback;
   this.author = '';
-  
+
   // If the first char in a line has one of the following attributes
   // it will be considered as a line marker
 };
@@ -38,49 +38,106 @@ var AttributeManager = function(rep, applyChangesetCallback)
 AttributeManager.lineAttributes = lineAttributes;
 
 AttributeManager.prototype = _(AttributeManager.prototype).extend({
-  
+
   applyChangeset: function(changeset){
     if(!this.applyChangesetCallback) return changeset;
-    
+
     var cs = changeset.toString();
     if (!Changeset.isIdentity(cs))
     {
       this.applyChangesetCallback(cs);
     }
-    
+
     return changeset;
   },
-  
+
   /*
     Sets attributes on a range
     @param start [row, col] tuple pointing to the start of the range
     @param end [row, col] tuple pointing to the end of the range
-    @param attribute: an array of attributes
+    @param attribs: an array of attributes
   */
   setAttributesOnRange: function(start, end, attribs)
   {
-    var builder = Changeset.builder(this.rep.lines.totalWidth());
+    // instead of applying the attributes to the whole range at once, we need to apply them
-    ChangesetUtils.buildKeepToStartOfRange(this.rep, builder, start);
+    // line by line, to be able to disregard the "*" used as line marker. For more details,
-    ChangesetUtils.buildKeepRange(this.rep, builder, start, end, attribs, this.rep.apool);
+    // see https://github.com/ether/etherpad-lite/issues/2772
-    return this.applyChangeset(builder);
+    var allChangesets;
+    for(var row = start[0]; row <= end[0]; row++) {
+      var rowRange = this._findRowRange(row, start, end);
+      var startCol = rowRange[0];
+      var endCol   = rowRange[1];
+
+      var rowChangeset = this._setAttributesOnRangeByLine(row, startCol, endCol, attribs);
+
+      // compose changesets of all rows into a single changeset, as the range might not be continuous
+      // due to the presence of line markers on the rows
+      if (allChangesets) {
+        allChangesets = Changeset.compose(allChangesets.toString(), rowChangeset.toString(), this.rep.apool);
+      } else {
+        allChangesets = rowChangeset;
+      }
+    }
+
+    return this.applyChangeset(allChangesets);
   },
 
-  /* 
+  _findRowRange: function(row, start, end)
+  {
+    var startCol, endCol;
+
+    var startLineOffset = this.rep.lines.offsetOfIndex(row);
+    var endLineOffset   = this.rep.lines.offsetOfIndex(row+1);
+    var lineLength      = endLineOffset - startLineOffset;
+
+    // find column where range on this row starts
+    if (row === start[0]) { // are we on the first row of range?
+      startCol = start[1];
+    } else {
+      startCol = this.lineHasMarker(row) ? 1 : 0; // remove "*" used as line marker
+    }
+
+    // find column where range on this row ends
+    if (row === end[0]) { // are we on the last row of range?
+      endCol = end[1]; // if so, get the end of range, not end of row
+    } else {
+      endCol = lineLength - 1; // remove "\n"
+    }
+
+    return [startCol, endCol];
+  },
+
+  /*
+    Sets attributes on a range, by line
+    @param row the row where range is
+    @param startCol column where range starts
+    @param endCol column where range ends
+    @param attribs: an array of attributes
+  */
+  _setAttributesOnRangeByLine: function(row, startCol, endCol, attribs)
+  {
+    var builder = Changeset.builder(this.rep.lines.totalWidth());
+    ChangesetUtils.buildKeepToStartOfRange(this.rep, builder, [row, startCol]);
+    ChangesetUtils.buildKeepRange(this.rep, builder, [row, startCol], [row, endCol], attribs, this.rep.apool);
+    return builder;
+  },
+
+  /*
     Returns if the line already has a line marker
     @param lineNum: the number of the line
   */
   lineHasMarker: function(lineNum){
     var that = this;
-    
+
     return _.find(lineAttributes, function(attribute){
-      return that.getAttributeOnLine(lineNum, attribute) != ''; 
+      return that.getAttributeOnLine(lineNum, attribute) != '';
     }) !== undefined;
   },
-  
+
   /*
     Gets a specified attribute on a line
     @param lineNum: the number of the line to set the attribute for
-    @param attributeKey: the name of the attribute to get, e.g. list  
+    @param attributeKey: the name of the attribute to get, e.g. list
   */
   getAttributeOnLine: function(lineNum, attributeName){
     // get  `attributeName` attribute of first char of line
@@ -95,10 +152,10 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     }
     return '';
   },
-  
+
   /*
     Gets all attributes on a line
-    @param lineNum: the number of the line to get the attribute for 
+    @param lineNum: the number of the line to get the attribute for
   */
   getAttributesOnLine: function(lineNum){
     // get attributes of first char of line
@@ -112,7 +169,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
       {
         op = opIter.next()
         if(!op.attribs) return []
-        
+
         Changeset.eachAttribNumber(op.attribs, function(n) {
           attributes.push([this.rep.apool.getAttribKey(n), this.rep.apool.getAttribValue(n)])
         }.bind(this))
@@ -121,33 +178,33 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     }
     return [];
   },
-  
+
   /*
     Gets all attributes at a position containing line number and column
     @param lineNumber starting with zero
     @param column starting with zero
-    returns a list of attributes in the format 
+    returns a list of attributes in the format
     [ ["key","value"], ["key","value"], ...  ]
   */
   getAttributesOnPosition: function(lineNumber, column){
     // get all attributes of the line
     var aline = this.rep.alines[lineNumber];
-    
+
     if (!aline) {
         return [];
     }
     // iterate through all operations of a line
     var opIter = Changeset.opIterator(aline);
-    
+
     // we need to sum up how much characters each operations take until the wanted position
     var currentPointer = 0;
-    var attributes = [];    
+    var attributes = [];
     var currentOperation;
-    
+
     while (opIter.hasNext()) {
       currentOperation = opIter.next();
-      currentPointer = currentPointer + currentOperation.chars;      
+      currentPointer = currentPointer + currentOperation.chars;
-      
+
       if (currentPointer > column) {
         // we got the operation of the wanted position, now collect all its attributes
         Changeset.eachAttribNumber(currentOperation.attribs, function (n) {
@@ -156,44 +213,44 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
             this.rep.apool.getAttribValue(n)
           ]);
         }.bind(this));
-        
+
         // skip the loop
         return attributes;
       }
     }
     return attributes;
-    
+
   },
-  
+
   /*
-    Gets all attributes at caret position 
+    Gets all attributes at caret position
     if the user selected a range, the start of the selection is taken
-    returns a list of attributes in the format 
+    returns a list of attributes in the format
     [ ["key","value"], ["key","value"], ...  ]
   */
   getAttributesOnCaret: function(){
     return this.getAttributesOnPosition(this.rep.selStart[0], this.rep.selStart[1]);
   },
-  
+
   /*
     Sets a specified attribute on a line
     @param lineNum: the number of the line to set the attribute for
     @param attributeKey: the name of the attribute to set, e.g. list
     @param attributeValue: an optional parameter to pass to the attribute (e.g. indention level)
-  
+
   */
   setAttributeOnLine: function(lineNum, attributeName, attributeValue){
     var loc = [0,0];
     var builder = Changeset.builder(this.rep.lines.totalWidth());
     var hasMarker = this.lineHasMarker(lineNum);
-    
+
     ChangesetUtils.buildKeepRange(this.rep, builder, loc, (loc = [lineNum, 0]));
 
     if(hasMarker){
       ChangesetUtils.buildKeepRange(this.rep, builder, loc, (loc = [lineNum, 1]), [
         [attributeName, attributeValue]
       ], this.rep.apool);
-    }else{      
+    }else{
         // add a line marker
         builder.insert('*', [
           ['author', this.author],
@@ -202,10 +259,10 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
           [attributeName, attributeValue]
         ], this.rep.apool);
     }
-    
+
     return this.applyChangeset(builder);
   },
-  
+
  /**
    * Removes a specified attribute on a line
    *  @param lineNum the number of the affected line
@@ -243,7 +300,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
 
    return this.applyChangeset(builder);
  },
-  
+
    /*
      Toggles a line attribute for the specified line number
      If a line attribute with the specified name exists with any value it will be removed
@@ -256,7 +313,7 @@ AttributeManager.prototype = _(AttributeManager.prototype).extend({
     return this.getAttributeOnLine(lineNum, attributeName) ?
       this.removeAttributeOnLine(lineNum, attributeName) :
       this.setAttributeOnLine(lineNum, attributeName, attributeValue);
-    
+
   }
 });
 

src/static/js/ace2_inner.js

@@ -2423,6 +2423,9 @@ function Ace2Inner(){
       var opIter = Changeset.opIterator(rep.alines[n]);
       var indexIntoLine = 0;
       var selectionStartInLine = 0;
+      if (documentAttributeManager.lineHasMarker(n)) {
+        selectionStartInLine = 1; // ignore "*" used as line marker
+      }
       var selectionEndInLine = rep.lines.atIndex(n).text.length; // exclude newline
       if (n == selStartLine)
       {