Yes, some extra eyes sounds good. I'll hold off on commit this then.

Text datablocks have always been handled similar to scenes as far as I know. It's not clear to me why that is being changed to fake user now as a fix, instead of adding it to the list of exceptions.

The new fake user button in the text editor is not helpful to most end users, it's mostly a button that helps you lose data by accident. If the datablock selector is redesigned and this moves into a menu it's less of a risk. I guess for text datablocks autogenerated by scripts there may be a use case.

Also, we should stop adding things like this in new code:

if (ELEM(GS(id->name), ID_WM, ID_WS, ID_SCE, ID_SCR, ID_LI)) {

That's what IDTypeInfo is for.

In D10983#282174, @Brecht Van Lommel (brecht) wrote:

It's not clear to me why that is being changed to fake user now as a fix, instead of adding it to the list of exceptions.

Right, my initial fix was to just add the text data blocks to be excluded from the recursive purge.
However after talking to Bastien, he told me that he thought that wasn't what we wanted to do.

Instead we would implement fake user for text data blocks so that text generated by scripts or
plugins that are tied to an object can be cleaned up when the object is removed.

I talked to Demeter about this as well, and he agreed with Bastien that having this functionality for addons/scripts is nice.
(As it can be a PITA to clean up auto generated script files that are not used anymore)

I agree that the current "fake user" button in the editor might not be optimal.
But after I discussed some alternatives with Julian, I think that having it there will make this functionality be discoverable by script and addon writers.
(And regular users of course)
However, I fully agree with you that for most people this is probably useless information and/or a liability in that they could uncheck it and lose data.
But this is also the case in other parts of blender as well, so I don't know if we want to go the "dummy proof" route or not.

Also, we should stop adding things like this in new code:

if (ELEM(GS(id->name), ID_WM, ID_WS, ID_SCE, ID_SCR, ID_LI)) {

That's what IDTypeInfo is for.

So I could change that the the preferred IDTypeInfo in the patch?
Just checking as I didn't add it, but I just changed the current code.

In D10983#282174, @Brecht Van Lommel (brecht) wrote:

Also, we should stop adding things like this in new code:

if (ELEM(GS(id->name), ID_WM, ID_WS, ID_SCE, ID_SCR, ID_LI)) {

That's what IDTypeInfo is for.

This is entirely unrelated to that patch, this code is there since ages. Of course at some point it would be a nice cleanup, like ton of other similar cases in existing code. But please do not mix it with a bugfix/behavior change patch

In D10983#282173, @Brecht Van Lommel (brecht) wrote:

Text datablocks have always been handled similar to scenes as far as I know. It's not clear to me why that is being changed to fake user now as a fix, instead of adding it to the list of exceptions.

The new fake user button in the text editor is not helpful to most end users, it's mostly a button that helps you lose data by accident. If the datablock selector is redesigned and this moves into a menu it's less of a risk. I guess for text datablocks autogenerated by scripts there may be a use case.

I do not see how having that kind of undocumented, spread all around the code exceptions is a good thing. We have an official mechanism to keep data around even if unused (fake user), I would much rather remove exceptions and weird special corner cases for some specific types, rather than spread that behavior into more generic ID management code. Unless there is a strong valid reason to do so.

That being said, might be worth checking if purge should not take into account the 'ensure real' flag too... But issue is this flag is primarily intended for editors (aka 'weak' actual data users)... Using it would prevent e.g. deletion of an image shown in an image editor, even if it is not actually used by any other ID.

To be clear, don't let my comments hold up a fix for this issue since it's pretty critical. The review is still marked as approved.

The current solution works, and you don't need my approval since I'm no longer chief architect.