The way we implemented undoing transactions was horrible. We'd process
the original transaction, but with a reversed=true flag. That meant we
had to keep track of the 'reversed' flag everywhere, and use ternaries
like insert = reversed ? op.remove : op.insert; all over the place to
access transaction operations. Redo then worked by reapplying the
transaction. We would verify that this was OK by tracking whether the
transaction was in an applied state or an undone state.
This commit makes it so every transaction can only be applied once. To
undo, you obtain a mirror image of the transaction with tx.reverse(),
then apply that. To redo, you clone the original transaction with
tx.clone() and apply that. All the code that had to use ternaries to
check whether the transaction was being applied in reverse or not is
gone now, because you can only apply a given transaction forwards,
never in reverse.
Bonus:
* Make ve.dm.Document's .completeHistory a simple array of
transactions, rather than transaction/boolean pairs
* In the protection of double application test, clone the example
document properly; it modified ve.dm.example.data, which was "fine"
because it ran .commit() and .rollback() the same number of times
Change-Id: I3050c5430be4a12510f22e20853560b92acebb67
Replaces newFromNodeReplacement(). newFromNodeReplacement was very
simplistic and didn't support metadata or internal list items, so
if you had comments or references inside of the data you were editing
(reference contents or an image caption), they'd get mangled.
With this, you can do:
newDoc = doc.getDocumentSlice( node );
// Edit newDoc
tx = ve.dm.Transaction.newFromDocumentReplace( doc, node, newDoc );
surface.change( newDoc );
and that takes care of metadata, internal list items, and things like
references that reference internal list items.
ve.dm.Document.js:
* In getDocumentSlice(), store a reference to the original document
and the number of items in its InternalList at the time of slicing
in the created slice. This is used for reconciliation when the
modified slice is injected back into the parent document with
newFromDocumentReplace().
ve.dm.InternalList.js:
* Add a method for merging in another InternalList. This provides a
mapping from old to new InternalList indexes so the linear model data
being injected by newFromDocumentReplace() can have its InternalList
indexes remapped.
ve.dm.Transaction.js:
* Replace newFromNodeReplacement() with newFromDocumentReplace()
ve.ui.MWMediaEditDialog.js, ve.ui.MWReferenceDialog.js:
* Use getDocumentSlice/newFromDocumentReplace for editing captions/refs
* Change insertion code path to insert an empty internalItem/caption, then
newFromDocumentReplace into that
* Add empty internalList to new mini-documents
ve/test/dm/ve.dm.Transaction.test.js:
* Replace newFromNodeReplacement tests with newFromDocumentReplace tests
ve-mw/test/dm/ve.dm.Transaction.test.js (new):
* Add tests for newFromDocumentReplace with mwReference nodes
ve.dm.mwExample.js:
* Add data for newFromDocumentReplace with mwReference tests
VisualEditor.hooks.php:
* Add new test file
Bug: 52102
Change-Id: I4aa980780114b391924f04df588e81c990c32983
This version pushes a `replaceMetadata` operation after a `replace` to
fixup trailing metadata if there is no inserted region and the removed
region includes metadata. This avoids a corner case where the
size of the metadata arrays inserted/removed in `Transaction.pushReplace()`
do not match the size of the data arrays inserted/removed.
We remove the now-unused `Document.getMetadataReplace()` method.
We also adjust `MetaList.onTransact()` so that it continues to work
properly when the number of metadata entries in `replace.insert` is
not the same as the number of metadata entries in `replace.remove`.
Change-Id: I1d600405b855ca1cb569853bb885b0752df47173
In ve.dm.Document.getMetadataReplace(), we used to only merge metadata
if the amount removed is larger than the amount inserted. But this
could end up putting metadata in odd positions, for example if you
have Foo[[Category:Bar]]Baz and you delete 'ooBa' and replace it with
{image}xxx{/image}, then the category ends up inside the image.
We should always merge metadata when a segment is deleted, so that it
appears outside any new structure added.
There's a weird corner case here when a segment is removed but no
insertion is made: the removed metadata then needs to get glommed onto
the next element. We extend the insert/replace metadata array
when this happens.
Bug: 53444
Bug: 53445
Change-Id: I51d55fb370b473273f9cf152fdd0f356377d4109
Was previously broken as getSlice was using ve.Range#equals to
compare ranges which is direction-sensitive.
Bug: 51538
Change-Id: Ib58d1d8fd11b62388c111a5da66171d13a9db9c2
These have been pointing to the same method for a while now,
we can safely remove these obsolete aliases and just use it
as generic copy.
* Each file touched by my editor had its new line at EOF fixed
where absent
* Don't copy an otherwise unused empty object
(ve.dm.Converter)
* Use common ve#copy syntax instead to create a link
(ve.dm.Document, ve.dm.example)
* Remove redundant conditionals for isArray/copyArray/copyObject
(ve.dm.example)
Change-Id: If560e658dc1fb59bf01f702c97e3e82a50a8a255
Whenever there is more than 2 spaces (except the extra space
on a continued line of an @ tag, or the extra space on a
continued line of a list item) it causes a <pre> context.
Removed both spurious spaces that caused a <pre> and ones that
didn't but looked like it could.
When making an ordered or unordered list, the first item needs
to be on a new line and in block context (e.g. an empty line
before it). Otherwise it is rendered inline as 1. Foo 2. Bar
(such as in #rebuildNodes where both the ordered and unordered
lists were broken).
Change-Id: Id0f154854afbdc8e5a8387da92e6b2cdf0875f69
This ensures that attributes and properties that are supposed to be
stripped on copypaste are actually stripped.
Bug: 49307
Change-Id: I8c90f4a0b33acba6eea3180cc077f8dc440e6e7b
Broke reference insertion because it removed
newFromNodeReplacement(), which is still in use.
This reverts commit 1765e39b40
Change-Id: I043997715474ad4850329ff903eb7a8c61c8b453
This function builds a transaction that takes a document slice and
inserts it back into the document it came from, applying any changes
that were made.
This makes editing document slices simple:
slicedDoc = doc.getDocumentSlice( captionNode );
// Edit slicedDoc using a surface
tx = ve.dm.Transaction.newFromDocumentInsertion( doc, captionNode, slicedDoc );
surface.change( tx );
Specifically, newFromDocumentInsertion replaces the node's contents
with the document's contents (meaning any changes made to the node in
the meantime are lost). It also merges the stores internal lists
of the two documents and remaps indexes accordingly. This means editing
of references inside of references is supported.
This functionality is not specific to slices, and can also be used to
safely insert data from a paste buffer, with internal list data being
transplanted correctly.
ve.dm.MetaLinearData:
* Make merge( [ undefined, undefined, ... ] ) return undefined rather
than [].
ve.dm.Document:
* In getDocumentSlice, store a pointer to the original dm.Document in
the new one, and also store the length of the internal list. This
allows us to figure out which internal list items the two documents
have in common when we insert the modified slice back into the main
document.
* In getMetadataReplace, optionally take the inserted metadata as a
parameter, to allow for operations that insert both data and metadata.
Per Ed's review, rewrite this function to return null rather than {}
if no metadata needs to be replaced.
ve.dm.InternalList:
* Add method to merge two internal lists
ve.dm.Transaction:
* Remove newFromNodeReplacement and replace it with newFromDocumentInsertion.
* In pushReplace, optionally take the inserted metadata as a parameter.
Change-Id: I786ee7bad796aa54bc242993b4de3ad18ad0773e
Also keep items in the order they appear in the document
and grouped by group and key.
Additions and removals are triggered by the new root/unroot events.
Change-Id: Ia3e90ccfdab88f352b89992b90554e5f03ff9952
Objectives:
* Fix compareObjects so it doesn't break when given arrays
* Remove compareArrays
* Rename compareObjects to compare and update callers of both methods
Changes:
ve.js
* Loosen check for whether to recurse so both arrays and objects qualify
* Remove compareArrays
* Rename compareObjects to compare
ve.dm.MWTemplateNode.js, ve.dm.AnnotationSet.js, ve.dm.Document.js,
ve.dm.Transaction.js
* Update uses of compare(Arrays|Objects) to use compare
Change-Id: I7d4f7ceb28c0389f0157b7598e291f21393b5b85
A document slice is a document built from a data slice of an existing
document. It's completely separate from the original document and has
its own store and internalList. The new document's data also contains
the entirety of the original document's internal list. It's possible
to create a document slice of data located inside the internal list,
in which case the resulting document will contain that data twice (one
mutable copy at the top level, and one immutable copy in the internal
list).
ve.dm.Document.js:
* Optionally take an internalList in the constructor. This allows us to
create a document with a clone of an existing internalList rather than
an empty one.
* Add edgeMetadata flag to getFullData()
ve.dm.IndexValueStore.js, ve.dm.InternalList.js:
* Make these classes cloneable
Change-Id: I93e06f764ace16aee9df941b07f8c2bff1a28e2b
Getting & setting the cursor is done with byte offsets
instead of data model offset (characters) so we need to
be able to convert between the two as well as splitting
characters.
TODO: The regex only works on surrogate pairs, not
yet combining accents.
fixupInsertion will combine a combining mark with the
character to its left it it can.
Bug: 48630
Change-Id: I8d936fb15d82f73cd45fac142c540a7950850d55
The code to record the first/last child stacks was written before
I decided on using a recursive call going only one step when such
a stack was encountered.
As we only ever test the length of the stack most of the code around
calculating the stack can be thrown away, and all we need is a simple
test to see if we are the first/last child of the immediate parent.
Change-Id: Iffb03c649e166c9f89061a4d944fa07633f16aba
The way it operated was evil. It did a depth-first search from the root,
finding the node using reference equality. For documents with deep
structures, this could take a long time. Inez did some profiling and
found it was called tens of millions of times on a complex document.
Kill getOffsetFromNode() and move its functionality to getOffset().
The logic has been completely rewritten: getOffset() now traverses
up from the node rather than down from the root, and pretty much does
the reverse of what getNodeFromOffset() does. This should be much more
efficient even without offset caching in the node objects (which we may
still implement later).
Change-Id: I125f9fa423c40db6472e2c4a7c94214218ba3bc7
fixUpInsertion now returns an object with both data and offset
which allows offset changes.
Within fixUpInsertion we lazy-generate first/lastChildStack which
is a list of parent nodes for which the current node is the first/last
child. Whenever we try to close off a node we check these stacks and
if they are populated we instead use a recursive call to start
fixUpInsertion again but with the offset shifted by 1.
Bug: 46799
Change-Id: Ic51dd03725c11f1f7e279929534ee3afea14d662
The fixUpStack is actually redundant code and closingStack
and openingStack handle all our cases. It was causing the
insertion to try to correct balance itself in the middle
of inserting two paragraphs, causing the creation on an
empty paragraph between them.
Added a test case for the fix and other cases to make
sure removing fixUpStack hasn't caused problems.
Bug: 46800
Change-Id: I35e54165709ac56e8116359a7c3b487eecf08ff7
Before, it took an array of objects and translated those to indexes
using the store. Literally every caller outside of the test suite got
an array of indexes from the linear model, translated those to objects,
then passed them into the AnnotationSet constructor which translated
them right back to indexes.
The previous behavior was kind of ridiculous on its face, but the
reason we found it is because Inez was investigating the performance
degradation when bolding a line and found that half of it was due
to the hundreds of ve.getHash() calls caused by this behavior.
Change-Id: I38df8ae9f6392849dacf477ea2f804283c964417
ve.dm.Transaction
* Replace operations are now built directly from the
linear model and automatically determine what metadata
replace information they need to include:
** retainMetadata
** replaceMetadata
** insertMetadata
ve.dm.Document
* Metadata array created empty and padded out after data parsing
as we are no longer using Document.spliceData to build it (a new
test checks for correct metadata length)
* spliceData replaced with getMetadataReplace, which instead returns
transactional steps of spliceData (retain, replace, insert)
ve.dm.MetaLinearData
* Add function for merging metadata items together. Only used
once in the code (Document.getMetadataReplace) but useful
for generating test data.
ve.dm.MetaList
* Replace operations with metadata need to calculate new offset
and indices directly, but can't be applied immediately lest they
put a metaItem out of place and affect findItem.
ve.dm.MetaItem
* Add methods to support queued moves as required by MetaList
Test files
* Updated to match new pushReplace API
* Remove any instances of Document.spliceData
* Extra check on sparse metadata array length
* Rewrite spliceData tests as getMetadataReplace tests
* Count expected cases in Transaction(Processor) tests
Bug: 46954
Change-Id: I4edad1c2dd37c723bff2792bab7d694ef17a86dc
Created an IndexValueStore class which can store any object and return
an integer index to its hash map.
Linear data is now stored in ve.dm.LinearData instances. Two subclasses
for element and meta data contain methods specific to those data types
(ElementLinearData and MetaLinearData).
The static methods in ve.dm.Document that inspected data at a given
offset are now instance methods of ve.dm.ElementLinearData.
AnnotationSets (which are no longer OrderedHashSets) have been moved
to /dm and also have to be instantiated with a pointer the store.
Bug: 46320
Change-Id: I249a5d48726093d1cb3e36351893f4bff85f52e2
Initially just with a Wordbreak module to implement Unicode standard
on 'Default Word Boundaries'. Due to it's standaloneability this has
been written as a separate library. Non-BMP characters are currently
not supported.
Bug: 44085
Change-Id: Ieafa070076f4c36855684f6bc179667e28af2c25
Previously these were static functions in TransactionProcessor
which instantiated a TP called .process() on it. These are now
methods of ve.dm.Document.
Also moved the emission of the 'transact' event on the document from
TransactionProcessor to Document itself, and moved the tests asserting
double application is protected against from TP to Document (because
the corresponding code moved as well).
Change-Id: I7c9f22a14accaf0ba1f70d5aa4f0573bb7e677d0
This is just a prototype - if it will be as useful as I expect
then it will get refactored (and probably get a better name +
location).
Change-Id: Ice1a2bd7d498d9d8438c35239216f01bd3db1826
We'll need this to let a MetaList observe a Document and adjust to changes.
We should probably also have SurfaceFragment listen to this event instead.
Change-Id: I9e811e242969eb44afe0b4fa8153d0fb1b0071cd
Before, replacement operations that both inserted and removed data at
the same time would be treated as removals followed by insertions,
so we'd reap the metadata from the affected range and move it to the
start of the range. For a pure replacement, this doesn't make any sense.
Instead, preserve the first min(insertLength, removeLength) elements in
the metadata array, then perform a pure insertion splice or a pure
removal splice for the length adjustment. Any metadata reaped in a
removal splice is restored at the offset where we started removing,
after the preserved portion.
These changes make the behavior of metadata reaping saner in general
(the previous behavior had the potential to move metadata around if it
was near a paragraph opening or closing and you converted the paragraph
to a heading), and makes the behavior match up with translateOffset(),
which is desirable for MetaList synchronization.
Change-Id: If9a1c6a7cf43ead7e3e1e8f6e081b139ca65fa53
Rather than meta-things being special kinds of nodes, they are now a
separate class of things (MetaItems) along with Nodes and Annotations.
* Created a generic ve.dm.MetaItem that meta items inherit from.
There will be actual instances of this class as well in the upcoming
meta group code.
* Renamed MetaNode to AlienMetaItem, MWMetaNode to MWMetaItem,
'metaBlock'/'metaInline' to 'alienMeta'
* Created a MetaItemFactory, handle meta items in the ModelRegistry
* Kill ve.dm.Node.static.isMeta, now obsolete
ve.dm.Converter:
* Pass in the MetaItemFactory
* Look up data element types in the ModelRegistry rather than the
NodeFactory, because they can be either nodes or meta items
* Document createDataElement() and make explicit that modelClass can be
either a node or a meta item
* Handle meta items in getDataFromDom()
* In getDomFromData(), check the MetaItemFactory as well as the NodeFactory
Change-Id: I893709c6f3aa00f85c1b905b70f9f4e597bdeada
Replacing [a-zA-Z] with a long unicode expression which encompasses
all the characters in the unicode 'letters' category. Similarly replacing
[0-9] with an expression for 'numbers'.
Bug: 44085
Change-Id: Idd403339caa24769ce08133dda06ab6d4b9d694e
Also in this commit is a minor fix to the regular expression so it
behaves as documented (the hyphen needed escaping).
Bug: 44085
Change-Id: Idc315e2dce79be8f028b5681c60f74e175b9d869
Have created builders for insertion, removal, and single element replacement.
In adding Document.getMetadata which is nearly identical to Document.getData,
the two functions have been refactored to use a common static method
getDataSlice, with this.data/this.metadata as an argument.
Document.spliceMetadata has been added. It is essentially spliceData with
the data/metadata synchronisation issue.
Metadata cursor position is now tracked in the TransactionProcessor. Cursor
advancement has been moved to a function so the metadata cursor can be reset
every time the data cursor is moved.
There were unhit bugs in the TransactionProcessor run test section, where
the data being loaded into the test documents wasn't always being deep-copied,
and the assert was looking at getData instead of getFulldata (which wouldn't
be able to test metadata changes).
Change-Id: Ieb20ab3e7827bc7ff04148f147da6728eb1eb666
This gets rid of the meta-specific hack in ve.dm.Document
Later on, I want to use this to get rid of meta-specific hacks in the
converter too, by pushing some of that logic into the ModelFactory.
Change-Id: I1dbee1654fa32d9c7cd521ec325b2553cde219d1
Continues where Ibb682332a6084e357104183641a104e3ae1e253f left off, adding tests and removing inconsistencies between the behavior of the document constructor, which was adding empty text nodes to empty paragraphs, and correcting other tests which expected empty text nodes to be there as well.
Change-Id: I414d061cdd494b8023f14e944eda2910a4dab0d4
