Merge "Moving ref group knowledge into a dedicated data structure"

extension.json

@@ -72,6 +72,7 @@
 			"remoteExtPath": "Cite/modules/ve-cite",
 			"scripts": [
 				"ve.dm.MWDocumentReferences.js",
+				"ve.dm.MWGroupReferences.js",
 				"ve.dm.MWReferenceModel.js",
 				"ve.dm.MWReferencesListNode.js",
 				"ve.dm.MWReferenceNode.js",

modules/ve-cite/ve.ce.MWReferencesListNode.js

@@ -240,12 +240,15 @@ ve.ce.MWReferencesListNode.prototype.update = function () {
 		this.$refmsg.text( emptyText );
 		this.$element.append( this.$refmsg );
 	} else {
-		const groupedByParent = this.docRefs.getGroupRefsByParents( listGroup );
+		const groupRefs = this.docRefs.getGroupRefs( listGroup );
-		const topLevelNodes = groupedByParent[ '' ] || [];
 		this.$reflist.append(
-			topLevelNodes.map( ( node ) => this.renderListItem(
+			// FIXME: Clean up access functions.
-				nodes, internalList, groupedByParent, refGroup, node
+			Object.keys( groupRefs.footnoteNumberLookup )
-			) )
+				.filter( ( listKey ) => groupRefs.footnoteNumberLookup[ listKey ][ 1 ] === -1 )
+				.sort( ( aKey, bKey ) => groupRefs.footnoteNumberLookup[ aKey ][ 0 ] - groupRefs.footnoteNumberLookup[ bKey ][ 0 ] )
+				.map( ( listKey ) => this.renderListItem(
+					nodes, internalList, groupRefs, refGroup, listKey
+				) )
 		);
 
 		this.updateClasses();
@@ -259,24 +262,23 @@ ve.ce.MWReferencesListNode.prototype.update = function () {
  * @private
  * @param {Object} nodes Node group object, containing nodes and key order array
  * @param {ve.dm.InternalList} internalList Internal list
- * @param {Object.<string, ve.dm.MWReferenceNode[]>} groupedByParent Mapping
+ * @param {ve.dm.MWGroupReferences} groupRefs object holding calculated information about all group refs
- *  from parent ref name (or '' for top-level) to refs
  * @param {string} refGroup Reference group
- * @param {ve.dm.MWReferenceNode} node Reference node to render as a footnote body
+ * @param {string} key top-level reference key, doesn't necessarily exist
  * @return {jQuery} Rendered list item
  */
-ve.ce.MWReferencesListNode.prototype.renderListItem = function ( nodes, internalList, groupedByParent, refGroup, node ) {
+ve.ce.MWReferencesListNode.prototype.renderListItem = function ( nodes, internalList, groupRefs, refGroup, key ) {
-	const listIndex = node.getAttribute( 'listIndex' );
+	const keyedNodes = nodes.keyedNodes[ key ] || [];
-	const key = internalList.keys[ listIndex ];
+	const node = keyedNodes ? keyedNodes[ 0 ] : null;
-	const keyedNodes = ( nodes.keyedNodes[ key ] || [] )
+	const listIndex = node ? node.getAttribute( 'listIndex' ) : null;
-		.filter(
+	const backlinkNodes = keyedNodes.filter(
-			// Exclude placeholders and references defined inside the references list node
+		// Exclude placeholders and references defined inside the references list node
-			( backRefNode ) => !backRefNode.getAttribute( 'placeholder' ) && !backRefNode.findParent( ve.dm.MWReferencesListNode )
+		( backRefNode ) => !backRefNode.getAttribute( 'placeholder' ) && !backRefNode.findParent( ve.dm.MWReferencesListNode )
-		);
+	);
 
 	const $li = $( '<li>' )
-		.css( '--footnote-number', `"${ this.docRefs.getIndexLabel( refGroup, key ) }."` )
+		.css( '--footnote-number', `"${ groupRefs.getIndexLabel( key ) }."` )
-		.append( this.renderBacklinks( keyedNodes, refGroup ), ' ' );
+		.append( this.renderBacklinks( backlinkNodes, refGroup ), ' ' );
 
 	// Generate reference HTML from first item in key
 	const modelNode = internalList.getItemNode( listIndex );
@@ -320,18 +322,8 @@ ve.ce.MWReferencesListNode.prototype.renderListItem = function ( nodes, internal
 				e.preventDefault();
 			} );
 		}
-		const listKey = node.getAttribute( 'listKey' );
-		const subrefs = groupedByParent[ listKey ] || [];
-		if ( subrefs.length ) {
-			$li.append(
-				$( '<ol>' ).append(
-					subrefs.map( ( subNode ) => this.renderListItem(
-						nodes, internalList, groupedByParent, refGroup, subNode
-					) )
-				)
-			);
-		}
 	} else {
+		// TODO: Special rendering for missing parent of orphaned subrefs?
 		$li.append(
 			$( '<span>' )
 				.addClass( 've-ce-mwReferencesListNode-muted' )
@@ -339,6 +331,17 @@ ve.ce.MWReferencesListNode.prototype.renderListItem = function ( nodes, internal
 		).addClass( 've-ce-mwReferencesListNode-missingRef' );
 	}
 
+	const subrefs = groupRefs.getSubrefs( key );
+	if ( subrefs.length ) {
+		$li.append(
+			$( '<ol>' ).append(
+				subrefs.map( ( subNode ) => this.renderListItem(
+					nodes, internalList, groupRefs, refGroup, subNode.getAttribute( 'listKey' )
+				) )
+			)
+		);
+	}
+
 	return $li;
 };
 

modules/ve-cite/ve.dm.MWDocumentReferences.js

@@ -19,6 +19,11 @@ ve.dm.MWDocumentReferences = function VeDmMWDocumentReferences( doc ) {
 
 	// Properties
 	this.doc = doc;
+	/**
+	 * Holds the information calculated for each group.
+	 *
+	 * @member {Object.<string, ve.dm.MWGroupReferences>}
+	 */
 	this.cachedByGroup = {};
 
 	doc.getInternalList().connect( this, { update: 'updateGroups' } );
@@ -68,26 +73,19 @@ ve.dm.MWDocumentReferences.prototype.updateGroups = function ( groupsChanged ) {
 /**
  * @private
  * @param {string[]} groupName Name of the reference group which needs to be
- *  updated
+ *  updated, with prefix
  */
 ve.dm.MWDocumentReferences.prototype.updateGroup = function ( groupName ) {
-	const refsByParent = this.getGroupRefsByParents( groupName );
+	const nodeGroup = this.doc.getInternalList().getNodeGroup( groupName );
-	const topLevelNodes = refsByParent[ '' ] || [];
+	this.cachedByGroup[ groupName ] = ve.dm.MWGroupReferences.static.makeGroupRefs( nodeGroup );
+};
 
-	const indexNumberLookup = {};
+/**
-	for ( let i = 0; i < topLevelNodes.length; i++ ) {
+ * @param {string} groupName with or without prefix
-		const topLevelNode = topLevelNodes[ i ];
+ * @return {ve.dm.MWGroupReferences}
-		const topLevelKey = topLevelNode.getAttribute( 'listKey' );
+ */
-		indexNumberLookup[ topLevelKey ] = ve.dm.MWDocumentReferences.static.contentLangDigits( i + 1 );
+ve.dm.MWDocumentReferences.prototype.getGroupRefs = function ( groupName ) {
-		const subrefs = ( refsByParent[ topLevelKey ] || [] );
+	return this.cachedByGroup[ groupName.startsWith( 'mwReference/' ) ? groupName : 'mwReference/' + groupName ];
-		for ( let j = 0; j < subrefs.length; j++ ) {
-			const subrefNode = subrefs[ j ];
-			const subrefKey = subrefNode.getAttribute( 'listKey' );
-			// FIXME: RTL, and customization of the separator like with mw:referencedBy
-			indexNumberLookup[ subrefKey ] = `${ ve.dm.MWDocumentReferences.static.contentLangDigits( i + 1 ) }.${ ve.dm.MWDocumentReferences.static.contentLangDigits( j + 1 ) }`;
-		}
-	}
-	this.cachedByGroup[ groupName ] = indexNumberLookup;
 };
 
 ve.dm.MWDocumentReferences.prototype.getAllGroupNames = function () {
@@ -122,54 +120,5 @@ ve.dm.MWDocumentReferences.static.contentLangDigits = function ( num ) {
  *  marker or reflist item number.
  */
 ve.dm.MWDocumentReferences.prototype.getIndexLabel = function ( groupName, listKey ) {
-	return ( this.cachedByGroup[ 'mwReference/' + groupName ] || {} )[ listKey ];
+	return this.getGroupRefs( groupName ).getIndexLabel( listKey );
-};
-
-/**
- * Get all refs for a group, organized by parent ref
- *
- * This is appropriate when rendering a reflist organized hierarchically by
- * subrefs using the `extends` feature.
- *
- * @param {string} groupName Filter by this group.
- * @return {Object.<string, ve.dm.MWReferenceNode[]>} Mapping from parent ref
- * name to a list of its subrefs.  Note that the top-level refs are under the
- * `null` value.
- */
-ve.dm.MWDocumentReferences.prototype.getGroupRefsByParents = function ( groupName ) {
-	const nodeGroup = this.doc.getInternalList().getNodeGroup( groupName );
-	const indexOrder = ( nodeGroup ? nodeGroup.indexOrder : [] );
-	// Compile a list of all top-level node names so that we can handle orphans
-	// while keeping them in document order.
-	const seenTopLevelNames = new Set(
-		indexOrder
-			.map( ( index ) => nodeGroup.firstNodes[ index ] )
-			.filter( ( node ) => node && !node.element.attributes.extendsRef && !node.element.attributes.placeholder )
-			.map( ( node ) => node.element.attributes.listKey )
-			.filter( ( listKey ) => listKey )
-	);
-
-	// Group nodes by parent ref, while iterating in order of document appearance.
-	return indexOrder.reduce( ( acc, index ) => {
-		const node = nodeGroup.firstNodes[ index ];
-		if ( !node || node.element.attributes.placeholder ) {
-			return acc;
-		}
-
-		let extendsRef = node.element.attributes.extendsRef || '';
-
-		if ( !seenTopLevelNames.has( extendsRef ) ) {
-			// Promote orphaned subrefs to become top-level refs.
-			// TODO: Ideally this would be handled by creating placeholder error
-			// nodes as is done by the renderer.
-			extendsRef = '';
-		}
-
-		if ( acc[ extendsRef ] === undefined ) {
-			acc[ extendsRef ] = [];
-		}
-		acc[ extendsRef ].push( node );
-
-		return acc;
-	}, {} );
 };

modules/ve-cite/ve.dm.MWGroupReferences.js

@@ -0,0 +1,128 @@
+'use strict';
+
+/*!
+ * @copyright 2024 VisualEditor Team's Cite sub-team and others; see AUTHORS.txt
+ * @license MIT
+ */
+
+/**
+ * Holds information about the refs from a single Cite group.
+ *
+ * This structure is persisted in memory until a document change affects a ref
+ * tag from this group, at which point it will be fully recalculated.
+ *
+ * @private
+ * @constructor
+ */
+ve.dm.MWGroupReferences = function VeDmMWGroupReferences() {
+	// Mixin constructors
+	OO.EventEmitter.call( this );
+
+	// Properties
+	this.footnoteNumberLookup = {};
+	// FIXME: push labeling to presentation code and drop from here.
+	this.footnoteLabelLookup = {};
+	this.subRefsByParent = {};
+
+	/** @private */
+	this.topLevelCounter = 1;
+	this.nodeGroup = null;
+};
+
+/* Inheritance */
+
+OO.initClass( ve.dm.MWGroupReferences );
+
+/* Static Methods */
+
+/**
+ * Rebuild information about this group of references.
+ *
+ * @param {Object} nodeGroup InternalList group object containing refs.
+ * @return {ve.dm.MWGroupReferences}
+ */
+ve.dm.MWGroupReferences.static.makeGroupRefs = function ( nodeGroup ) {
+	const result = new ve.dm.MWGroupReferences();
+	result.nodeGroup = nodeGroup;
+
+	( nodeGroup ? nodeGroup.indexOrder : [] )
+		.map( ( index ) => nodeGroup.firstNodes[ index ] )
+		// FIXME: debug null nodes
+		.filter( ( node ) => node && !node.getAttribute( 'placeholder' ) )
+		.forEach( ( node ) => {
+			const listKey = node.getAttribute( 'listKey' );
+			const extendsRef = node.getAttribute( 'extendsRef' );
+
+			if ( !extendsRef ) {
+				result.getOrAllocateTopLevelIndex( listKey );
+			} else {
+				result.addSubref( extendsRef, listKey, node );
+			}
+		} );
+
+	return result;
+};
+
+/* Methods */
+
+/**
+ * @private
+ * @param {string} listKey Full key for the top-level ref
+ * @return {number[]} Allocated topLevelIndex
+ */
+ve.dm.MWGroupReferences.prototype.getOrAllocateTopLevelIndex = function ( listKey ) {
+	if ( this.footnoteNumberLookup[ listKey ] === undefined ) {
+		const number = this.topLevelCounter++;
+		this.footnoteNumberLookup[ listKey ] = [ number, -1 ];
+		this.footnoteLabelLookup[ listKey ] = ve.dm.MWDocumentReferences.static.contentLangDigits( number );
+	}
+	return this.footnoteNumberLookup[ listKey ][ 0 ];
+};
+
+/**
+ * @private
+ * @param {string} parentKey Full key of the parent reference
+ * @param {string} listKey Full key of the subreference
+ * @param {ve.dm.MWReferenceNode} subrefNode Subref to add to internal tracking
+ */
+ve.dm.MWGroupReferences.prototype.addSubref = function ( parentKey, listKey, subrefNode ) {
+	if ( this.subRefsByParent[ parentKey ] === undefined ) {
+		this.subRefsByParent[ parentKey ] = [];
+	}
+	this.subRefsByParent[ parentKey ].push( subrefNode );
+	const subrefIndex = this.subRefsByParent[ parentKey ].length;
+
+	const topLevelIndex = this.getOrAllocateTopLevelIndex( parentKey );
+	this.footnoteNumberLookup[ listKey ] = [ topLevelIndex, subrefIndex ];
+	this.footnoteLabelLookup[ listKey ] = ve.dm.MWDocumentReferences.static.contentLangDigits( topLevelIndex ) +
+		// FIXME: RTL, and customization of the separator like with mw:referencedBy
+		'.' + ve.dm.MWDocumentReferences.static.contentLangDigits( subrefIndex );
+};
+
+/**
+ * @return {ve.dm.MWReferenceNode[]}
+ */
+ve.dm.MWGroupReferences.prototype.getAllRefsInDocumentOrder = function () {
+	return Object.keys( this.footnoteNumberLookup )
+		.sort( ( aKey, bKey ) => this.footnoteNumberLookup[ aKey ][ 0 ] - this.footnoteNumberLookup[ bKey ][ 0 ] )
+		.map( ( listKey ) => this.nodeGroup.keyedNodes[ listKey ] )
+		.filter( ( nodes ) => !!nodes )
+		.map( ( nodes ) => nodes[ 0 ] );
+};
+
+/**
+ * @param {string} parentKey parent ref key
+ * @return {ve.dm.MWReferenceNode[]} List of subrefs for this parent
+ */
+ve.dm.MWGroupReferences.prototype.getSubrefs = function ( parentKey ) {
+	return this.subRefsByParent[ parentKey ] || [];
+};
+
+/**
+ * @deprecated TODO: push to presentation
+ * @param {string} listKey full ref key
+ * @return {string} rendered number label
+ */
+ve.dm.MWGroupReferences.prototype.getIndexLabel = function ( listKey ) {
+	return this.footnoteLabelLookup[ listKey ];
+};

modules/ve-cite/ve.ui.MWReferenceSearchWidget.js

@@ -154,17 +154,9 @@ ve.ui.MWReferenceSearchWidget.prototype.buildSearchIndex = function () {
 			// FIXME: Should be impossible to reach
 			continue;
 		}
-		const groupedByParent = docRefs.getGroupRefsByParents( groupName );
+		const groupRefs = docRefs.getGroupRefs( groupName );
-		let flatNodes = [];
+		const flatNodes = groupRefs.getAllRefsInDocumentOrder()
-		if ( filterExtends ) {
+			.filter( ( node ) => !filterExtends || !node.getAttribute( 'extendsRef' ) );
-			flatNodes = ( groupedByParent[ '' ] || [] );
-		} else {
-			// flatMap
-			( groupedByParent[ '' ] || [] ).forEach( ( parentNode ) => {
-				flatNodes.push( parentNode );
-				flatNodes = flatNodes.concat( groupedByParent[ parentNode.getAttribute( 'listKey' ) ] || [] );
-			} );
-		}
 
 		index = index.concat( flatNodes.map( ( node ) => {
 			const listKey = node.getAttribute( 'listKey' );

tests/qunit/ve-cite/ve.dm.MWDocumentReferences.test.js

@@ -17,9 +17,8 @@ QUnit.test( 'extends test', ( assert ) => {
 	const doc = ve.dm.citeExample.createExampleDocument( 'extends' );
 	const docRefs = ve.dm.MWDocumentReferences.static.refsForDoc( doc );
 
-	// FIXME: Shows that the class doesn't handle orphans correctly
+	assert.strictEqual( docRefs.getIndexLabel( '', 'auto/0' ), '1.1' );
-	assert.strictEqual( docRefs.getIndexLabel( '', 'auto/0' ), '3.1' );
+	assert.strictEqual( docRefs.getIndexLabel( '', 'auto/1' ), '2' );
-	assert.strictEqual( docRefs.getIndexLabel( '', 'auto/1' ), '1' );
+	assert.strictEqual( docRefs.getIndexLabel( '', 'literal/orphaned' ), '3.1' );
-	assert.strictEqual( docRefs.getIndexLabel( '', 'literal/orphaned' ), '2' );
+	assert.strictEqual( docRefs.getIndexLabel( '', 'literal/ldr' ), '1' );
-	assert.strictEqual( docRefs.getIndexLabel( '', 'literal/ldr' ), '3' );
 } );

tests/qunit/ve-cite/ve.dm.citeExample.js

@@ -728,7 +728,7 @@ ve.dm.citeExample.domToDataCases = {
 				 data-mw='{"name":"ref","body":{"html":"Bar"},"attrs":{"extends":"foo"}}'
 				 class="mw-ref reference">
 					<a>
-						<span class="mw-reflink-text"><span class="cite-bracket">[</span>1<span class="cite-bracket">]</span></span>
+						<span class="mw-reflink-text"><span class="cite-bracket">[</span>1.1<span class="cite-bracket">]</span></span>
 					</a>
 				</sup>
 			</p>

tests/qunit/ve-cite/ve.ui.MWReferenceSearchWidget.test.js

@@ -15,14 +15,16 @@ function getInternalListMock( hasNode ) {
 	} : {};
 	const groups = hasNode ? {
 		'mwReference/': {
-			indexOrder: [ 0 ]
+			indexOrder: [ 0 ],
+			firstNodes: [ node ],
+			keyedNodes: { [ listKey ]: [ node ] }
 		}
 	} : {};
 	const docRefsMock = {
 		getAllGroupNames: () => ( Object.keys( groups ) ),
-		getGroupRefsByParents: () => ( { '': [ node ] } ),
 		getIndexLabel: () => ( '1' ),
-		getItemNode: () => ( node )
+		getItemNode: () => ( node ),
+		getGroupRefs: ( groupName ) => ( ve.dm.MWGroupReferences.static.makeGroupRefs( groups[ groupName ] ) )
 	};
 	const docMock = {
 		getStorage: () => ( docRefsMock ),
@@ -31,7 +33,8 @@ function getInternalListMock( hasNode ) {
 	const mockInternalList = {
 		getDocument: () => ( docMock ),
 		getNodeGroups: () => ( groups ),
-		getItemNode: () => ( node )
+		getItemNode: () => ( node ),
+		getNodeGroup: ( groupName ) => ( groups[ groupName ] )
 	};
 	docMock.getInternalList = () => ( mockInternalList );
 	node.getDocument = () => ( docMock );