Commit graph

18 commits

Author SHA1 Message Date
Ed Sanders 67e9d5d1dd (bug 45029) Transactions for metadata modification.
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
2013-02-14 17:27:27 -08:00
Catrope 591f2e7396 Change the HTML attribute prefix from html/ to html/0/
This means that <p data-foo="bar"> will now be converted to a paragraph
with attributes {"html/0/data-foo":"bar"} rather than {"html/foo":"bar"}

This paves the way for multi-element node (about group) handling in the
node API: nodes representing multiple DOM elements will have html/i/attr
to represent an attribute of the i'th DOM element.

Change-Id: Iea52bdccd721942ca708c8f9f47e934524809845
2013-02-06 12:00:43 -08:00
Trevor Parscal 16310f7535 Added tests and fixed inconsistencies for empty text node handling
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
2013-01-29 15:13:17 -08:00
Trevor Parscal 8d33a3de0d Major Documentation Cleanup
* Made method descriptions imperative: "Do this" rather than "Does this"
* Changed use of "this object" to "the object" in method documentation
* Added missing documentation
* Fixed incorrect documentation
* Fixed incorrect debug method names (as in those VeDmClassName tags we add to functions so they make sense when dumped into in the console)
* Normalized use of package names throughout
* Normalized class descriptions
* Removed incorrect @abstract tags
* Added missing @method tags
* Lots of other minor cleanup

Change-Id: I4ea66a2dd107613e2ea3a5f56ff54d675d72957e
2013-01-16 15:37:59 -08:00
Timo Tijhof b11bbed7a6 JSDuck: Generated code documentation!
See CODING.md for how to run it.

Mistakes fixed:
* Warning: Unknown type function
  -> Function
* Warning: Unknown type DOMElement
  -> HTMLElement
* Warning: Unknown type DOM Node
  -> HTMLElement
* Warning: Unknown type Integer
  -> Mixed
* Warning: Unknown type Command
  -> ve.Command
* Warning: Unknown type any
  -> number
* Warning: Unknown type ve.Transaction
  -> ve.dm.Transaction
* Warning: Unknown type ve.dm.AnnotationSet
  -> ve.AnnotationSet
* Warning: Unknown type false
  -> boolean
* Warning: Unknown type ve.dm.AlienNode
  ve.dm doesn't have a generic AlienNode like ve.ce
  -> Unknown type ve.dm.AlienInlineNode|ve.dm.AlienBlockNode
* Warning: Unknown type ve.ve.Surface
  -> ve.ce.Surface
* ve.example.lookupNode:
  -> Last @param should be @return
* ve.dm.Transaction.prototype.pushReplace:
  -> @param {Array] should be @param {Array}
