[Geany-devel] GProject - the second attempt
nick.treleaven at xxxxx
Fri Jul 9 11:57:09 UTC 2010
On Thu, 8 Jul 2010 23:59:04 +0200
Jiří Techet <techet at 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
> >> 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.
> >> 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.
> >> 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
2. An API function to show the dialog with an argument for file
The advantage would be that the dialog is simpler - less controls &
> >> 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?
> (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
> >> 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.
More information about the Devel