Decent workaround to dirty a project after bad layer handling

[elpaso]

Fixes #20743

@nyalldawson I don't know why https://github.com/qgis/QGIS/blob/master/src/app/qgisapp.cpp#L5809 but that is basically blocking a plain setDirty call from within the bad layer handlers.

If you can think of a better/cleaner solution than a timer (and that do not involve an API change in the bad layer handlers), please let me know.

[nyalldawson]

Here's a potential alternative approach:

1. Add a "layersChanged" signal to QgsHandleBadLayersHandler, and emit this signal from QgsHandleBadLayersHandler::handleBadLayers if layers were changed by the QgsHandleBadLayers dialog

2. In Qgisapp.cpp:1242, store a reference to the QgsHandleBadLayersHandler instance as a new QgisApp member, mAppBadLayersHandler.

3. In QgisApp::addProject, tweak the constructor to setup a connection to mAppBadLayersHandler layersChanged signal, something like this:

std::unique_ptr< QgsProjectDirtyBlocker > dirtyBlocker = qgis::make_unique< QgsProjectDirtyBlocker >( QgsProject::instance() );
QObject connectionScope; // manually control scope of layersChanged lambda connection - we need the connection automatically destroyed when this function finishes
bool badLayersHandled = false;
connect( mAppBadLayersHandler, &QgsHandleBadLayersHandler::layersChanged, &connectionScope, [&badLayersHandled] { badLayersHandled = true; });

and then at the end of that function

dirtyBlocker.reset(); // allow project dirtying again
if ( badLayersHandled )
    QgsProject::instance()->setDirty(true);
return true;

This avoids the timer approach, whilst still keeping the original intention of the dirty blocking. What do you think?

[elpaso]

Here's a potential alternative approach:

Looks good.

QObject connectionScope; // manually control scope of layersChanged lambda connection - we need the connection automatically destroyed when this function finishes

Do we really need this? The signal will never be emitted again after the initial bad layer handling.

[nyalldawson]

The signal will never be emitted again after the initial bad layer handling.

Isn't the bad layer handler tied to the life of the project instance? (I.e. forever?)

[elpaso]

Yes, but what are you trying to avoid here? Multiple connections to the same lambda any time QgisApp::addProject is executed? Would a Qt::UniqueConnection work in this case (maybe it's the lambda to prevent this) ?

[nyalldawson]

Exactly - unique connections aren't possible with lambdas

[elpaso]

std::call_once would probably also help here.

[luipir]

std::call_once would probably also help here.

I would prefer using the standard life-cycle mechanism of event connection... e.g. connect to connectionScope variable life-cycle as proposed by @nyalldawson . I'ts a kind of trick but a clean trick (btw IMHO too many lambdas respect moving and documenting in .h private or static method, but it's a question of style)

[elpaso]

IMHO explicit is better than implicit: std::call_once would clearly indicate what's happening, while the connectionScope trick requires knowledge of Qt and a comment to understand what's for (at least for me).

I'm generally in favor of using std:: over Qt's black magic whenever I can, but I'll follow Nyall's advice here.

[nyalldawson]

I'd have to see the code, but I don't think call once will help here. If you use it to only setup the connection on the first project read, then a permanent connection is being created with the lambda capturing a reference to a local variable - which will go out of scope after the function ends the first time and subsequent reads would crash.

[elpaso]

@nyalldawson you are right, I'll go for your proposed solution.