wikimedia/mediawiki-extensions-Cite
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/Cite synced 2024-09-23 10:20:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

Consolidate duplicate "is reference list empty" code paths

Browse source 
Introduce a static method so we don't need to copy paste code.

Note that the static method still largely duplicates what the method
.buildIndex() will later do. Both loops iterate the reference groups
and the references in each group. The main difference is that the
"is empty" check stops extremely early the moment it finds any
non-empty group.

That's also why I'm convinced it's not worth caching the result.
I benchmarked it and it's nanoseconds. But there are more reasons:

The non-static .isIndexEmpty() method is currently only used when
Citoid is active the same time. Which means the cached result was
entirely unused on installations without Citoid.

Bug: T356871
Change-Id: Id5c4295086bc977ef52ad141be9962d2eecb1bcc
This commit is contained in:
thiemowmde 2024-02-22 19:28:17 +01:00
parent 2349c3ceec
commit c921c76dd2
3 changed files with 22 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

30
modules/ve-cite/ve.ui.MWReferenceSearchWidget.js
View file

@ -26,7 +26,6 @@ ve.ui.MWReferenceSearchWidget = function VeUiMWReferenceSearchWidget( config ) {
// Properties
this.index = null;
this.indexEmpty = true;
this.wasUsedActively = false;
// Initialization
@ -38,6 +37,23 @@ ve.ui.MWReferenceSearchWidget = function VeUiMWReferenceSearchWidget( config ) {
OO.inheritClass( ve.ui.MWReferenceSearchWidget, OO.ui.SearchWidget );
/* Static Methods */
/**
* @param {ve.dm.InternalList} internalList
* @return {boolean}
*/
ve.ui.MWReferenceSearchWidget.static.isIndexEmpty = function ( internalList ) {
const groups = internalList.getNodeGroups();
// Doing this live every time is cheap because it stops on the first non-empty group
for ( const groupName in groups ) {
if ( groupName.indexOf( 'mwReference/' ) === 0 && groups[ groupName ].indexOrder.length ) {
return false;
}
}
return true;
};
/* Methods */
ve.ui.MWReferenceSearchWidget.prototype.onQueryChange = function () {
@ -82,15 +98,6 @@ ve.ui.MWReferenceSearchWidget.prototype.setInternalList = function ( internalLis
this.internalList = internalList;
this.internalList.connect( this, { update: 'onInternalListUpdate' } );
this.internalList.getListNode().connect( this, { update: 'onListNodeUpdate' } );
const groups = internalList.getNodeGroups();
for ( const groupName in groups ) {
if ( groupName.indexOf( 'mwReference/' ) === 0 && groups[ groupName ].indexOrder.length ) {
this.indexEmpty = false;
return;
}
}
this.indexEmpty = true;
};
/**
@ -201,7 +208,8 @@ ve.ui.MWReferenceSearchWidget.prototype.buildSearchIndex = function () {
* @return {boolean} Index is empty
*/
ve.ui.MWReferenceSearchWidget.prototype.isIndexEmpty = function () {
return this.indexEmpty;
return !this.internalList ||
ve.ui.MWReferenceSearchWidget.static.isIndexEmpty( this.internalList );
};
/**

15
modules/ve-cite/ve.ui.MWUseExistingReferenceCommand.js
View file

@ -31,19 +31,8 @@ OO.inheritClass( ve.ui.MWUseExistingReferenceCommand, ve.ui.Command );
* @override
*/
ve.ui.MWUseExistingReferenceCommand.prototype.isExecutable = function ( fragment ) {
if ( !ve.ui.MWUseExistingReferenceCommand.super.prototype
.isExecutable.apply( this, arguments )
) {
return false;
}
const groups = fragment.getDocument().getInternalList().getNodeGroups();
for ( const groupName in groups ) {
if ( groupName.indexOf( 'mwReference/' ) === 0 && groups[ groupName ].indexOrder.length ) {
return true;
}
}
return false;
return ve.ui.MWUseExistingReferenceCommand.super.prototype.isExecutable.apply( this, arguments ) &&
!ve.ui.MWReferenceSearchWidget.static.isIndexEmpty( fragment.getDocument().getInternalList() );
};
/* Registration */

7
tests/qunit/ve-cite/ve.ui.MWReferenceSearchWidget.test.js
View file

@ -66,14 +66,9 @@ QUnit.test( 'isIndexEmpty', function ( assert ) {
const widget = new ve.ui.MWReferenceSearchWidget();
assert.true( widget.isIndexEmpty() );
// XXX: This is a regression test with a fragile setup. Please feel free to delete this test
// when you feel like it doesn't make sense to update it.
const internalList = {
connect: () => null,
getListNode: () => ( { connect: () => null } ),
widget.internalList = {
getNodeGroups: () => ( { 'mwReference/': { indexOrder: [ 0 ] } } )
};
widget.setInternalList( internalList );
assert.false( widget.isIndexEmpty() );
} );