On Thu, 8 Jul 2010 23:59:04 +0200 Jiří Techet techet@gmail.com wrote:
Hi Nick,
thanks for looking at the patches. Let me know if I should re-send them by email or modify them in some way.
Can you send the patches by email please - my git-fu is not yet good enough (slowly improving though).
I'll try to give a very brief comment on each/some below without really reading the code.
That's why some of my comments were vague - I mostly didn't look at the code changes.
Use relative paths in the project files
Hmm, I thought there was something like this already, but haven't checked, so maybe.
No, there's nothing like that. I understand that using full paths may be useful in some special cases to move the project file separately from the project files to a completely different place without breaking things, but in my opinion being able to move a project together with the project file to a different directory (PC) without breaking the paths is much more useful.
OK.
Don't be annoying when not necessary
When reloading a file with ctrl+R don't display the warning dialog that the unsaved changes might be lost when the file has not been modified.
What if you accidentally pressed Ctrl-R? You could lose work.
No, what the patch does is that it checks whether the file is modified (and not saved) and pops up the dialog if and only if it is modified. So there should be no danger that you lose your work in progress.
OK, I didn't realise.
(The patch (mis)uses short evaluation of boolean expressions in C so this is probably what you have overlooked.)
Looks fine to me, I didn't check the code change before.
Make it possible to change project patterns by plugins
Plugins can override project patterns. In that case the "file patterns" field is made inactive in the Project tab of the project properties dialog.
So this patch also shows the currently hidden field too. Maybe OK
Right. The field is made insensitive when the patterns are overridden by plugins. Also what I changed is that the patterns are space-separated like in the FIF dialog (and not newline-separated) to be consistent.
OK.
Use project patterns in the FIF dialog
When "use project patterns" is selected, the project patterns are copied to the "files" edit box and it becomes not editable. It should be considered whether the "files" checkbox should exist - it may not be clear if the edit box is greyed out because "use project patterns" is selected or because "files" is unselected. The "use project patterns" checkbox is inactive when no project is loaded.
Maybe
Hmm, I still need to check the code properly, but I'm having second thoughts about this. Simpler alternatives would be:
1. Always append project patterns to the Files drop-down list on showing the dialog (if it's not already there), it's up to the user to choose it.
2. An API function to show the dialog with an argument for file patterns.
The advantage would be that the dialog is simpler - less controls & complexity.
Make the project menu accessible by plugins
Not really a hard requirement but since the project plugin settings can now be also integrated to the project settings dialog, it looks strange when the project-related items are in the Tools menu.
What items do you want the plugin to put in the Project menu?
OK.
(The "generate tags" entry is related to my other plugin, which is currently a work in progress. With this plugin you can generate the ctags file using this entry (all files under the base path satisfying the patterns are included) and then, use the editor's context menu to find the given tag. You can find the result of such a search in the message window in the screenshot. The tag manager that geany uses is too slow for big projects so I can't use it for this purpose.)
I'm not sure that could integrate with Geany's tag functionality, but it's up to you.
Add signals for project options dialog opening and closing
These signals can be used by plugins to add their settings tab and read the settings when the user presses OK. The code had to be reorganized slightly because first project-dialog-confirmed has to be emitted (so the plugin can read the settings) and project-save afterwards.
Maybe OK - I think project_dialog_confirmed may be unnecessary as plugins could connect to the dialog response signal from the project_dialog_create callback.
Well, I emit this signal only if the dialog was successfully validated by geany. So for instance if you specify an invalid project name in the dialog, the signal is not emitted until you correct it and press OK again. In what you propose it would be emitted every time I guess.
OK, good point, and it allows the validator API to be added later if we decide to.
Connecting to the dialog response signal is what the plugin_configure symbol currently does, so probably we should add a plugin_configured symbol and encourage users to use that instead, for the same reason (validators).
http://www.geany.org/manual/reference/pluginsymbols_8c.html#a5d574ade6576014...
Prevent -Wmissing-prototypes report warning when compiling a plugin
We discussed this before. A plugin would also need prototypes for the other functions like plugin_init. So I said we could change the doc/pluginsymbols.c file into a header to do this.
I know, this is a complete set of patches I use against geany.
But I don't understand why the plugins would need doc/pluginsymbols.c to be changed to doc/pluginsymbols.h - I thought this was needed if geany itself was compiled with the flag (I haven't tried it myself so I don't know what's needed for that).
It's currently just for generating documentation.
plugin_init() has to be implemented by the plugin so all the plugin has to do is to declare the function as well before it implements it. At least for gproject this patch is all I need to compile it without warnings.
I thought it would be easier to avoid having to declare e.g. plugin_init and other function symbols.
But I might apply the patch.
Regards, Nick