User forums > Using Code::Blocks

Hiccups while typing (continuation)

<< < (7/8) > >>

Pecan:
@ ollydbg

I stopped overthinking this, simplified, and came up this the 7th inning. I think this is a homerun.
Give it a try. Thanks.


--- Code: ---Index: classbrowser.cpp
===================================================================
--- classbrowser.cpp (revision 13611)
+++ classbrowser.cpp (working copy)
@@ -46,7 +46,7 @@
 
 #include "parser/ccdebuginfo.h"
 
-#include <stack>
+//unused #include <stack>
 #include <chrono>
 
 #define CC_CLASS_BROWSER_DEBUG_OUTPUT 0
@@ -242,6 +242,9 @@
             filter = bdfProject;
 
         m_Parser->ClassBrowserOptions().displayFilter = filter;
+        // Update stale CB globals in TempParser before WriteOptions() //(ph 2025/02/10)
+        if (m_Parser == m_ParseManager->GetTempParser())
+            m_Parser->ReadOptions();
         m_Parser->WriteOptions();
         UpdateClassBrowserView();
     }
Index: parsemanager.h
===================================================================
--- parsemanager.h (revision 13611)
+++ parsemanager.h (working copy)
@@ -9,8 +9,8 @@
 #include "parsemanager_base.h"
 #include "parser/parser.h"
 
-#include <queue>
-#include <map>
+//unused #include <queue>
+// unused #include <map>
 #include <memory>
 #include <unordered_map>
 
@@ -257,6 +257,7 @@
     //(ph 2024/01/25)
     void SetSymbolsWindowHasFocus(bool trueOrFalse){ m_SymbolsWindowHasFocus = trueOrFalse;}
     bool GetSymbolsWindowHasFocus(){return m_SymbolsWindowHasFocus;}
+    ParserBase* GetTempParser(){return m_TempParser;} //(ph 2025/02/10)
 
 protected:
     /** When a Parser is created, we need a full parsing stage including:
Index: resources/manifest.xml
===================================================================
--- resources/manifest.xml (revision 13611)
+++ resources/manifest.xml (working copy)
@@ -3,7 +3,7 @@
     <SdkVersion major="1" minor="10" release="0" />
     <Plugin name="CodeCompletion">
         <Value title="Code completion" />
-        <Value version="1.0.5 24/01/29" />
+        <Value version="1.0.6 25/02/10" />
         <Value description="This plugin provides a symbols browser for your projects and code-completion inside the editor.
 
 


--- End code ---

ollydbg:
Hi, Pecan, thanks for your hard work!
I debugged your patch yesterday. I think this patch is simple, but it still has some errors. Let me describe the issue again. I think in our code, we just "write" or "read" from the .conf file too much. I think every Parser instance's option is valid, so I don't think there is some "stale" option.

The problem is that we have several Parser instances. For example, we have ParserA, ParserB, ParserTemp. When there is no cbp opened, the ParserTemp always exists. I think when every Parser instance get constructed, it will "read" the options from the .conf file, that's OK.



--- Code: ---                                  +----------+     
         +------------------------|Parser A  |     
         |                        +----------+     
 +-------|------+                                 
 |Parse option  |                 +----------+     
 |Browser option|-----------------|Parser B  |     
 +-------|------+                 +----------+     
         |                                         
         |                        +-----------+   
         +------------------------|Parser Temp|   
                                  +-----------+   

--- End code ---

1, When the Parser object get constructed, all the option should be copied from the .conf file to the object.

2, When the user open a editor->CodeCompletion option dialog, and edit some options, and click the "OK" button (in the OnApply function), I think all the options will be saved.

3, When we switch the Parser object(for example, the active project is switched from A to B, or from A to Temp), when switching, we don't need to "read all the options from the .conf file", because different Parser maintain its own option.

4, When user tweak the ClassBrowser option, I think it should save to the .conf file immediately after changing the ClassBrowser option.

5, When a Parser get closed, I think we don't need to "save" its option to the .conf file", because those options were either saved in the above "2" and "4" steps.

Pecan:
@ ollydbg

Thanks for the clarity of that last message.
Here's the result of the re-work. For CB global settings:
1) write changes to .conf when user makes a change.
2) Do not write CB globals when closing a single file or when a parser is closing.
3) The TempParser must be updated when all parsers are closed else it writes its stale global settings (when CodeCompletion plugin initialized) to the .conf .


--- Code: ---Index: ccoptionsdlg.cpp
===================================================================
--- ccoptionsdlg.cpp (revision 13611)
+++ ccoptionsdlg.cpp (working copy)
@@ -183,6 +183,12 @@
 
 void CCOptionsDlg::OnApply()
 {
+    cbProject* pProject = Manager::Get()->GetProjectManager()->GetActiveProject();
+     // Remember the project that changed the .conf data //(ph 2025/02/04)
+    m_ParseManager->SetOptsChangedByProject(pProject);
+    // Renember the Parser that changed the .conf data //(ph 2025/02/04)
+    m_ParseManager->SetOptsChangedByParser(&(m_ParseManager->GetParser())); //(ph 2025/02/07)
+
     ConfigManager* cfg = Manager::Get()->GetConfigManager(_T("code_completion"));
 
     // -----------------------------------------------------------------------
Index: parsemanager.cpp
===================================================================
--- parsemanager.cpp (revision 13611)
+++ parsemanager.cpp (working copy)
@@ -623,6 +623,7 @@
         // if the active parser is deleted, set the active parser to nullptr
         if (it->second == m_Parser)
         {
+            SetClosingParser(m_Parser); //(ph 2025/02/12)
             m_Parser = nullptr;
             SetParser(m_TempParser); // Also updates class browser
         }
Index: parsemanager.h
===================================================================
--- parsemanager.h (revision 13611)
+++ parsemanager.h (working copy)
@@ -9,8 +9,8 @@
 #include "parsemanager_base.h"
 #include "parser/parser.h"
 
-#include <queue>
-#include <map>
+//unused #include <queue>
+// unused #include <map>
 #include <memory>
 #include <unordered_map>
 
@@ -258,6 +258,16 @@
     void SetSymbolsWindowHasFocus(bool trueOrFalse){ m_SymbolsWindowHasFocus = trueOrFalse;}
     bool GetSymbolsWindowHasFocus(){return m_SymbolsWindowHasFocus;}
 
+    // Set or return Project that changed "Global setting" in workspace
+    cbProject* GetOptsChangedByProject(){ return m_pOptsChangedProject;}
+    void SetOptsChangedByProject(cbProject* pProject){m_pOptsChangedProject = pProject;}
+    // Set or return Parser that changed "Global setting" in Single File workspace
+    ParserBase* GetOptsChangedByParser(){ return m_pOptsChangedParser;}
+    void SetOptsChangedByParser(ParserBase* pParserBase){m_pOptsChangedParser = pParserBase;}
+    ParserBase* GetTempParser(){return m_TempParser;}
+    ParserBase* GetClosingParser(){return m_pClosingParser;} //(ph 2025/02/12)
+    void        SetClosingParser(ParserBase* pParser){m_pClosingParser = pParser;} //(ph 2025/02/12)
+
 protected:
     /** When a Parser is created, we need a full parsing stage including:
      * 1, parse the priority header files firstly.
@@ -474,11 +484,12 @@
      */
     bool RemoveProjectFromParser(cbProject* project);
 
+
 private:
     typedef std::pair<cbProject*, ParserBase*> ProjectParserPair;
     typedef std::list<ProjectParserPair>       ParserList;
 
-    /** a list holing all the cbp->parser pairs, if in one parser per project mode, there are many
+    /** a list holding all the cbp->parser pairs, if in one parser per project mode, there are many
      * many pairs in this list. In one parser per workspace mode, there is only one pair, and the
      * m_ParserList.begin()->second is the common parser for all the projects in workspace.
      */
@@ -527,6 +538,13 @@
     bool m_ClassBrowserViewIsStale = true;
     bool m_SymbolsWindowHasFocus = false;
 
+    //The latest project to change the .conf file //(ph 2025/02/04)
+    cbProject* m_pOptsChangedProject = nullptr;
+    //The latest parser to change the .conf file //(ph 2025/02/04)
+    ParserBase* m_pOptsChangedParser = nullptr;
+    // the currently closing parser
+    ParserBase* m_pClosingParser = nullptr;
+
 };
 
 #endif // PARSEMANAGER_H
Index: parser/parser.cpp
===================================================================
--- parser/parser.cpp (revision 13611)
+++ parser/parser.cpp (working copy)
@@ -32,9 +32,10 @@
 
 #include "parser.h"
 #include "parserthreadedtask.h"
+#include "../parsemanager.h" //(ph 2025/02/04)
 
 #include "../classbrowser.h"
-#include "../classbrowserbuilderthread.h"
+//unused - #include "../classbrowserbuilderthread.h"
 
 
 #ifndef CB_PRECOMP
@@ -921,19 +922,64 @@
 
 void Parser::WriteOptions()
 {
+     // Global settings bug fix (ph 2025/02/12)
+     //https://forums.codeblocks.org/index.php/topic,25955 Hiccups while typing
+
+    // Assemble status to determine if a Parser or Project changed a global setting.
+    ParseManager*   pParseMgr = (ParseManager*)m_Parent;
+    ParserBase*     pTempParser = pParseMgr->GetTempParser();
+    ParserBase*     pClosingParser = pParseMgr->GetClosingParser(); //see ParseManger::DeleteParser()
+    ParserBase*     pCurrentParser = &(pParseMgr->GetParser());     //aka: m_parser
+
+    bool isClosingParser  = pCurrentParser == pClosingParser;
+    bool isTempParser     = pTempParser == pCurrentParser;
+    bool globalOptionChanged = pParseMgr->GetOptsChangedByParser() or pParseMgr->GetOptsChangedByProject();
+
+    // If this is a parser close, do not allow CB global settings writes.
+    bool allowGlobalUpdate = false;
+
+    // When no closing parser, but CB globals were changed, write to .conf
+    if ( (not isClosingParser) and globalOptionChanged)
+        allowGlobalUpdate = true;
+
+    // Closing parsers are not allowed to write to CB globals.
+    //  CB Globals were already written when when user changed the setting.
+    if (isClosingParser) allowGlobalUpdate = false;
+    // If no changes to the CB globals, no need to write
+    if (not globalOptionChanged)
+        allowGlobalUpdate = false; // no global settings have changed
+
     ConfigManager* cfg = Manager::Get()->GetConfigManager(_T("code_completion"));
 
-    // Page "Code Completion"
-    cfg->Write(_T("/use_SmartSense"),                m_Options.useSmartSense);
-    cfg->Write(_T("/while_typing"),                  m_Options.whileTyping);
+    // ----------------------------------------------------------------------------
+    // set any user changed CB global settings for CodeCompletion
+    // ----------------------------------------------------------------------------
+    if (allowGlobalUpdate)
+    {
+        // Page "Code Completion"
+        cfg->Write(_T("/use_SmartSense"),                m_Options.useSmartSense);
+        cfg->Write(_T("/while_typing"),                  m_Options.whileTyping);
 
-    // Page "C / C++ parser"
-    cfg->Write(_T("/parser_follow_local_includes"),  m_Options.followLocalIncludes);
-    cfg->Write(_T("/parser_follow_global_includes"), m_Options.followGlobalIncludes);
-    cfg->Write(_T("/want_preprocessor"),             m_Options.wantPreprocessor);
-    cfg->Write(_T("/parse_complex_macros"),          m_Options.parseComplexMacros);
-    cfg->Write(_T("/platform_check"),                m_Options.platformCheck);
+        // Page "C / C++ parser"
+        cfg->Write(_T("/parser_follow_local_includes"),  m_Options.followLocalIncludes);
+        cfg->Write(_T("/parser_follow_global_includes"), m_Options.followGlobalIncludes);
+        cfg->Write(_T("/want_preprocessor"),             m_Options.wantPreprocessor);
+        cfg->Write(_T("/parse_complex_macros"),          m_Options.parseComplexMacros);
+        cfg->Write(_T("/platform_check"),                m_Options.platformCheck);
 
+    }
+    // clean out any parser flags used to guard CB global settings
+    pParseMgr->SetOptsChangedByParser(nullptr);  //(ph 2025/02/12)
+    pParseMgr->SetOptsChangedByProject(nullptr); //(ph 2025/02/12)
+    pParseMgr->SetClosingParser(nullptr);
+    // if currrent parser == TempParser, force it to update, else the next
+    // display of the setting dialog will show TempParser stale cached settings.
+    if (isTempParser) pTempParser->ReadOptions();
+
+    // ----------------------------------------------------------------------------
+    // set any user changed ClassBrowser settings
+    // ----------------------------------------------------------------------------
+
     // Page "Symbol browser"
     cfg->Write(_T("/browser_show_inheritance"),      m_BrowserOptions.showInheritance);
     cfg->Write(_T("/browser_expand_ns"),             m_BrowserOptions.expandNS);
Index: resources/manifest.xml
===================================================================
--- resources/manifest.xml (revision 13611)
+++ resources/manifest.xml (working copy)
@@ -3,7 +3,7 @@
     <SdkVersion major="1" minor="10" release="0" />
     <Plugin name="CodeCompletion">
         <Value title="Code completion" />
-        <Value version="1.0.5 24/01/29" />
+        <Value version="1.0.6 25/02/12" />
         <Value description="This plugin provides a symbols browser for your projects and code-completion inside the editor.
 
 


--- End code ---

ollydbg:
Hi, thanks for your work.

I tested this patch. One issue I see is:

when I have projectA loaded, and later projectB loaded. Now, if I active projectB active, and I open the CC setting(option) dialog, change some settings, and click the apply button, the settings were saved to the .conf file. In this time, the ParserA and ParserB actually have different Parser settings. Now, if I switch the active Parser to projectA, and if I open the CC option dialog again, the setting is the new modified projectB's CC settings. Suppose in this case, the ParserA already finished its parsing, and its Parsing option is not the options when you see in the CC setting dialog.

So, what is the time we need to "read the Parser option from the .conf to the Parser instance", I think it should be in the constructor, and this should be the only place a Parser instance to read the initial parsing option.

Another thing I see which may be not correct (in the old code, not in your patch) is in this function:


--- Code: ---// ----------------------------------------------------------------------------
void ClassBrowser::SetParser(ParserBase* parser)
// ----------------------------------------------------------------------------
{
    if (m_Parser == parser)
        return;

    m_Parser = parser;
    if (m_Parser)
    {
        const int sel = XRCCTRL(*this, "cmbView", wxChoice)->GetSelection();
        BrowserDisplayFilter filter = static_cast<BrowserDisplayFilter>(sel);
        if (!m_ParseManager->IsParserPerWorkspace() && filter == bdfWorkspace)
            filter = bdfProject;

        m_Parser->ClassBrowserOptions().displayFilter = filter;
        m_Parser->WriteOptions();
        UpdateClassBrowserView();
    }
    else
        CCLogger::Get()->DebugLog("SetParser: No parser available.");
}

--- End code ---

When we switch the Parser in the class browser, why do we need to "WriteOptions()"? I think we should only call something like "WriteClassBrowserOptions()", because I think switch the Parser only need to write the ClassBrowserOptions to the .conf file, but not the whole Parser options.

Pecan:

--- Quote from: ollydbg on February 13, 2025, 07:41:00 am ---So, what is the time we need to "read the Parser option from the .conf to the Parser instance", I think it should be in the constructor, and this should be the only place a Parser instance to read the initial parsing option.

--- End quote ---

I don't understand this request at all.
 
If a parser only reads the settings at parser creation time, when the user later changes the global settings but the parser is not allowed to read them after the change, the user now is parsing without the settings he/she requested.

If that user was me, I'd call that a bug.

I think it would be better to throw up a message box saying "Sorry, sniff, global settings will not be applied for a project/parser until you request a re-parse of all the other loaded projects." Just kidding...

This is a hard problem. If we don't apply changes, we violate UI standards. If we do change them, a busy parser won't apply them anyway.

Maybe we should put up a message box that says, these changes will only be applied on the next re-parse for a project.
We could list the projects that have a 'busy' not done flag.

I remember this situation from about 13 years ago. The client asked me to throw up a message box that said "Options cannot be changed while a workspace is loaded".

Navigation

[0] Message Index

[#] Next page

[*] Previous page