#584 AddToExtensions not working within scripted project wizard SetupProject

[Walt]

Create a Scripted project wizard.
Within SetupProject(project) function add:

project.AddToExtensions(_T("code_completion/+search_path:add=include_path"));

Expected behaviour: "include_path" is added to project properties, C/C++ parser options.
Actual behaviour: Silently fails. (No warning. No addition to project file.)

Tested in CodeBlocks 16.01 and nightly 20171217 svn 11246 on Windows 7 64-bit OS.

[bluehazzard]

Ok, i know what is going on here:
On project creation this steps are executed
1) The project gets saved and all plugins call their save methods. The plugins do NOT look if the file was modified, but write all their options to the file, overwriting what was there
2) Your AddExtension method is called and the configuration is added to the project file
3) The project gets saved again. The plugins do NOT look if the file was modified and overwrite all settings you have made in this file.

Culprit function in this case:
src\plugins\codecompletion\nativeparser.cpp:1268

void NativeParser::OnProjectLoadingHook(cbProject* project, TiXmlElement* elem, bool loading)
{
    if (loading)
    {
        // Hook called when loading project file.
        wxArrayString& pdirs = GetProjectSearchDirs(project);

        TiXmlElement* CCConf = elem->FirstChildElement("code_completion");
        if (CCConf)
        {
            TiXmlElement* pathsElem = CCConf->FirstChildElement("search_path");
            while (pathsElem)
            {
                if (pathsElem->Attribute("add"))
                {
                    wxString dir = cbC2U(pathsElem->Attribute("add"));
                    if (pdirs.Index(dir) == wxNOT_FOUND)
                        pdirs.Add(dir);
                }

                pathsElem = pathsElem->NextSiblingElement("search_path");
            }
        }
    }
    else
    {
        // Hook called when saving project file.
        wxArrayString& pdirs = GetProjectSearchDirs(project);

        // since rev4332, the project keeps a copy of the <Extensions> element
        // and re-uses it when saving the project (so to avoid losing entries in it
        // if plugins that use that element are not loaded atm).
        // so, instead of blindly inserting the element, we must first check it's
        // not already there (and if it is, clear its contents)
        TiXmlElement* node = elem->FirstChildElement("code_completion");
        if (!node)
            node = elem->InsertEndChild(TiXmlElement("code_completion"))->ToElement();
        if (node)
        {
            node->Clear();
            for (size_t i = 0; i < pdirs.GetCount(); ++i)
            {
                TiXmlElement* path = node->InsertEndChild(TiXmlElement("search_path"))->ToElement();
                if (path) path->SetAttribute("add", cbU2C(pdirs[i]));
            }
        }
    }
}

I think all plugins have this behaviour and i think this was reported many times in the forum. The current way does not work with this implementation and i think this is a core feature of the scripted wizard to modify also plugins. My purposed solution is to call the load function of the plusins before saving it. If the plugin node is not present all should work as expected. If a plugin node is present in the project file it will get loaded correctly and after that we can save without problems.
I have not tried it right now, i would like to hear a opinion from a developer first.

[bluehazzard]

On a second thought this may be more complicated. I think we have to extend the scriptedwizard an call a function "SetupPlugins". In this script function the script can modify the plugins xml section. We can call this "SetupPlugins" after we have saved the project file for a first time. Then, after "SetupPlugins" we can load the file again and all should work...

The project is saved on project creation and in the line plugins/scriptedwizard/wiz.cpp:536 after the "SetupProject" squirrel function is called. This second save overwrites the settings made in "SetupProject"

[Teodor Petrov]

I have a patch series which removes the project hooks, but I'm not sure if it can be separated from the original feature I'm working on...

[Walt]

Do we need to notify plugins at all when running the wizard? If not then we could modify cbProject::Save to bool Save(bool notifyPlugins = true); triggering notifiction if notifyPlugins is true then call theProject->Save(false) from the wizard.
If I can get wxWidgets and CodeBlocks to compile I will submit a patch.
If we do need to notify plugins then we could add another SquirrelFunction called "Finalise" and call theProject->Save(false) after that.

[Walt]

Find attached a patch that reloads the plugin configuration before saving the project.

[Walt]

Should I create a separate ticket with the patch or is it sufficient to just add the patch here? The fix is quite simple.

[bluehazzard]

@oBFusCATed What do you think about this fix? This should be fixed for sure, i just don't know if this is the right way... From a code size perspective this seems nice and should work. I have looked at the plugins and all seem to load the project over this hooks

EDIT: Also related https://sourceforge.net/p/codeblocks/tickets/513/
EDIT2: Even more related: https://sourceforge.net/p/codeblocks/tickets/512/

[Teodor Petrov]

I don't like the whole system. As I've mentioned earlier, I've already removed it in a branch, but unfortunately I don't have the time to finish the main objective of the branch, so I don't know when I'll do it.

About the patch:
0. The name of the method is wrong. It should be something like invalidate extensions.
1. I don't like that a new api is added. Especially one that must be removed in the future.
2. Isn't it possible to add some modified flag or to reset the storage on every call to addExtension, which will cause a reparse the next time an extension value is used?

[bluehazzard]

Are you talking about the loader and save hooks? How do you implement them in the new branch? Virtual functions?

[Teodor Petrov]

I don't remember. Probably with some api that gives access to these directly.

[bluehazzard]

access to these directly.

Access what directly? Are you still working on this? Respectively are you planning to complete this in the next time? I think this API feature is important and we should fix it. And if it is only temporary....

[Teodor Petrov]

Not at the moment. I can try to inspect this and see I can extract the hook related stuff. The original change would require quite a lot of work and testing and I don't have this time now. :(

Please ping me in a month, I hope I can find the time in this time frame.

[Walt]

@Teodor Petrov My patch does add one function to cbProject but that is surely backward compatible. I pondered the name of the function for some time. I don't think it invalidates the plugins. It allows plugins to refresh their configuration from the project. But I don't really mind what it is called and expected some feedback on the naming. It is very simple code which seems to resolve this issue. We currntly have an API call (AddToExtensions) that is ineffectual (in scripted wizard) because after it is called the values are wiped.

This bug is blocking project wizard being as effecive as it could.

[Teodor Petrov]

I've proposed a better name, I've not said that it invalidates extensions. As faras I can see it invalidates the m_pExtensionsElement and your new method reparses it.

But my desire is to fix the problem without a public method. Because
1. there may be other places where this needs to be called, but are not clear at the moment
2. the api will have to be removed in the future and all users will have to be fixed or they'll become incompatible/unbuildable.
3. I think it could be done with a some modified flag...

[JimH]

This is something I am interested in, but appears to have stalled. Any progress to report? Anything I can help with?

[bluehazzard]

I am with obfuscated and against the provided solution by adding a function to the porject class. I dont see the point in loading Plugins trought the project. This should happen from withtin
the scripted wizard, by calling a function that informs all plugins to reload the project.
If obfuscated removes the loader hooks we probably have to whait until he finished this work.
I also do not think that this can be solved with a flag, because the function AddToExtension does not know about the plugins it has changed and so it can not set the according flag for the plugin.

I am willing to invest time in this and fix it as soon as possible, but we have to find the right way to do it...

[JimH]

I am presuming the Obfuscvated==Teodor! I see he plans to have a look in the month timescale which would be good, but it appers this change has been in the ether for over a year, so anything we can do to help to speed this up, e.g. testing, we would be happy to put the effort in. Be a good chance for us to learn more about the codebase, and also get a requested bug fix in.