Hi,
while adding the ctags parser for go here
https://github.com/geany/geany/pull/373
I noticed that it's necessary to change the tag types (the GoKinds[] array in the case of go) to match the supported types in the tag manager and Geany. This has to be done for every parser because the tag types aren't standardized in any way in ctags and can be more or less arbitrary.
In my opinion, doing this in the parser itself is a bit unfortunate - when updating a parser from the ctags repository, these have to be changed. Worse, if ever something like ctags shared library happens and all the parser support is moved there, these will have to be always changed in every parser after updating to new ctags version.
I think it would be better to keep the tag types inside the individual parsers as they are and have a separate table in the tag manager which will map the ctags type to the type used by Geany.
The only disadvantage of this approach I can think of is that the type names might change in ctags and we could easily miss this change and not update the mapping. For this reason it would be good to add some "integrity check" function which goes through all the kinds in ctags for every language and checks whether they are mapped to Geany's tag types and vice versa. But I think this can be done quite easily so it isn't a real problem.
What do you think?
Cheers,
Jiri
Hey,
Le 07/11/2014 17:38, Jiří Techet a écrit :
[…]
I noticed that it's necessary to change the tag types (the GoKinds[] array in the case of go) to match the supported types in the tag manager and Geany. This has to be done for every parser because the tag types aren't standardized in any way in ctags and can be more or less arbitrary.
In my opinion, doing this in the parser itself is a bit unfortunate - when updating a parser from the ctags repository, these have to be changed. Worse, if ever something like ctags shared library happens and all the parser support is moved there, these will have to be always changed in every parser after updating to new ctags version.
I think it would be better to keep the tag types inside the individual parsers as they are and have a separate table in the tag manager which will map the ctags type to the type used by Geany.
Yep, I was thinking the same a few days ago while discussing a little CTags shared library, and had the same idea :)
When upstream CTags was basically dead it didn't matter much as we pretty much never updated anything, but now there is some activity again (even though it's not upstream yet) it would probably make sense.
Note that there is another potential issue when importing a parser: use of MIO. It affects fewer parsers, but a few are using fpos_t (mostly through getInputFilePosition()), and we can't use this for memory parsing. It's generally just a matter of s/fpos_t/MIOPos/ and the compiler will whine, but that's a step.
The only disadvantage of this approach I can think of is that the type names might change in ctags and we could easily miss this change and not update the mapping. For this reason it would be good to add some "integrity check" function which goes through all the kinds in ctags for every language and checks whether they are mapped to Geany's tag types and vice versa. But I think this can be done quite easily so it isn't a real problem.
Yeah, updating would still require some care, but not much more than right now. And as you say we could have a tool listing missing mappings.
What do you think?
I didn't really think of the implications so I can't tell if it has other problems, but I feel like it's a good way to go.
Regards, Colomban
On Fri, Nov 7, 2014 at 7:20 PM, Nick Treleaven nick.treleaven@btinternet.com wrote:
On 07/11/2014 17:26, Colomban Wendling wrote:
Yep, I was thinking the same a few days ago while discussing a little CTags shared library, and had the same idea
+1
Good, if there's an agreement here, I can have a look at it. But I'll wait until some of my TM patches get merged. There are a bit too many of them floating around and I'm slightly getting lost in all of my branches.
Another thing I have noticed is that it would be good to make sTagEntryInfo as close to ctags as possible - for instance, the "signature" member in Geany is called "argList" in ctags and "seekPosition" is unused and not present in ctags.
Le 08/11/2014 12:27, Jiří Techet a écrit :
[…]
Good, if there's an agreement here, I can have a look at it. But I'll wait until some of my TM patches get merged. There are a bit too many of them floating around and I'm slightly getting lost in all of my branches.
I'll try to merge the tm branch as soon as possible, just a few small comments as you saw.
Another thing I have noticed is that it would be good to make sTagEntryInfo as close to ctags as possible - for instance, the "signature" member in Geany is called "argList" in ctags and "seekPosition" is unused and not present in ctags.
Good idea indeed. I just gave it a look, and it should be pretty easy but for our "varType" field (a string) v.s. CTags' "typeRef" (a pair of strings of the form [kind, name]). Those are not really the same, and are both somewhat interesting (though ours seems a little better to me):
Our varType is used to report a function's return type, or a variable type, or the type mapped to a typedef for example. And it includes the whole type, including 'struct', '*'s and '[]' -- which is both useful (for the user to see) and partly impractical (for using it from the code).
CTags' typeRef seems only used to report C typedefs, and has two parts, a "kind" part and a name part. For example, `typedef struct foo bar;` would generate a typeRef of ["struct", "foo"] -- which is partly useful to resolve the correct definition of "foo" in case there are several, but not so much as generally you can rule out anything but types when resolving a typedef.
The problem here is that there isn't always a "kind" to associate with the return type of a function, there only is if it's a custom type, and even then only it if isn't a typedef. E.g. we have nothing to put in typeRef's "kind" for `int printf(const char *fmt, ...)`, or even `GeanyDocument *document_get_current(void)`.
For now it's probably not so much of a problem as in CTags only the C parser seems to use it -- and we won't want to just use this one right away because it misses some things ours handle (yet the contrary is probably also true) -- but it might be useful to solve this problem anyway.
So, what do you think about this?
Le 08/11/2014 17:50, Colomban Wendling a écrit :
Le 08/11/2014 12:27, Jiří Techet a écrit :
[…]
Good, if there's an agreement here, I can have a look at it. But I'll wait until some of my TM patches get merged. There are a bit too many of them floating around and I'm slightly getting lost in all of my branches.
I'll try to merge the tm branch as soon as possible, just a few small comments as you saw.
Another thing I have noticed is that it would be good to make sTagEntryInfo as close to ctags as possible - for instance, the "signature" member in Geany is called "argList" in ctags and "seekPosition" is unused and not present in ctags.
Good idea indeed. I just gave it a look, and it should be pretty easy but for our "varType" field (a string) v.s. CTags' "typeRef" (a pair of strings of the form [kind, name]). […]
I did the easy removals/renames in https://github.com/b4n/geany/commits/ctags-tag-entry (that's on top of TM branch, but easy to change or pick up specific commits) to see how easy it'd be -- and this part was trivial.
Cheers, Colomban
On Sat, Nov 8, 2014 at 7:30 PM, Colomban Wendling lists.ban@herbesfolles.org wrote:
I did the easy removals/renames in https://github.com/b4n/geany/commits/ctags-tag-entry (that's on top of TM branch, but easy to change or pick up specific commits) to see how easy it'd be -- and this part was trivial.
Yes, it was these easy to do renames I had in mind. Regarding the hard to do ones, I don't know, I'm not that familiar with ctags - it would be best to get both in sync - either by changing Geany-ctags or fishman-ctags.
Cheers, Jiri