2008/11/6 Nick Treleaven
<nick.treleaven@btinternet.com>
Hi,
Sorry for the delay. (As I said in another mail, my net connection was
broken for 2 weeks.)
> I have committed a prototype of the first part of the changes
> proposed for the build system to
> the build system branch.
Cool. I just made some minor commits to fix a warning and refactor
some GUI code.
Ok, warning that this is still a prototype, GUI code is still being changed to get the full functionality shown in the geany.txt I committed recently.
BTW there are some formatting issues:
1. Some spaces instead of tabs being used for indentation (e.g. in
build.c).
2. Spacing - instead of:
if( blah!=foo )some_func( foo );
use:
if (blah != foo) some_func(foo);
Please use the existing code style (see HACKING).
Do you have a configuration for one of the usual source formatters to get the format you want, I AM going to continue to get it wrong because it is different to the standard format I use all day every day at work. The fingers just automatically put spaces on the other side of the ( ) etc. Even if I'm thinking about it, it will get done wrong from time to time ;-). You are correct that it should be consistent within Geany so if there is a formatter that will generate the format used throughout Geany I will ensure that I use it.
The indentation is the most important one, could you fix this please?
Well, all editing has been done in Geany with indentation set to tabs so I don't know where the problem came from??? but I can fix it.
About the code:
1. No real need to check if ptr is null before g_free(ptr).
Ok
2. I think we should move the new build_* GeanyProject fields to a
GeanyProjectPrivate struct, similar to
GeanyDocumentPrivate/GeanyFiletypePrivate. The implementation of those
fields might change, so they shouldn't be in the plugin API. I can make
this change unless someone disagrees.
Don't worry, I'll do that because I have already made some changes in that area to support the full functionality. No point in doing it on the prototype when there are more changes to come.
> If a project is open the dialog will have extra fields to provide
> labels and commands to replace
> the "Make" series of menu items on the build menu.
(I guess replacing the make menu items is not implemented yet.)
Did you have a project open? the prototype only adds these items to the menu or the build commands dialog when a project is open. The full version will also show them for preferences which isn't implemented in this version. Coming real soon now though :-) maybe
And underscores in the user defined labels will set the accelerator correctly too.
> On the menu any
> label or command which
> is not set will revert to the default (the same as without any
> project). By default no labels or
> commands are set in the project file, so unless the user does
> something (sets them) there
> will be no change. These labels and commands are saved and restored
> with the project file.
Maybe by default they should be set to the default make command
labels and commands. Otherwise the dialog is unintuitive - just empty
boxes.
The full version will work that way, see the manual. Note these are generated if there is nothing saved in the project or preferences. That way the menu label strings are translated like any string is. It is assumed that when a user edits a label they will do it in the language they want and so no translation is needed for the label saved in project or preferences, but it is important that there is no default value saved that isn't translated.
> Note when using commands set in the project file, the "make" command
> path in tools preferences
> is not used so that each command can have a different path. Therefore
> the command entered in
> the "Set Build Menu Commands" dialog must include any path it
> requires. Any command that
> reverts to default will still use the "make" command path so that it
> operates as usual.
OK.
> At the moment there is no way to reset project commands to default
> without hand editing the
> project file.Setting them to blank does just that, it doesn't reset to
> default. A "reset to default"
> button will be added.
Hmm, why not just use the default if they're blank?
Because in the full version blank means don't show this menu item, this makes the build commands operate the same as the filetype commands which do not show unused commands.
NOTE: this brings up one significant change needed for the full version, the can compile, can build, flags of the filetype will no longer be used since the existance (or not) of such a command is what determines if a menu item is shown. Otherwise users would not be able to add filetype commands to unused filetype menu items. So in the full version, a commandwhich is not set will mean use the default whilst one set to nothing (item name) will mean don't use. Have to allow 'command set to nothing' just in case someone wants to use the make custom dialog to type the whole command, possibly useful for occasionally used commands whilst still capturing and parsing the command output.
But are there any other important uses of these flags besides controlling what is shown in the build menu?
Best Regards
Lex
Regards,
Nick