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.
Further comments below.
On Wed, Jul 7, 2010 at 18:23, Nick Treleaven nick.treleaven@btinternet.com wrote:
On Sun, 4 Jul 2010 00:54:17 +0200 Jiří Techet techet@gmail.com wrote:
This time I did my best to integrate the plugin to geany as well as possible so there are more patches for geany itself than before. Apart from patches needed for gproject there are several more patches that fix some bugs in geany or are meant to improve the usability. The
Just to mention: I'm not sure if I/we have much time in the next few weeks, so reviewing (& applying) them might get delayed.
No problem, take your time.
geany patches are here:
http://gitorious.org/~techy/geany/gproject-geany
under the "for_review" branch. Below there is a detailed log of the changes. I expect some discussion for some of the patches (they represent my view on the problem but you may have a different opinion) so when replying to this email it's probably best to take the commit title as the email's title and also keep the detailed commit message in the body of the email so everybody knows what we're talking about. Once there is some agreement about the future of the patches, I can resend them by email - I just don't want to spam the mailing list with too many messages.
I'll try to give a very brief comment on each/some below without really reading the code.
Rewrite tab switching queue
maybe OK
Remove the "set" button from the project properties dialog
Unless I miss something the button just adds %d to the corresponding fields, but this is already the default settings so I don't see any point of doing it.
I think so too.
I was wrong about %d being default (it was set this way in my global settings) but I still don't like the button much. First it takes too much time for my brain to understand what the long label says, second it's no problem to set the %d manually. But I don't care too much about this particular patch.
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.
File name in the project settings dialog shouldn't look it is editable
OK.
Use wider entry for project file path
OK.
When closing tab, return to the document at the top of the MRU list
maybe OK
Open the file in the msgwindow even if no linenumber is specified
OK
Make it possible for plugins to change the base directory of msgwindow
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.
(The patch (mis)uses short evaluation of boolean expressions in C so this is probably what you have overlooked.)
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
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?
http://dl.dropbox.com/u/2554438/project-menu.png
The "find in project files" item is identical to "find in files" but always using the project's base directory no matter what file is active.
The reason why I think it's better to put these entries into the project menu is that when the user uses my gproject plugin, it is completely invisible for him that it's some kind of extension of geany - everything is integrated to the current project handling and it would look a bit strange if some entries related to the project would be under the project menu and some under the tools 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.)
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. Apart from that, I pass only the notebook as the signal's argument so I cannot connect to anything that's outside of it (like the buttons).
One more thing that might be needed by plugins (but which isn't needed by gproject so I haven't added the API methods) is the ability to add their own validators of the dialog entries the given plugins added. There could be something like register_validator() and unregister_validator() whose argument would be a pointer to the validator function. Geany would keep a list of these pointers and run all the functions to see if everything is valid before it sends the above signal. The functions could also return some error string used by a pop up error dialog. As I said, I don't need it but could be added when some plugin requires such a functionality.
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). 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.
Cheers,
Jiri
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel