[Geany-devel] GProject - the second attempt

[Jiří Techet]

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 at btinternet.com> wrote:
> On Sun, 4 Jul 2010 00:54:17 +0200
> Jiří Techet <techet at 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 at uvena.de
> http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
>