From 330d2b079d3e8eb79c77725491635c17377a8bae Mon Sep 17 00:00:00 2001 From: Luiza Pagliari Date: Tue, 8 Sep 2015 11:55:36 -0300 Subject: [PATCH] Fix 2772. Skipping line marker when applying attribs to a range --- src/static/js/AttributeManager.js | 153 ++++++++++++++++++++---------- src/static/js/ace2_inner.js | 3 + 2 files changed, 108 insertions(+), 48 deletions(-) diff --git a/src/static/js/AttributeManager.js b/src/static/js/AttributeManager.js index 3f464908..d4d20a1f 100644 --- a/src/static/js/AttributeManager.js +++ b/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 - - an array `alines` containing 1 attributes string for each line + + + A document representation contains + - 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()); - ChangesetUtils.buildKeepToStartOfRange(this.rep, builder, start); - ChangesetUtils.buildKeepRange(this.rep, builder, start, end, attribs, this.rep.apool); - return this.applyChangeset(builder); + // instead of applying the attributes to the whole range at once, we need to apply them + // line by line, to be able to disregard the "*" used as line marker. For more details, + // see https://github.com/ether/etherpad-lite/issues/2772 + 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); - + } }); diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 10dd0e4c..d1657a7c 100644 --- a/src/static/js/ace2_inner.js +++ b/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) {