Author Topic: New projects "Custom Vars" tab  (Read 13394 times)

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
New projects "Custom Vars" tab
« on: November 18, 2019, 08:15:53 pm »
Hello everyone.

I made ( for me ) a new "Custom vars" panel in the "Build options" configuration panel.
It looks like this :



A minimalistic doc is at https://github.com/earlgrey-bis/erg.cbproject-custom-vars/tree/doc

Are you interested.

Regards.
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: New projects "Custom Vars" tab
« Reply #1 on: November 18, 2019, 08:39:46 pm »
Are this the variables for the global variables (Settings->Variables)? Or are this some custom variables? If custom: why not use a UI for the global variables? This was on my todo list for a long time.... Project specific global variables with notes... the project specific variables would only be used if no global variable is set...
« Last Edit: November 18, 2019, 08:41:37 pm by BlueHazzard »

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #2 on: November 18, 2019, 09:16:13 pm »
Are this the variables for the global variables (Settings->Variables)?
No
Quote
Or are this some custom variables?
Yes, Project->Build options->Custom variables. It replaces the current "Custom variables" tab
Quote
If custom: why not use a UI for the global variables?...
I dont get you
==============================================================================================
Custom variables are per-project, saved in the project file ( .cbp ).
Global variables, Environment variables are attached to an OS user, saved in C::B configuration files ( .config/codeblocks/default.conf for me )

* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline BlueHazzard

  • Developer
  • Lives here!
  • *****
  • Posts: 3353
Re: New projects "Custom Vars" tab
« Reply #3 on: November 19, 2019, 11:47:52 am »
I like this GUI.... I would add a "comment" or "note" column (also to the project file)

I am not quite sure what obfuscated thinks about the wxDataViewListCtrl, because in the past it was quite buggy on different platforms...

I also do not know if we want to add this before the next release, but in general i really like this! Can you create a patch?

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #4 on: November 19, 2019, 06:56:36 pm »
I like this GUI.... I would add a "comment" or "note" column (also to the project file)
Should be possible with a thing like this in compileoptionsbase.h:
Code
typedef struct 
{
    wxString    a_value;
    wxString    a_comment;
} wxString2;
WX_DECLARE_STRING_HASH_MAP(wxString2, String2Hash);
And with a slight modification of all code parts that uses CompileOptionsBase::m_Vars. There are not too many apparently.

Quote
I am not quite sure what obfuscated thinks about the wxDataViewListCtrl, because in the past it was quite buggy on different platforms...
U right, that is the real problem

