@elextr Thanks for noticing - apparently scope info is missing, this also affects current master. It seems we don't have asciidoc and rest unit tests :(.

@techee overlapping comments :)

Would tests have picked up extension stuff?

Would tests have picked up extension stuff?

Yep, this would appear in the tags file. I'll grab some tests from uctags (and maybe check if we have at least 1 test for every file type) so problems like that don't appear in the future - especially important now that we make many changes in our ctags implementation

@techee since master is also broken please make your patch a separate PR on master so it can be fixed.

I've opened a PR: #2019. It lacks test cases and should be tested by some Asciidoc guy :) @techee @elextr I likely won't have time to touch this before late evening at best, so feel free to work on improving this, test cases fixes and stuff. I'll mention if I'm back on it before.

Sorry, hoped to work on this earlier but didn't have time in the last two days (will address the issues hopefully today).

@b4n @techee I used the asciidoc.c from upstream ctags on top of this PR (and changed the map_ASCIIDOC in tm_parser.c to match) IT WORKS!!!! 🥇 😀

Cool! If it's the case (I haven't checked yet), I'd suggest using the upstream versions and if there's a better way to implement this, then we could go the (hopefully new standard) "implement in uctags, import to Geany" way. @b4n What do you think?

So all we have to do is merge this quickly and import the upstream 😄 (well and @b4n make the change to tm_parser.c properly).

And even more cool thing is having @elextr excited to get this merged into Geany soon :-).

@b4n Is it necessary to have a separate fix for master if it gets fixed in this branch? Master is mildly broken because of this now but it's not something super-critical like a crash and I think it could wait until this pull request is merged.

why the TxtToTags cases are removed? or t2t isn't Txt2Tags?

Ah, OK, the test got broken and without checking I somehow assumed it was Actionscript. Seems there's a problem with scope separators in this test - I'll look into this.

@techee pushed 3 commits.

89ed899a7836e325c90ebf2b2f71362fa4ec1663 Rename rest.c to rst.c to match upstream filename 1cb2b2c464956129e666b6adffbd95068fdcbadb Use upstream asciidoc and rst parsers aea36b485c0f5ff3d6007bdc0342298c20f30461 Add asciidoc and rst unit tests

elextr commented on this pull request.

