Menu

#83 codecompletion's setting not saved correctly

Next_Release
open
nobody
Bug_Report
2016-02-24
2014-11-02
ollydbg
No

Reported by OBF, and I have get the reason, but no idea how to fix it.

Discussion

  • Morten MacFly

    Morten MacFly - 2015-02-07
    • Type: --> Undefined
     
  • ollydbg

    ollydbg - 2016-02-19
    • Type: Undefined --> Bug_Report
     
  • ollydbg

    ollydbg - 2016-02-19
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,2 +1 @@
    -http://forums.codeblocks.org/index.php/topic,19753.msg134964/topicseen.html#msg134964
    -Reported by OBF, and I have get the reason, but no idea how to fix it.
    +[Reported by OBF](http://forums.codeblocks.org/index.php/topic,19753.msg134964/topicseen.html#msg134964), and I have get the reason, but no idea how to fix it.
    
     
  • ollydbg

    ollydbg - 2016-02-19

    Steps to reproduce this bug:
    1, open c::b
    2, open a simple project
    3, go to cc setting dialog, in the C/C++ parser page, change an option
    4, close the dialog, and close the simple project
    5, go to cc setting dialog, in the C/C++ parser page, you see the option is not changed yet

    I'm not sure what is the correct way to save options.

     
  • ollydbg

    ollydbg - 2016-02-19

    This is my analysis post: Re: The 27 October 2014 build (10016) is out., and OBF suggest that don't save the options of the temp parser, but I don't think this solve the issue.

    For example, when you open the C::B, and temp parser is created. Then you first open a.cbp, and then b.cbp, now, there are three instance of the parser object, when you open the cc dialog, you can only change the active project's parser setting, thus if you change the parser setting for b.cbp, after that, you just close b.cbp. OK, the setting is saved to configure file, but now, a.cbp is still active, and close a.cbp will agains save the setting from a.cbp, which still results the bug.

    Here are some solutions:
    1,
    one way to solve this is that we can "only" change the setting of the "temp parser", since when a parser is created, it will use the settings from the configure file, thus the setting in the temp parser get cloned.

    since we don't save the parser setting inside the cbp, which means we have only one single place (the conf file) to save the settings.

    2,
    Another way to solve this issue is that: we only save the parser setting to configure file when we close the cc setting dialog, not in the destructor of the parser instance.

    any one can give more ideas?
    Thanks.

     
  • Teodor Petrov

    Teodor Petrov - 2016-02-19

    I don't understand all the drama about this.

    To solve this:
    1. Decouple the settings from the parser object by extracting them in a separate object inside the CCManager.
    2. When the settings have changed then this means that all the parsers must be destroyed and recreated, so they can apply the new settings.
    3. On exit just store the settings from the new object.

    I think this should be doable. If not please explain why?

     
  • ollydbg

    ollydbg - 2016-02-21

    @OBF, you method make the code much simpiler, because all the options are only managed by ccmamanger, so you don't need to specify a different option for a single project(parser).

    I just use the git blame to look at some old revision:

    void Parser::ReadOptions()
    {
        ConfigManager* cfg = Manager::Get()->GetConfigManager(_T("code_completion"));
    
        // one-time default settings change: upgrade everyone
        bool force_all_on = !cfg->ReadBool(_T("/parser_defaults_changed"), false);
        if (force_all_on)
        {
            cfg->Write(_T("/parser_defaults_changed"),       true);
    
            cfg->Write(_T("/parser_follow_local_includes"),  true);
            cfg->Write(_T("/parser_follow_global_includes"), true);
            cfg->Write(_T("/want_preprocessor"),             true);
            cfg->Write(_T("/parse_complex_macros"),          true);
            cfg->Write(_T("/platform_check"),                true);
        }
        ...
        ...
    

    The option "parser_defaults_changed" looks come from an very old commit 2872:

    SHA-1: 7887b756ab6d77beb357c31e185032237ff1eb6a
    
    * * Changed default settings for code-completion parser: parse local includes, global includes and preprocessor symbols are enabled now. Everyone's settings will be changed to these defaults.
    - Changed code-completion plugin version number. Was unfair to still be at 0.1 ;).
    
    git-svn-id: http://svn.code.sf.net/p/codeblocks/code/trunk@2872 2a5c6006-c6dd-42ca-98ab-0921f2732cef
    

    So, I guess this commit (mandrav) did add some mechanism that we keep a default parsing options. When a user open the CC dialog, he is seeing the parser option for the current parser. I think maybe, we need another page, which show the "default option".

     
  • Teodor Petrov

    Teodor Petrov - 2016-02-21

    Why clutter the ui even more? If you want to have default options, just add a reset button somewhere.

     
  • ollydbg

    ollydbg - 2016-02-24

    I agree, adding a new page is a bad idea, I think two radio bottons are required here.

    [radio button 1] setting for active parsers
    [radio button 2] setting for default parsers
    

    When the CC setting dialog is opened, the radio button 1 is seleted, and user can change the setting for the current parser, but those settings won't be saved to the configure files. If the radio button 2 is selected, then the default setting is shown, and user can change those values, and will be saved to the conf files.

    But currently I don't have much motivation to fix this issue, because we have many other important issues in CC.

     
  • Teodor Petrov

    Teodor Petrov - 2016-02-24

    I still don't understand why you want to change the settings of the active parsers?
    And why do you want to introduce such complex scheme?
    My proposal is pretty simple and robust, why don't you like it?

     
    • ollydbg

      ollydbg - 2016-02-28

      I still don't understand why you want to change the settings of the active parsers?

      I want to keep the old bad code made by mandrav, but it looks like this make code more complex.

      And why do you want to introduce such complex scheme?
      My proposal is pretty simple and robust, why don't you like it?

      I think I will follow your advice, thanks!

       

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.