[Geany-devel] GProject - the second attempt

Fri Jul 9 20:53:17 UTC 2010

On Fri, Jul 9, 2010 at 13:57, Nick Treleaven
<nick.treleaven at btinternet.com> wrote:
> 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).

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:
>
> 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.

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).

>
> 2. 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?
>>
>> http://dl.dropbox.com/u/2554438/project-menu.png
>
> 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#a5d574ade65760148f4d1fcdd41d12b7
>
>> >>     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 at uvena.de
> http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
>