Bug report #7633

QgsMapLayerRegistry.removeMapLayers does not properly handle the argument "theEmitSignal"

Added by Gregory LOEB over 7 years ago. Updated over 7 years ago.

Status:Closed
Priority:Normal
Assignee:-
Category:-
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:No Resolution:
Crashes QGIS or corrupts data:No Copied to github as #:16555

Description

The signal layersWillBeRemoved is emitted whatever value has the "*bool theEmitSignal*" argument of "*removeMapLayers*" method.
Indeed, the last line of method removeMapLayers is:
emit layersWillBeRemoved( theLayerIds );
Yet, this method begins with those lines
if ( theEmitSignal )
emit layersWillBeRemoved( theLayerIds );

Then when layers are removed from QGIS the signal layersWillBeRemoved is emitted once when theEmitSignal agument is set to false and twice when it is set to true.
Thus the last line of the removeMapLayers(QStringList theLayerIds, bool theEmitSignal) has to be removed to correct this mis-behaviour.

History

#1 Updated by Matthias Kuhn over 7 years ago

IMHO the theEmitSignal is bad practice and it should be considered to remove it for 2.0. I already did that for selectionChanged events.
If you subscribe to a signal and you are not guaranteed to get it... Seems kinda strange.

Why was that parameter introduced originally?

#2 Updated by Matthias Kuhn over 7 years ago

  • Subject changed from removeMapLayers method from QgsMapLayerRegistry class seems to contain a mistake to QgsMapLayerRegistry.removeMapLayers does not properly handle the argument "theEmitSignal"

#3 Updated by Matthias Kuhn over 7 years ago

  • Status changed from Open to Closed

Thank you for reporting this Issue Gregory.
I'm glad we could resolve this before 2.0 API freeze.

The API has been reworked in 193b6154d213254672061267bac8e6e8ad5ea1f3

Also available in: Atom PDF