* Warning: ve.BranchNode.js:27: {@link ve.Node#hasChildren} links to non-existing member
  -> (removed)
* Warning: ve.LeafNode.js:21: {@link ve.Node#hasChildren} links to non-existing member
  -> (removed)

Differences fixed:
* Variadic arguments are like @param {Type...} [name]
  instead of @param {Type} [name...]
* Convert all file headers from /** to /*! because JSDuck tries
  to parse all /** blocks and fails to parse with all sorts of
   errors for "Global property", "Unnamed property", and
  "Duplicate property".
  Find: \/\*\*([^@]+)(@copyright)
  Replace: /*!$1$2
* Indented blocks are considered code examples.
  A few methods had documentation with numbered lists that were
  indented, which have now been updated to not be intended.
* The free-form text descriptions are parsed with Markdown,
  which requires lists to be separated from paragraphs by an
  empty line.
  And we should use `backticks` instead of {braces} for inline
  code in text paragraphs.
* Doc blocks for classes and their constructor have to be
  in the correct order (@constructor, @param, @return must be
  before @class, @abstract, @extends etc.)
* `@extends Class` must not have Class {wrapped}
* @throws must start with a {Type}
* @example means something else. It is used for an  inline demo
  iframe, not code block. For that simply indent with spaces.
* @member means something else.
  Non-function properties are marked with @property, not @member.
* To create a link to a class or member, in most cases the name
  is enough to create a link. E.g. Foo, Foo.bar, Foo.bar#quux,
  where a hash stands for "instance member", so Foo.bar#quux,
  links to Foo.bar.prototype.quux (the is not supported, as
  "prototype" is considered an implementation detail, it only
  indexes class name and method name).
  If the magic linker doesn't work for some case, the
  verbose syntax is {@link #target label}.
* @property can't have sub-properties (nested @param and @return
  values are supported, only @static @property can't be nested).
  We only have one case of this, which can be worked around by
  moving those in a new virtual class. The code is unaltered
  (only moved down so that it isn't with the scope of the main
  @class block). ve.dm.TransactionProcessor.processors.

New:
* @mixins: Classes mixed into the current class.
* @event: Events that can be emitted by a class. These are also
  inherited by subclasses. (+ @param, @return and @preventable).
  So ve.Node#event-attach is inherited to ve.dm.BreakNode,
  just like @method is.
* @singleton: Plain objects such as ve, ve.dm, ve.ce were missing
  documentation causing a tree error. Documented those as a
  JSDuck singleton, which they but just weren't documented yet.
  NB: Members of @singleton don't need @static (if present,
  triggers a compiler warning).
* @chainable: Shorthand for "@return this". We were using
  "@return {classname}" which is ambiguous (returns the same
  instance or another instance?), @chainable is specifically
  for "@return this". Creates proper labels in the generated
  HTML pages.

Removed:
* @mixin: (not to be confused with @mixins). Not supported by
  JSDuck. Every class is standalone anyway. Where needed marked
  them @class + @abstract instead.

Change-Id: I6a7c9e8ee8f995731bc205d666167874eb2ebe23
2013-01-05 01:16:32 +01:00
Catrope 5e2c421b77 Make annotating inline elements actually work
The transaction builder would step around inline content elements when
building annotation transactions. This is now fixed.

I also tweaked the processor to tolerate attempts to annotate inline
closings. This allows the builder to generate simpler transactions,
because it doesn't have to step around the closing.

Change-Id: I1e0d7f95b38bad1b35b3e125a53350d2d126a7de
2012-11-27 14:41:40 -08:00
Catrope 79e4b139fb (bug 42212) Fix JS error when inserting after alien at the end
TransactionProcessor was using parentOuterRange without checking whether
it was present, so it was exploding for indexInNode results.

Now checking for parentOuterRange presence, and falling back to
nodeOuterRange when missing.

This fix causes inconsistencies with zero-length text nodes. We should
fix these eventually, but for now I've just made the unit tests
tolerant of zero-length-text-node deviations.

Change-Id: Id9eadd57a0d5fcbaf009c0781da0a03928aebb31
2012-11-19 14:35:30 -08:00
Catrope 04a999f991 Add change marking for Parsoid's benefit
* Add map of change markers per offset to Transaction
* Map is populated by TransactionProcessor
* Markers are reversed on rollback
* Removals aren't marked, Parsoid can detect these using DSR
  discontinuities

Change-Id: I2290886ab411c6ad6162044ed85c091313613e51
2012-11-06 10:11:11 -08:00
Timo Tijhof 4cc2101ffd Test: Enforce # of expected assertions.
Change-Id: I041c792d1841f69677f8c7d38f67108475a0afc9
2012-10-25 22:06:07 +02:00
Catrope 29cff8c105 Guard transactions against double commit/rollback
* Store the applied state in the Transaction
* Store the Transaction in the TransactionProcessor (previously, only
  its operations were stored)
* Have commit() and rollback() throw exceptions when passed transactions
  with the wrong applied state
* Add tests for this behavior

Change-Id: I27b7a96fdf4d3555d78f64c05a03702ea560c802
2012-10-24 17:47:41 -07:00
Catrope 405581f6b8 New annotation API: update tests
Change-Id: I301a1d89c33dd68197d629d187a9a5be8cbf3852
2012-10-12 15:07:28 -07:00
Catrope 5e7c14c868 Manage annotations in ve.AnnotationSet object
Introduced the ve.AnnotationSet class to manage sets of annotations. This
is a generalization of ve.OrderedHashSet, a class that manages a set
using an array and an object keyed by hash.

Converted everything that stores, tracks or passes around annotations to
use ve.AnnotationSet. In particular, this means the linear model now
contains AnnotationSets instead of hash-keyed objects.

This allows us to maintain the order of annotations in the linear model,
and will help fix bugs with annotation ordering and splitting.

Change-Id: I50975b0a95f4cc33017a0b59fdede9ed1eff0124
2012-09-06 14:39:38 -07:00
Timo Tijhof 23c5b0d02c Make use of new jshint options
* Restricting "camelcase":
  No changes, we were passing all of these already

* Explicitly unrestricting "forin" and "plusplus"
  These are off by default in node-jshint, but some distro of jshint
  and editors that use their own wrapper around jshint instead of
  node-jshint (Eclipse?) may have different defaults. Therefor
  setting them to false explicitly. This also serves as a reminder
  for the future so we'll always know we don't pass that, in case
  we would want to change that.

* Fix order ("quotemark" before "regexp")

* Restricting "unused"
  We're not passing all of this, which is why I've set it to false
  for now. But I did put it in .jshintrc as placeholder.
  I've fixed most of them, there's some left where there is no clean
  solution.

* While at it fix a few issues:
 - Unused variables ($target, $window)
 - Bad practices (using jQuery context for find instead of creation)
 - Redundant /*global */ comments
 - Parameters that are not used and don't have documentation either
 - Lines longer than 100 chars @ 4 spaces/tab

* Note:
 - ve.ce.Surface.prototype.onChange takes two arguments but never
   uses the former. And even the second one can be null/undefined.
   Aside from that, the .change() function emits
   another event for the transaction already. Looks like this
   should be refactored a bit, two more separated events probably
   or one that is actually used better.
 - Also cleaned up a lot of comments, some of which were missing,
   others were incorrect
 - Reworked the contentChange event so we are no longer using the
   word new as an object key; expanded a complex object into multiple
   arguments being passed through the event to make it easier to work
   with and document

Change-Id: I8490815a508c6c379d5f9a743bb4aefd14576aa6
2012-08-10 02:50:30 +02:00
Trevor Parscal d8ee3c2c29 After much research on error objects, native = good, custom = bad
Stack traces, line numbers, etc. All the approaches I've seen are bad hacks. This is the best way to go.

Change-Id: Ib12e9d2ecfe610bcc89d046005e35cc13efa3d99
2012-08-08 10:48:53 -07:00
Trevor Parscal b4de3ead08 Throw ve.Error instead of string literals
Throwing strings is bad because it doesn't include a lot of important
information that an error object does, such as a stack trace or where
the error was actually thrown from.

ve.Error inherits directly from Error. In the future we may create
more specific subclasses and/or do custom stuff.

Some interesting reading on the subject:
* http://www.devthought.com/2011/12/22/a-string-is-not-an-error/

Change-Id: Ib7c568a1dcb98abac44c6c146e84dde5315b2826
2012-08-08 06:19:00 +02:00
Trevor Parscal 13ccb68ae1 Cleanup - all jshint conditions are now met
Also:
* Removed a lot of dead code in Surface that was used in the now dead and gone sandbox.
* Changed from throwing an exception when calling getBalancedData on a range that produces no results from selectNodes to just returning []

Change-Id: Icf27094724eae5b90eec21308f9e26afe877e3ee
2012-08-03 18:56:04 -07:00
Timo Tijhof 88f6089952 Kranitor #1: On-boarding
'''Kranitor commits''' are commits by Krinkle with his janitor hat on.
Must never contain functional changes mixed with miscellaneous changes.

.gitignore:
 * Add .DS_Store to the ignore list so that browsing the directories
   on Mac OS X, will not add these files to the list of untracked
   files.
 * Fix missing newline at end of file

.jshintrc
 * raises -> throws
 * +module (QUnit.module)
 * remove 'Node' (as of node-jshint 1.7.2 this is now part of
   'browser:true', as it should be)

Authors:
 * Adding myself

MWExtension/VisualEditor.php
 * Fix default value of wgVisualEditorParsoidURL to not
   point to the experimental instance in WMF Labs.

Issues:
 * ve.ce.TextNode:
  - Fix TODO: Don't perform a useless clone of an already-jQuerified object.
  - Use .html() to set html content instead of encapsulating between
    two strings. This is slightly faster but more importantly safer,
    and prevents situations where the resulting jQuery collection
    actually contains 2 elements instead of 1, thus messing up
    what .contents() is iterating over.
 * ve.ce.Document.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.Document.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.Transaction.test.js
  - Fix: ReferenceError: assert is not defined
 * ve.dm.TransactionProcessor.test.js
  - Fix: ReferenceError: assert is not defined
 * ext.visualEditor.viewPageTarget
  - Missing dependency on 'mediawiki.Title'

Code conventions / Misc cleanup
 * Various JSHint warnings.
 * Whitespace
 * jQuery(): Use '<tag>' for element creation,
   use '<valid><xml/></valid>' for parsing
 * Use the default operator instead of ternary when the condition and
   first value are the same.
   x = foo ? foo : bar; -> x = foo || bar;
   Because contrary to some programming language (PHP...), in JS the
   default operator does not enforce a boolean result but returns the
   original value, hence it being called the 'default' operator, as
   opposed to the 'or' operator.
 * No need to call addClass() twice, it takes a space-separated list
   (jQuery splits by space and adds if needed)
 * Use .on( event[, selector], fn ) instead of the deprecated
   routers to it such as .bind(), .delegate() and .live().
   All these three are now built-in and fully compatible with .on()
 * Add 'XXX:' comments for suspicious code that I don't want to change
   as part of a clean up commit.
 * Remove unused variables (several var x = this; where x was not
   used anywhere, possibly from boilerplate copy/paste)
 * Follows-up Trevor's commit that converts test suites to the new
   QUnit format. Also removed the globals since we no longer use those
   any more.

Change-Id: I7e37c9bff812e371c7f65a6fd85d9e2af3e0a22f
2012-07-27 14:40:00 -07:00
Trevor Parscal 76bac7d152 Move test files
Change-Id: Id0a0bd5b4a91f702cad34e9f5e7f2121763abffd
2012-07-25 14:35:49 -07:00
Renamed from tests/ve/dm/ve.dm.TransactionProcessor.test.js (Browse further)