Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
New projects "Custom Vars" tab
oBFusCATed:
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.
earlgrey:
--- Quote from: oBFusCATed 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.
--- End quote ---
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.
--- End quote ---
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.
--- End quote ---
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.
--- End quote ---
I dont understand, be more precise please.
oBFusCATed:
--- Quote from: earlgrey on November 20, 2019, 08:05:53 pm ---No, my goal is to modernize the CustomVars UI. CustomVars are very important because they avoid the proliferation of EnvVars.
--- End quote ---
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.
oBFusCATed:
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.
earlgrey:
--- Quote from: oBFusCATed on November 20, 2019, 08:47:42 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.
--- End quote ---
Its a lot of work, if you are really interested I can work on it.
[EDIT]
And thanx for your time and the review !
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version