static TMParserMapEntry map_ASCIIDOC[] = {

- {'n', tm_tag_namespace_t}, - {'m', tm_tag_member_t}, - {'d', tm_tag_macro_t}, - {'v', tm_tag_variable_t}, - {'s', tm_tag_struct_t}, + {'c', tm_tag_namespace_t}, + {'s', tm_tag_member_t}, + {'S', tm_tag_macro_t}, + {'t', tm_tag_variable_t}, + {'T', tm_tag_struct_t}, + {'u', tm_tag_undef_t}, + {'a', tm_tag_undef_t},

@techee the last two don't work, is that because you used `tm_undef_t`, and if so why not allow them to work?

techee commented on this pull request.

static TMParserMapEntry map_ASCIIDOC[] = {

- {'n', tm_tag_namespace_t}, - {'m', tm_tag_member_t}, - {'d', tm_tag_macro_t}, - {'v', tm_tag_variable_t}, - {'s', tm_tag_struct_t}, + {'c', tm_tag_namespace_t}, + {'s', tm_tag_member_t}, + {'S', tm_tag_macro_t}, + {'t', tm_tag_variable_t}, + {'T', tm_tag_struct_t}, + {'u', tm_tag_undef_t}, + {'a', tm_tag_undef_t},

Yes, that's the reason - I disabled them intentionally for now. We didn't have these two in the parser before and I didn't want to spend time exploring what exactly would happen with them in the sidebar if we mapped them to something existing. Can be improved in the future.

Hint to everybody else, try this PR with a different language for xmas and new year, and be sure to report your results :)

I have added some more unit tests - again, I made sure we get the same output when these tests are applied on top of Geany 1.34 tag to be sure the recent ctags changes didn't break something.

@techee pushed 1 commit.

608fe0f08e4fb2a00c5f5a5d42e8cd6725b120e4 Initialize trashbox

@elextr they are disabled by this patchset: from the OP, *“[…] This patch also disables regex parsing […]”*.

@elextr the annoying part is that it brings an extra bundled copy of GNU Regex for platforms that don't have `regcomp()` (Windows is the most obvious candidate), yet GLib already does something very similar to provide GRegex. So on some situations we could end with 2 copies of it :confused:

Currently we have a GRegex-based version of it, but it's not necessarily 100% compatible, and certainly doesn't have all the new stuff from upstream. The question @techee raises is: is it worth maintaining this for the few parsers that use it? What should we do? To be fair, the most important use case for upstream is dynamic parser definition (with various command-line switches) allowing to create parsers fairly easily for fairly simple things (although it got more and more powerful lately) -- and we don't have support for this, at least not yet.

techee commented on this pull request.

{

- const kindOption *kindOpt; - - if (hasRegexKind (language, kind)) - return isRegexKindEnabled (language, kind); - else if (hasXcmdKind (language, kind)) - return isXcmdKindEnabled (language, kind);

The new code is how it looks upstream. I don't think the original code very relevant for us - it just checks some command line options to see if the parser is enabled.

b4n commented on this pull request.

{

- const kindOption *kindOpt; - - if (hasRegexKind (language, kind)) - return isRegexKindEnabled (language, kind); - else if (hasXcmdKind (language, kind)) - return isXcmdKindEnabled (language, kind);

Ah OK great then

So can't we just make it a shallow shim over glib instead of including the ctags one?

Probably we could make some wrapper around it but there are minor semantic differences between the APIs which are easy to miss and which we probably won't notice in the future when something in upstream uctags changes and we just grab the sources from there.

All the parsers upstream from the "parsers" directory are non-regex ones and the regex ones are only those from

https://github.com/universal-ctags/ctags/tree/master/optlib

but none of the languages seem too interesting for Geany (we have a hand-written markdown parser). So we won't miss much from the upstream by killing the regex parser support. So the only problem are Actionscript and Cobol parsers. As I said, personally, I would drop them. But the second best solution would be rewriting them as non-regex parsers (but preferably not by me - @b4n you said something like it would be fun rewriting those - I would really hate myself for stealing the fun from you :-)

Hehe 😄 I gave COBOL a little try, but it now it seems a lot more of a nightmare to do it right (from what I gathered, not only the official syntax uses a pretty random notation, but it also seem close to impossible to parse without knowing every single bit of it -- e.g. it's not easy to locate "statements" or alike). But well… I'll still try and find a way.

If you really insist on having this fun, couldn't you just use the readLineFromInputFile() ctags interface and more or less only simulate the regex expressions from the existing regex parser (without actually having to learn how the language looks like)?

@elextr Since you like finding all the possible cases where the current C parser fails, would you give it a try?

@techee ahh, no, not until this gets committed, _then_ you can make a PR to use the new C++ parser instead of the one in c.c. After all once this is committed we will need another ctags PR to replace it :grin:

@techee ahh, no, not until this gets committed, then you can make a PR to use the new C++ parser instead of the one in c.c.

Sure. I just thought you might be interested to try it before and see if it fixes some of the problems with the existing C/C++ parser for you.

Merged #2018 into master.

I don't think we should release with this until the broken languages are fixed.

This PR shouldn't have been merged until that was done, as per @b4n's review requesting changes first, or at least until there was some kind of consensus that it should be merged.

@elextr please don't squash-merge this kind of big PRs with great commit splitting, we now lost the whole history here, and although yes, some commits were big already, most were very well split apart. I'll also make bisecting a potential issue a lot harder.

Anyway, as per the merge itself: I obviously am not a fan, but well, I also admit we need to move forward at some point, and this kind of forces (my) hand. Sad it happens right when I'm back with some time for Geany dev… well I guess them I should have *started* with this one yesterday instead of #2092.

1. it fixes three languages, Asciidoc, Rest (Geanys own documentation) and Latex and progresses the process of making it easier to maintain parsers from upstream.

Let's just be fair: I already proposed a standalone fix for Asciidoc and Rest (#2019) without this PR, and you rejected it because merging this was a better choice -- which is fair enough, but then not so much an argument for pushing this one :) I wasn't aware that Latex was broken in master, probably to blame on my lack of Geany time lately.

2. It has been working for me for months

That's a great point :)

3. If we keep delaying every ctags update for a year we will always have huge updates :(

yes, but with this PR, so long as it can still be merged in Geany, we should have a clean-ish diff from uctags, meaning update should be fairly easy no matter how big they are, as most can just be applied as-is.

The downside is it loses Cobol

An ActionScript as well. I don't think we can release in the current state without those, especially as we have at least one active COBOL user that even submitted changes recently.

Anyway, we now have to pull up our sleeves and fix what needs to be fixed before the release. Help is welcome.

please don't squash-merge this kind of big PRs with great commit splitting

Could not the squashed merge be reverted and re-merged with the individual commits? (not that I agree with merging this at all until it doesn't cause serious regressions).

I don't know where I can ask: what about "Hacking Geany" > "Adding a filetype"? Is this section needs updating?

@elextr, yes, section ```Adding a TagManager parser``` is interesting firstly (possible changes in updating ```add_top_level_items()``` & adding ```TMParserMapEntry ```). Ok, thanks, I thought so, but I decided to clarify this moment, just in case :)