There is various language-specific code related to tag manager which is scattered across Geany codebase. First, it makes such code hard to maintain, second, it makes it hard to discover for people adding support for a new parser to Geany. This code includes:
1. Formatting of functions for calltips and popups for sidebar (also for variables). This is language-specific where e.g. function return type can be in front of function for C but behind function for go. 2. Displaying of calltips for object constructors based on certain method name (__init__() arguments after typing `MyClass(` for Python) - there were actually two different implementations of this feature, one for Python, one for D. 3. Code detecting whether scope autocompletion should be triggered - the characters triggering it are specific to used languages.
Since `tm_parser.c` already contains language-specific mappings from parsers, it seems to be a good home for this kind of code.
The only remaining language-specific tag manager related code is the mappings of tags to the symbol tree in `symbols.c` which I'd like to move to `tm_parser.c` once the various parser pull requests are merged (not to introduce merge conflicts). You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3060
-- Commit Summary --
* Move function/variable formatting into tm_parser * Handle calltips of Python constructors like D constructors * Move language-specific part of constructor calltip lookup to tm_parser * Move language-specific parts of whether to trigger scope autocompletion to tm_parser * Remove invalid python-specific code * Display python type annotations correctly formatted
-- File Changes --
M src/editor.c (131) M src/editor.h (2) M src/symbols.c (17) M src/tagmanager/tm_ctags.c (33) M src/tagmanager/tm_parser.c (119) M src/tagmanager/tm_parser.h (9) M tests/ctags/cython_sample.pyx.tags (2) M tests/ctags/py_constructor_arglist.py.tags (2) M tests/ctags/simple.py.tags (2)
-- Patch Links --
https://github.com/geany/geany/pull/3060.patch https://github.com/geany/geany/pull/3060.diff
LGBI and I like the approach and implementation: old, cluttered code refactored into more readable code. I'll try the changes for a bit and am willing to merge soon.
@eht16 Great, thanks. The idea of this refactoring is basically that if someone wants to add new parser or update update a parser to a newer version with new capabilities, `tm_parser.c` should be the only file that has to be modified.
@techee are you fine to have the commits squashed together or keep them as is or do you want to squash them yourself?
@techee are you fine to have the commits squashed together or keep them as is or do you want to squash them yourself?
Just to make sure - are you really talking about this PR or rather #3055? No problem to squash them for #3055 but for this PR it's better to have commits separate IMO.
I was talking about *this* PR as I want to merge it.
Ah, ok, IMO these commits are quite nicely separated with separate commit messages for each of the changes and make more sense individually than squashed I think.
Merged #3060 into master.
Thanks for merging!
@eht16 Can we please not use the "rebase and merge" strategy? That makes it look like a bunch of commits was pushed directly onto the branch with no trace to the PR. Just look at the master branch now. Lets use squash and normal merge only.
Ok, sorry about that. Will do in the future.
github-comments@lists.geany.org