Quote
I also do not know if we want to add this before the next release, but in general i really like this! Can you create a patch?
Patches for each file are in the "pub" branch. There is a script for linux that apply the patches automatically.
Should I rather create a patch for the whole trunk ?
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New projects "Custom Vars" tab
« Reply #5 on: November 19, 2019, 09:01:37 pm »
If you want to provide patches which could be reviewed it is best to make a git branch from a relatively new master (see my mirror: https://github.com/obfuscated/codeblocks_sf ).

I have no opinion on wxDVC probably it is too buggy in wx2.8 and wx3.0, but if its bugs doesn't affect the specific usage requirements for this case, then all is fine.

I'm not sure, but I guess I'd prefer if this was applied to the global variables and not to the custom variables. Also per workspace override should be possible, too.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #6 on: November 20, 2019, 08:05:53 pm »
If you want to provide patches which could be reviewed it is best to make a git branch from a relatively new master.
Done, a branch called erg.customvars-01-base is at https://github.com/earlgrey-bis/obfuscated.cb/tree/erg.customvars-01-base
Quote
I have no opinion on wxDVC probably it is too buggy in wx2.8 and wx3.0, but if its bugs doesn't affect the specific usage requirements for this case, then all is fine.
Simple and basic usage of wxDVC, seems to works fine.
Quote
...I guess I'd prefer if this was applied to the global variables and not to the custom variables.
No, my goal is to modernize the CustomVars UI. CustomVars are very important because they avoid the proliferation of EnvVars.
Quote
Also per workspace override should be possible, too.
I dont understand, be more precise please.
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New projects "Custom Vars" tab
« Reply #7 on: November 20, 2019, 08:26:19 pm »
No, my goal is to modernize the CustomVars UI. CustomVars are very important because they avoid the proliferation of EnvVars.
CustomVars are stored in the project, so they cannot contain per user information without making life hard for projects developed by more then 1 person.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New projects "Custom Vars" tab
« Reply #8 on: November 20, 2019, 08:47:42 pm »
Review comments:

General stuff:
1. no underscores in variable names
2. no too much alignment (I don't understand why people want to align unrelated if-then statements, but I guess this is just me)
3. no abbreviations dvlc and dvls doesn't mean too much if you don't know what are they in the first place
4. use range-based for loops whenever possible, but no auto variables...
5. no single line if statements

Patch specific:
1. Public apis must be documented, no matter that the surrounding apis aren't.
2. Why do you need a second map? Why don't you make a structure and add a bool? Or just add some kind of a set with inactive or active variables? Prefer hash based containers if you don't need sorting.
3. xrc files are generated by tools, don't edit them by hand and don't leave commented stuff in them
4. Why have you changed the names of the buttons? Add and Edit are pretty common in C::B?
5. You need to provide better commit message.
6. Why are the new api calls virtual?
7. These api calls must be exposed to scripting.

(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #9 on: November 20, 2019, 09:46:22 pm »
Patch specific:
2. Why do you need a second map? Why don't you make a structure and add a bool? Or just add some kind of a set with inactive or active variables?
Because it keeps the m_Vars member intact for the rest of C::B code. Can be done ( and therefore add var comments ) but will change more C::B code.

3. xrc files are generated by tools, don't edit them by hand and don't leave commented stuff in them
Somebody give hints please for UI & .xrc modif please.

4. Why have you changed the names of the buttons? Add and Edit are pretty common in C::B?
"Clear" is not very comprehensive : does it delete the vars or just clear their content ? Or just one var ?
=> renamed to "Delete All"
=> Leads to keyboard shortcut problem ( D = "Delete", A = "Add", so what for "Delete All" ? )
=> "Add" renamed to "New" ( which is more appaired with "Delete" )
Current setup :
NEW -  BROWSE -  DELETE  - DELETE ALL
Tell what you want have.

6. Why are the new api calls virtual?
Because CompileOptionsBase:: methods related to m_Vars are. I asked myself the same question about them, because they are not overloaded anywhere.

7. These api calls must be exposed to scripting.
Somebody give hints please.


Its a lot of work, if you are really interested I can work on it.

[EDIT]
And thanx for your time and the review !
« Last Edit: November 20, 2019, 10:18:24 pm by earlgrey »
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New projects "Custom Vars" tab
« Reply #10 on: November 20, 2019, 10:19:38 pm »
I don't see anything which prevents this from being merged, but it will have to wait for after the release.

BTW why are you using dataviewlistctrl? Why don't you use listctrl with checkboxes?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #11 on: November 20, 2019, 10:59:55 pm »
I don't see anything which prevents this from being merged, but it will have to wait for after the release.
So I have time to fit the codestyle compliance. And will use wxSmith to modify the xrc file.
Quote
BTW why are you using dataviewlistctrl? Why don't you use listctrl with checkboxes?
Was the same as with GTK : widget, model, renderers.

* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #12 on: November 24, 2019, 08:14:55 pm »
This was on my todo list for a long time.... Project specific global variables with notes...
I got this working :


( Maybe should I have written demonstrates ? )

 I think it is complete now.

Should I merge this feature in my master dev branch ?
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: New projects "Custom Vars" tab
« Reply #13 on: November 25, 2019, 12:19:05 am »
No, merges please. SVN doesn't support them. Just post a branch for review (just post the link to the branch here). If you don't want this to be forgotten open a ticket on sf.net about this, too.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline earlgrey

  • Multiple posting newcomer
  • *
  • Posts: 101
Re: New projects "Custom Vars" tab
« Reply #14 on: November 27, 2019, 10:15:13 pm »
Hello C::B devs. It is my intention to replace the old code for looping among CompileOptionsBase vars :
Code
const CustomVarHash& v = object->GetAllVars();
for (CustomVarHash::const_iterator it = v.begin(); it != v.end(); ++it)
with a loop like this :
Code
for ( CustomVarHash::const_iterator * it = object->VarEnumGetFirst(properties_flags) ; it != nullptr ; it = object->VarEnumGetNext() )
Motivation : adding active / inactive ( or any other ) property to vars leads to select the vars you want to enumerate by their properties. I want to keep this selection in CompileOptionsBase.
Would the following piece of code be accepted in C::B, because it has static vars in cpp module and it is not STL container enum. Thanx.
N.B. I am aware that m_Vars and m_ActiveVars can be merged.

Code
~ in compileoptionsbase.cpp ~
//  these variables because in CB code, GetAllVars() is assumed const. So we cant define these vars as class members, since we modify them during the enumeration
static  int                                 m_VarEnumFlags;
static  CustomVarHash const             *   m_VarEnumHash;
static  CustomVarHash::const_iterator       m_VarEnumIterator;

Code
~ Little helper ~
void CompileOptionsBase::VarEnumInit(CustomVarHash const * _i_hash) const
{
    m_VarEnumHash       =   _i_hash;
    m_VarEnumIterator   =   m_VarEnumHash->begin();
}

Code
~ main search ~
CustomVarHash::const_iterator* CompileOptionsBase::VarEnumFind() const
{
    while ( m_VarEnumIterator != m_VarEnumHash->end() )
    {
        if ( m_VarEnumIterator->second.flags & m_VarEnumFlags )   ~ this is the test I want to keep in CompileOptionsBase ~
            return &m_VarEnumIterator;

        m_VarEnumIterator++;
    }

    if ( m_VarEnumHash == &m_InactiveVars )
        return nullptr;

    VarEnumInit(&m_InactiveVars);
    return VarEnumFind();
}

Code
~ for(...) impl ~
CustomVarHash::const_iterator* CompileOptionsBase::VarEnumGetFirst(int _i_flags) const
{
    m_VarEnumFlags = _i_flags;

    VarEnumInit(&m_ActiveVars);
    return VarEnumFind();
 }

CustomVarHash::const_iterator* CompileOptionsBase::VarEnumGetNext() const
{
    m_VarEnumIterator++;
    return VarEnumFind();
}
* OS = Debian Buster - Linux 4.19.06 x64 SMP
* C::B = svn11267 wx-3.0.4 - Linux, unicode 64 bit