On Fri, Jul 9, 2010 at 13:57, Nick Treleaven nick.treleaven@btinternet.com wrote:
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).
OK, will do (maybe I'll wait for a while to collect everyone else's comments and possibly modify the patches).
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:
- 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.
But this also means that every time a project is opened, the patterns have to be selected manually from the list (easy to forget to do so and it also won't be clear to the user which patterns in the list belong to the project).
- An API function to show the dialog with an argument for file
patterns.
I think it wouldn't be a good idea to force the patterns every time the dialog is opened (neither by default nor by any plugin). The user may have customized the patterns and this would just reset them.
The advantage would be that the dialog is simpler - less controls & complexity.
OK, I have one more idea how the dialog might look like, see the mockups below:
http://dl.dropbox.com/u/2554438/fif1.png http://dl.dropbox.com/u/2554438/fif2.png
For "all" and "project" the patterns entry would be disabled (for project it would display the project's patterns). I think this solution doesn't add much complexity - the two-state check button just changes to three-state combo box.
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.
The reason is simplicity. This plugin was one afternoon work for me. I used readtags.[ch] from ctags to read the tags file, then took the code from geany's build subsystem to run the tags generation asynchronously, then added some code to read the code under the cursor and after finding the tag I put the output to the message window. So the work can be characterized as stealing someone else's work, slightly modifying it and putting it together.
I spent very limited time looking at the tag manager so the following can be quite wrong. From what I have seen it creates some object hierarchy by going through the objects and for every object finding the related object (quadratic complexity at least). Probably this could be improved by using hash tables instead of lists but I would really have to dig into the code deeply to understand it and find where the bottlenecks are. In contrast, ctags tag generation is mostly (apart from sorting the tag file) linear. Just for illustration, for linux kernel the generation of the 140MB tag file containing about 1.6 million entries from all *.c and *.h files takes less than 20s.
In fact I think the best way to develop the tag manager would be _outside_ of geany so all other editors could benefit from it. Unfortunately from what I can see both tag manager and slightly less ctags are rather dead.
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).
Yep, sounds reasonable.
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.
I'll leave it up to you - if you decide for changing the source file into a header, it's better when it's done by you as I might break some things I'm not aware of.
Regards,
Jiri
But I might apply the patch.
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel